Thread: Re: pgsql: Sync our copy of the timezone library with IANA releasetzcode20

Hi Tom,
(moving to -hackers)

On Wed, Jul 17, 2019 at 10:26:45PM +0000, Tom Lane wrote:
> Sync our copy of the timezone library with IANA release tzcode2019b.
>
> A large fraction of this diff is just due to upstream's somewhat
> random decision to rename a bunch of internal variables and struct
> fields.  However, there is an interesting new feature in zic:
> it's grown a "-b slim" option that emits zone files without 32-bit
> data and other backwards-compatibility hacks.  We should consider
> whether we wish to enable that.

This is causing a compilation warning on Windows:
https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=whelk&dt=2019-07-19%2001%3A41%3A13&stg=make

"C:\buildfarm\buildenv\HEAD\pgsql.build\pgsql.sln" (Standardziel) (1)
->
"C:\buildfarm\buildenv\HEAD\pgsql.build\zic.vcxproj" (Standardziel)
(72) ->
 src/timezone/zic.c(2401): warning C4804: '-' : unsafe use of type
  'bool' in operation
[C:\buildfarm\buildenv\HEAD\pgsql.build\zic.vcxproj]

Buildfarm members using VS like whelk complains about that, and I can
see the warning myself.
--
Michael

Michael Paquier <michael@paquier.xyz> writes:
> This is causing a compilation warning on Windows:

>  src/timezone/zic.c(2401): warning C4804: '-' : unsafe use of type
>   'bool' in operation

Hmmm ... the code looks like

        bool        locut,
                    hicut;
        ...
        thistimecnt = -locut - hicut;

so I think your compiler has a point.  I shall complain to upstream.
At best, it's really unobvious what this code is meant to do, and
at worst (eg, depending on whether bool promotes to signed or unsigned
int) the results are unportable.

            regards, tom lane

I wrote:
> Michael Paquier <michael@paquier.xyz> writes:
>> This is causing a compilation warning on Windows:
> ...so I think your compiler has a point.  I shall complain to upstream.

The IANA folk want to fix it like this:

diff --git a/zic.c b/zic.c
index 8bf5628..a84703a 100644
--- a/zic.c
+++ b/zic.c
@@ -2145,7 +2145,7 @@ writezone(const char *const name, const char *const string, char version,
         }
         if (pass == 1 && !want_bloat()) {
           utcnt = stdcnt = thisleapcnt = 0;
-          thistimecnt = - locut - hicut;
+          thistimecnt = - (locut + hicut);
           thistypecnt = thischarcnt = 1;
           thistimelim = thistimei;
         }

I'm not quite convinced whether that will silence the warning, but
at least it's a bit less unreadable.

            regards, tom lane

On Fri, Jul 19, 2019 at 02:56:34PM -0400, Tom Lane wrote:
> I'm not quite convinced whether that will silence the warning, but
> at least it's a bit less unreadable.

Thanks for working with upstream on this.  From what I can see,
woodlouse & friends do not complain anymore.
--
Michael