MinGW compiler warnings in ecpg tests

These files don't use our printf replacement or the c.h porting
layer,
so unless we want to start doing that, I propose the attached patch
to
determine the appropriate format conversion the hard way.

I don't think such porting efforts are worth it for a single test case,
or in other words, if you ask me go ahead with your patch.

Michael
--
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Meskes at (Debian|Postgresql) dot Org
Jabber: michael at xmpp dot meskes dot org
VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL

On 2019-10-26 10:40, Michael Meskes wrote:

These files don't use our printf replacement or the c.h porting
layer,
so unless we want to start doing that, I propose the attached patch
to
determine the appropriate format conversion the hard way.

I don't think such porting efforts are worth it for a single test case,
or in other words, if you ask me go ahead with your patch.

committed

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Dear Peter, Michael,

Sorry for reviving the old thread. While trying to build postgres on msys2 by meson,
I faced the same warning. The OS is Windows 10.

```
$ ninja
[2378/2402] Compiling C object src/interfaces/ecpg/test/sql/sqlda.exe.p/meson-generated_.._sqlda.c.obj
../postgres/src/interfaces/ecpg/test/sql/sqlda.pgc: In function 'dump_sqlda':
../postgres/src/interfaces/ecpg/test/sql/sqlda.pgc:45:33: warning: format '%d' expects argument of type 'int', but argument 3 has type 'long long int' [-Wformat=]
45 | "name sqlda descriptor: '%s' value %I64d\n",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
......
49 | sqlda->sqlvar[i].sqlname.data, *(long long int *)sqlda->sqlvar[i].sqldata);
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| |
| long long int
```

Before building, I did below steps:

1. Installed required software listed in [1]/messages/by-id/9f4f22be-f9f1-b350-bc06-521226b87f7a@dunslane.net.
2. ran `meson setup -Dcassert=true -Ddebug=true /path/to/builddir`
3. moved to /path/to/builddir
4. ran `ninja`
5. got above warning

Attached file summarize the result of meson command, which was output at the end of it.
Also, belows show the version of meson/ninja.

```
$ ninja --version
1.11.1
$ meson -v
1.2.3
```

I was quite not sure the windows build, but I could see that gcc compiler was
used here. Does it mean that the compiler might not like the format string "%I64d"?
I modified like below and could be compiled without warnings.

```
--- a/src/interfaces/ecpg/test/sql/sqlda.pgc
+++ b/src/interfaces/ecpg/test/sql/sqlda.pgc
@@ -41,7 +41,7 @@ dump_sqlda(sqlda_t *sqlda)
                        break;
                case ECPGt_long_long:
                        printf(
-#ifdef _WIN32
+#if !defined(__GNUC__)
                                "name sqlda descriptor: '%s' value %I64d\n",
 #else
                                "name sqlda descriptor: '%s' value %lld\n",

```

[1]: /messages/by-id/9f4f22be-f9f1-b350-bc06-521226b87f7a@dunslane.net

Best Regards,
Hayato Kuroda
FUJITSU LIMITED

On Fri, Nov 10, 2023 at 9:00 PM Hayato Kuroda (Fujitsu)
<kuroda.hayato@fujitsu.com> wrote:

Sorry for reviving the old thread. While trying to build postgres on msys2 by meson,
I faced the same warning. The OS is Windows 10.

Yeah. This warning is visible on CI, and on fairywren since its MSYS2
upgrade a couple of months ago. Old MinGW didn't like %lld (I think
perhaps the printf from msvcrt.dll from 1996 didn't like it and MinGW
knew that), but new MinGW doesn't like %I64d (that's interesting, but
not relevant here because %lld is clearly the correct format string,
and it works). We should just revert that change. Here's a patch.

That leaves only three more annoying warnings:

../pgsql/src/backend/postmaster/postmaster.c:857:31: warning:
'__p__environ' redeclared without dllimport attribute: previous
dllimport ignored [-Wattributes]
857 | extern char **environ;

Those were there before the upgrade. POSIX says that environ should
not be declared by a header, but Windows apparently declares it, or at
least its cousin _environ, in <stdlib.h> which we include in c.h. I
have no idea why Visual Studio doesn't warn, or why the documentation
only tells you about _environ and not environ, or where the macro (?)
comes from that renames it, but it passes CI and is
warning-free on both toolchains if you just hide the offending
declarations.

Thomas Munro <thomas.munro@gmail.com> writes:

Yeah. This warning is visible on CI, and on fairywren since its MSYS2
upgrade a couple of months ago. Old MinGW didn't like %lld (I think
perhaps the printf from msvcrt.dll from 1996 didn't like it and MinGW
knew that), but new MinGW doesn't like %I64d (that's interesting, but
not relevant here because %lld is clearly the correct format string,
and it works). We should just revert that change. Here's a patch.

+1

Those were there before the upgrade. POSIX says that environ should
not be declared by a header, but Windows apparently declares it, or at
least its cousin _environ, in <stdlib.h> which we include in c.h. I
have no idea why Visual Studio doesn't warn, or why the documentation
only tells you about _environ and not environ, or where the macro (?)
comes from that renames it, but it passes CI and is
warning-free on both toolchains if you just hide the offending
declarations.

Isn't this likely to break things for every other Windows toolchain?
I think the concept might be OK, but we need a tighter #if condition.

An alternative could be to add the missing dllimport on Windows;
it's not clear whether other toolchains would care.

regards, tom lane

On Fri, Dec 6, 2024 at 4:13 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Thomas Munro <thomas.munro@gmail.com> writes:

Yeah. This warning is visible on CI, and on fairywren since its MSYS2
upgrade a couple of months ago. Old MinGW didn't like %lld (I think
perhaps the printf from msvcrt.dll from 1996 didn't like it and MinGW
knew that), but new MinGW doesn't like %I64d (that's interesting, but
not relevant here because %lld is clearly the correct format string,
and it works). We should just revert that change. Here's a patch.

+1

Thanks for looking. Pushed, and that fixed that on fairywren.

Those were there before the upgrade. POSIX says that environ should
not be declared by a header, but Windows apparently declares it, or at
least its cousin _environ, in <stdlib.h> which we include in c.h. I
have no idea why Visual Studio doesn't warn, or why the documentation
only tells you about _environ and not environ, or where the macro (?)
comes from that renames it, but it passes CI and is
warning-free on both toolchains if you just hide the offending
declarations.

Isn't this likely to break things for every other Windows toolchain?
I think the concept might be OK, but we need a tighter #if condition.

Cool, I'll do that for MinGW only then.

An alternative could be to add the missing dllimport on Windows;
it's not clear whether other toolchains would care.

I've been digging through its headers (working on a fix for the off_t
bug report) and noticed in passing that it probably thinks we're
re-declaring this function:

https://github.com/mingw-w64/mingw-w64/blob/8bcd5fc1a72c0b6da3147bf21a4a494c81d14fae/mingw-w64-headers/crt/stdlib.h#L221

Seems like a good idea to give that a wide berth :-)

Hi,

On 2024-12-06 15:44:20 +1300, Thomas Munro wrote:

On Fri, Dec 6, 2024 at 4:13 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Thomas Munro <thomas.munro@gmail.com> writes:

Yeah. This warning is visible on CI, and on fairywren since its MSYS2
upgrade a couple of months ago. Old MinGW didn't like %lld (I think
perhaps the printf from msvcrt.dll from 1996 didn't like it and MinGW
knew that), but new MinGW doesn't like %I64d (that's interesting, but
not relevant here because %lld is clearly the correct format string,
and it works). We should just revert that change. Here's a patch.

+1

Thanks for looking. Pushed, and that fixed that on fairywren.

Those were there before the upgrade. POSIX says that environ should
not be declared by a header, but Windows apparently declares it, or at
least its cousin _environ, in <stdlib.h> which we include in c.h. I
have no idea why Visual Studio doesn't warn, or why the documentation
only tells you about _environ and not environ, or where the macro (?)
comes from that renames it, but it passes CI and is
warning-free on both toolchains if you just hide the offending
declarations.

Isn't this likely to break things for every other Windows toolchain?
I think the concept might be OK, but we need a tighter #if condition.

Cool, I'll do that for MinGW only then.

I was looking at merging [1]/messages/by-id/CAN55FZ1_B1usTskAv+AYt1bA7abVd9YH6XrUUSbr-2Z0d5Wd8w@mail.gmail.com, however the backbranches < 18 fail in
CompilerWarnings due to this error [2]https://cirrus-ci.com/task/6526575971139584, after upgrading to trixie. Seems like
we ought to backpatch 7bc9a8bdd2d. Haven't checked yet whether 1319997d is
also required for a clean build.

Greetings,

Andres Freund

[1]: /messages/by-id/CAN55FZ1_B1usTskAv+AYt1bA7abVd9YH6XrUUSbr-2Z0d5Wd8w@mail.gmail.com
[2]: https://cirrus-ci.com/task/6526575971139584

Hi,

On 2025-10-30 17:39:24 -0400, Andres Freund wrote:

On 2024-12-06 15:44:20 +1300, Thomas Munro wrote:

On Fri, Dec 6, 2024 at 4:13 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Thomas Munro <thomas.munro@gmail.com> writes:

Yeah. This warning is visible on CI, and on fairywren since its MSYS2
upgrade a couple of months ago. Old MinGW didn't like %lld (I think
perhaps the printf from msvcrt.dll from 1996 didn't like it and MinGW
knew that), but new MinGW doesn't like %I64d (that's interesting, but
not relevant here because %lld is clearly the correct format string,
and it works). We should just revert that change. Here's a patch.

+1

Thanks for looking. Pushed, and that fixed that on fairywren.

Those were there before the upgrade. POSIX says that environ should
not be declared by a header, but Windows apparently declares it, or at
least its cousin _environ, in <stdlib.h> which we include in c.h. I
have no idea why Visual Studio doesn't warn, or why the documentation
only tells you about _environ and not environ, or where the macro (?)
comes from that renames it, but it passes CI and is
warning-free on both toolchains if you just hide the offending
declarations.

Isn't this likely to break things for every other Windows toolchain?
I think the concept might be OK, but we need a tighter #if condition.

Cool, I'll do that for MinGW only then.

I was looking at merging [1], however the backbranches < 18 fail in
CompilerWarnings due to this error [2], after upgrading to trixie. Seems like
we ought to backpatch 7bc9a8bdd2d. Haven't checked yet whether 1319997d is
also required for a clean build.

After verifying that 1319997d is not required I backpatched 7bc9a8bdd2d to all
the branches lacking it.

Greetings,

Andres Freund