Thread
Commits
-
Fix MinGW %d vs %lu warnings in back branches.
- fce6fd5da580 13.19 landed
- fa92c18efe24 14.16 landed
- 29cd8a41ccb5 15.11 landed
-
Bump MIN_WINNT for MINGW to clear a build error
- d700e8d75bc3 15.9 cited
-
CI CompilerWarnings test fails on 15 in mingw_cross_warning
Andres Freund <andres@anarazel.de> — 2024-11-17T01:59:52Z
Hi, See https://cirrus-ci.com/task/5880116075560960 [18:14:04.821] time make -s -j${BUILD_JOBS} world-bin [18:15:49.090] pg_locale.c: In function ‘get_collation_actual_version’: [18:15:49.090] pg_locale.c:1763:42: error: format ‘%d’ expects argument of type ‘int’, but argument 2 has type ‘long unsigned int’ [-Werror=format=] [18:15:49.090] 1763 | collversion = psprintf("%d.%d,%d.%d", [18:15:49.090] | ~^ [18:15:49.090] | | [18:15:49.090] | int [18:15:49.090] | %ld [18:15:49.090] 1764 | (version.dwNLSVersion >> 8) & 0xFFFF, [18:15:49.090] | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [18:15:49.090] | | [18:15:49.090] | long unsigned int I have no idea why we are seeing this error now when we didn't in the past - there don't seem to have been any relevant changes? It does reproduce on my debian sid machine, so it's something we ought to fix, I think? We did fix it in newer versions: Author: Peter Eisentraut <peter@eisentraut.org> Branch: master Release: REL_16_BR [a9bc04b21] 2023-03-24 07:21:40 +0100 Fix incorrect format placeholders The fields of NLSVERSIONINFOEX are of type DWORD, which is unsigned long, so the results of the computations being printed are also of type unsigned long. Peter, any reason you didn't backpatch that? Greetings, Andres Freund -
Re: CI CompilerWarnings test fails on 15 in mingw_cross_warning
Peter Eisentraut <peter@eisentraut.org> — 2024-11-18T11:09:26Z
On 17.11.24 02:59, Andres Freund wrote: > Hi, > > See https://cirrus-ci.com/task/5880116075560960 > > [18:14:04.821] time make -s -j${BUILD_JOBS} world-bin > [18:15:49.090] pg_locale.c: In function ‘get_collation_actual_version’: > [18:15:49.090] pg_locale.c:1763:42: error: format ‘%d’ expects argument of type ‘int’, but argument 2 has type ‘long unsigned int’ [-Werror=format=] > [18:15:49.090] 1763 | collversion = psprintf("%d.%d,%d.%d", > [18:15:49.090] | ~^ > [18:15:49.090] | | > [18:15:49.090] | int > [18:15:49.090] | %ld > [18:15:49.090] 1764 | (version.dwNLSVersion >> 8) & 0xFFFF, > [18:15:49.090] | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > [18:15:49.090] | | > [18:15:49.090] | long unsigned int > > I have no idea why we are seeing this error now when we didn't in the past - > there don't seem to have been any relevant changes? > > It does reproduce on my debian sid machine, so it's something we ought to fix, > I think? > > We did fix it in newer versions: > > Author: Peter Eisentraut <peter@eisentraut.org> > Branch: master Release: REL_16_BR [a9bc04b21] 2023-03-24 07:21:40 +0100 > > Fix incorrect format placeholders > > The fields of NLSVERSIONINFOEX are of type DWORD, which is unsigned > long, so the results of the computations being printed are also of > type unsigned long. > > Peter, any reason you didn't backpatch that? The more interesting patch is 495ed0ef2d7, which did - collversion = psprintf("%d.%d,%d.%d", + collversion = psprintf("%ld.%ld,%ld.%ld", whereas my patch just did - collversion = psprintf("%ld.%ld,%ld.%ld", + collversion = psprintf("%lu.%lu,%lu.%lu", The former change was part of a larger patch, so it was not a candidate for backpatching. As to why it's happening now, the code in question is guarded by #elif defined(WIN32) && _WIN32_WINNT >= 0x0600 so if it didn't happen before, maybe the _WIN32_WINNT value changed. -
Re: CI CompilerWarnings test fails on 15 in mingw_cross_warning
Thomas Munro <thomas.munro@gmail.com> — 2024-11-28T05:09:46Z
On Tue, Nov 19, 2024 at 12:09 AM Peter Eisentraut <peter@eisentraut.org> wrote: > The more interesting patch is 495ed0ef2d7, which did > > - collversion = psprintf("%d.%d,%d.%d", > + collversion = psprintf("%ld.%ld,%ld.%ld", This platform has sizeof(int) == sizeof(long), so it doesn't matter. > whereas my patch just did > > - collversion = psprintf("%ld.%ld,%ld.%ld", > + collversion = psprintf("%lu.%lu,%lu.%lu", I was worried the numbers might have been negative, but those expressions don't appear to allow that. Strings observed in real life are like "1539.5,1539.5" (which bears a passing resemblance to an ICU version, it has a few fairly low-valued human-originated numbers like a Unicode version swizzled around in a fairly small number of bits, but it doesn't matter too match how it works, it's enough to see that we shift down and mask). So I think it's OK to back-patch that change. See attached. As for why now, I think both fairywren and CI started warning/failing about 2 months ago[1][2], and that's when fairywren got a uname -r upgrade, so I guess msys was upgraded; as for CI I guess you'd have to check for the relevant ming-cross packages moving in Debian. Unfortunately there were a lot of random CI failures around that time and I didn't notice this particular circus at the time. [1] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=fairywren&dt=2024-10-02%2003%3A57%3A44 [2] https://github.com/postgres/postgres/runs/30890390821 -
Re: CI CompilerWarnings test fails on 15 in mingw_cross_warning
Thomas Munro <thomas.munro@gmail.com> — 2024-11-28T05:16:55Z
On Thu, Nov 28, 2024 at 6:09 PM Thomas Munro <thomas.munro@gmail.com> wrote: > As for why now, I think both fairywren and CI started warning/failing > about 2 months ago[1][2] Ohhh... it's because: commit d700e8d75bc3844d866bf15c8cadbd72d759422d Author: Andrew Dunstan <andrew@dunslane.net> Date: Mon Sep 30 11:32:32 2024 -0400 Bump MIN_WINNT for MINGW to clear a build error That was 15 only. It was in that thread too, I just forgot. Sorry, it took noticing that fairywren doesn't warn on 14 to trigger the memory :-) -
Re: CI CompilerWarnings test fails on 15 in mingw_cross_warning
Thomas Munro <thomas.munro@gmail.com> — 2024-11-28T22:40:21Z
Pushed. CI is finally returned to its full verdant splendour in the back-branches.