Horton

Thread

Commits

  1. Fix printf format string warning on MinGW.

  2. Backpatch: Fix warnings about declaration of environ on MinGW

  3. Fix warnings about declaration of environ on MinGW.

  4. Fix compiler warnings in ecpg tests

  1. MinGW compiler warnings in ecpg tests

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2019-10-25T19:32:45Z

    Under MinGW, when compiling the ecpg test files, you get these warnings:

sqlda.pgc: In function 'dump_sqlda':
sqlda.pgc:44:11: warning: unknown conversion type character 'l' in format [-Wformat=]
    printf("name sqlda descriptor: '%s' value %lld\n", sqlda->sqlvar[i].sqlname.data, *(long long int *)sqlda->sqlvar[i].sqldata);
sqlda.pgc:44:11: warning: too many arguments for format [-Wformat-extra-args]
sqlda.pgc:44:11: warning: unknown conversion type character 'l' in format [-Wformat=]
sqlda.pgc:44:11: warning: too many arguments for format [-Wformat-extra-args]

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.

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

  2. Re: MinGW compiler warnings in ecpg tests

    Michael Meskes <meskes@postgresql.org> — 2019-10-26T08:40:54Z

    > 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

  3. Re: MinGW compiler warnings in ecpg tests

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2019-10-29T08:40:48Z

    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

  4. RE: MinGW compiler warnings in ecpg tests

    Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> — 2023-11-10T07:59:49Z

    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].
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]: https://www.postgresql.org/message-id/9f4f22be-f9f1-b350-bc06-521226b87f7a%40dunslane.net

Best Regards,
Hayato Kuroda
FUJITSU LIMITED

  5. Re: MinGW compiler warnings in ecpg tests

    Thomas Munro <thomas.munro@gmail.com> — 2024-12-05T12:15:52Z

    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.

  6. Re: MinGW compiler warnings in ecpg tests

    Tom Lane <tgl@sss.pgh.pa.us> — 2024-12-05T15:11:11Z

    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

  7. Re: MinGW compiler warnings in ecpg tests

    Thomas Munro <thomas.munro@gmail.com> — 2024-12-06T02:44:20Z

    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 :-)

  8. Re: MinGW compiler warnings in ecpg tests

    Andres Freund <andres@anarazel.de> — 2025-10-30T21:39:24Z

    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], 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.

Greetings,

Andres Freund

[1] https://postgr.es/m/CAN55FZ1_B1usTskAv%2BAYt1bA7abVd9YH6XrUUSbr-2Z0d5Wd8w%40mail.gmail.com
[2] https://cirrus-ci.com/task/6526575971139584

  9. Re: MinGW compiler warnings in ecpg tests

    Andres Freund <andres@anarazel.de> — 2025-11-04T18:34:29Z

    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