[tz] Dubious coding in tzcode 2019b

Tom Lane tgl at sss.pgh.pa.us
Fri Jul 19 04:29:34 UTC 2019


Some compilers in the Postgres buildfarm are unhappy[1] about this
coding in 2019b's zic.c:

	register ptrdiff_t thistimei, thistimecnt, thistimelim;
	bool locut, hicut;
	...
	thistimecnt = - locut - hicut;

I think they have a point: at best this code is seriously opaque,
and at worst it's unportable.  I can't even figure out whether
this is assuming that bool promotes to signed or unsigned int;
and even if whichever choice that is supported by the C99 spec,
what will happen in environments where "bool" does not mean the
C99 type, but rather signed or unsigned char?  (For my immediate
purposes, that includes every released branch of Postgres :-(.)

I think this would be a lot better if it were written as some
if-tests that just consider locut and hicut as boolean values,
without any assumptions about how bool converts to integer.
If we must have such assumptions, please write them as explicit
casts.

While the compilers aren't whining about it, this line a bit
further down seems like equally bad style to me:

	convert(locut + thistimecnt + hicut, tzh.tzh_timecnt);

This also seems like it's making dubious assumptions about how
signed and possibly-unsigned values will mix.

			regards, tom lane

[1] https://www.postgresql.org/message-id/20190719035347.GJ1859@paquier.xyz


More information about the tz mailing list