Update timezone to C99

Started by Peter Eisentraut5 months ago7 messageshackers
Jump to latest
#1Peter Eisentraut
peter_e@gmx.net

The code in src/timezone/ still contained workarounds for not wanting to
rely on C99 <stdint.h> and <inttypes.h>, even though the rest of the
code uses those now.

To fix that, I re-downloaded the upstream code (same version as before),
applied the fixes described in src/timezone/README, except those related
to the integer types, and then put back the PostgreSQL-specific code and
massaged things to remove irrelevant differences. There were a few
other minor and cosmetic changes that I left in to improve alignment
with upstream.

Patch attached. Seems to work.

Attachments:

0001-Update-timezone-to-C99.patchtext/plain; charset=UTF-8; name=0001-Update-timezone-to-C99.patchDownload+138-164
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#1)
Re: Update timezone to C99

Peter Eisentraut <peter@eisentraut.org> writes:

To fix that, I re-downloaded the upstream code (same version as before),
applied the fixes described in src/timezone/README, except those related
to the integer types, and then put back the PostgreSQL-specific code and
massaged things to remove irrelevant differences. There were a few
other minor and cosmetic changes that I left in to improve alignment
with upstream.

Hm, I've had "re-sync TZ code with upstream" on my TODO list for
several years now. I believe there's been quite a bit of churn
upstream since tzcode2020d, some of it oriented towards this same
issue of code modernization. Maybe we should try to sync with
a newer release while we're at it.

regards, tom lane

#3Peter Eisentraut
peter_e@gmx.net
In reply to: Tom Lane (#2)
Re: Update timezone to C99

On 12.11.25 19:02, Tom Lane wrote:

Peter Eisentraut <peter@eisentraut.org> writes:

To fix that, I re-downloaded the upstream code (same version as before),
applied the fixes described in src/timezone/README, except those related
to the integer types, and then put back the PostgreSQL-specific code and
massaged things to remove irrelevant differences. There were a few
other minor and cosmetic changes that I left in to improve alignment
with upstream.

Hm, I've had "re-sync TZ code with upstream" on my TODO list for
several years now. I believe there's been quite a bit of churn
upstream since tzcode2020d, some of it oriented towards this same
issue of code modernization. Maybe we should try to sync with
a newer release while we're at it.

My idea was to do this C99 adjustment first so that the differences to
upstream are reduced, which would hopefully simplify updating to that
newer code.

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#3)
Re: Update timezone to C99

Peter Eisentraut <peter@eisentraut.org> writes:

On 12.11.25 19:02, Tom Lane wrote:

Hm, I've had "re-sync TZ code with upstream" on my TODO list for
several years now. I believe there's been quite a bit of churn
upstream since tzcode2020d, some of it oriented towards this same
issue of code modernization. Maybe we should try to sync with
a newer release while we're at it.

My idea was to do this C99 adjustment first so that the differences to
upstream are reduced, which would hopefully simplify updating to that
newer code.

Fair enough. I looked through the patch briefly and had a couple of
minor quibbles:

@@ -905,7 +906,7 @@ transtime(const int year, const struct rule *const rulep,
 			for (i = 1; i < rulep->r_week; ++i)
 			{
 				if (d + DAYSPERWEEK >=
-					mon_lengths[(int) leapyear][rulep->r_mon - 1])
+					mon_lengths[leapyear][rulep->r_mon - 1])
 					break;
 				d += DAYSPERWEEK;
 			}
@@ -915,7 +916,7 @@ transtime(const int year, const struct rule *const rulep,
 			 */
 			value = d * SECSPERDAY;
 			for (i = 0; i < rulep->r_mon - 1; ++i)
-				value += mon_lengths[(int) leapyear][i] * SECSPERDAY;
+				value += mon_lengths[leapyear][i] * SECSPERDAY;
 			break;
 	}

"leapyear" is bool, and I believe these casts-to-int were put in to
suppress compiler bleats about up-casting that to int. This probably
dates from when we equated bool to char, and maybe it's moot now,
but I'm not sure.

@@ -47,12 +49,15 @@ typedef int64 zic_t;
static ptrdiff_t const PTRDIFF_MAX = MAXVAL(ptrdiff_t, TYPE_BIT(ptrdiff_t));
#endif

-/*
- * The type for line numbers.  In Postgres, use %d to format them; upstream
- * uses PRIdMAX but we prefer not to rely on that, not least because it
- * results in platform-dependent strings to be translated.
- */
-typedef int lineno_t;
+/* The minimum alignment of a type, for pre-C11 platforms.  */
+#if __STDC_VERSION__ < 201112
+#define _Alignof(type) offsetof(struct { char a; type b; }, b)
+#endif

Since we've dropped pre-C11 support, I wonder why we'd include this
upstream workaround for that.

regards, tom lane

#5Peter Eisentraut
peter_e@gmx.net
In reply to: Tom Lane (#4)
Re: Update timezone to C99

On 14.11.25 22:11, Tom Lane wrote:

Peter Eisentraut <peter@eisentraut.org> writes:

On 12.11.25 19:02, Tom Lane wrote:

Hm, I've had "re-sync TZ code with upstream" on my TODO list for
several years now. I believe there's been quite a bit of churn
upstream since tzcode2020d, some of it oriented towards this same
issue of code modernization. Maybe we should try to sync with
a newer release while we're at it.

My idea was to do this C99 adjustment first so that the differences to
upstream are reduced, which would hopefully simplify updating to that
newer code.

Fair enough. I looked through the patch briefly and had a couple of
minor quibbles:

@@ -905,7 +906,7 @@ transtime(const int year, const struct rule *const rulep,
for (i = 1; i < rulep->r_week; ++i)
{
if (d + DAYSPERWEEK >=
-					mon_lengths[(int) leapyear][rulep->r_mon - 1])
+					mon_lengths[leapyear][rulep->r_mon - 1])
break;
d += DAYSPERWEEK;
}
@@ -915,7 +916,7 @@ transtime(const int year, const struct rule *const rulep,
*/
value = d * SECSPERDAY;
for (i = 0; i < rulep->r_mon - 1; ++i)
-				value += mon_lengths[(int) leapyear][i] * SECSPERDAY;
+				value += mon_lengths[leapyear][i] * SECSPERDAY;
break;
}

"leapyear" is bool, and I believe these casts-to-int were put in to
suppress compiler bleats about up-casting that to int. This probably
dates from when we equated bool to char, and maybe it's moot now,
but I'm not sure.

Correct, with bool as char this would have fallen afoul of
-Wchar-subscripts, which is included in -Wall.

I could remove these casts as a separate patch so that the reason can be
documented more clearly.

@@ -47,12 +49,15 @@ typedef int64 zic_t;
static ptrdiff_t const PTRDIFF_MAX = MAXVAL(ptrdiff_t, TYPE_BIT(ptrdiff_t));
#endif

-/*
- * The type for line numbers.  In Postgres, use %d to format them; upstream
- * uses PRIdMAX but we prefer not to rely on that, not least because it
- * results in platform-dependent strings to be translated.
- */
-typedef int lineno_t;
+/* The minimum alignment of a type, for pre-C11 platforms.  */
+#if __STDC_VERSION__ < 201112
+#define _Alignof(type) offsetof(struct { char a; type b; }, b)
+#endif

Since we've dropped pre-C11 support, I wonder why we'd include this
upstream workaround for that.

I figured it would be better to reduce divergences from upstream, even
if we don't need all the code.

#6Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#5)
Re: Update timezone to C99

Peter Eisentraut <peter@eisentraut.org> writes:

On 14.11.25 22:11, Tom Lane wrote:

"leapyear" is bool, and I believe these casts-to-int were put in to
suppress compiler bleats about up-casting that to int. This probably
dates from when we equated bool to char, and maybe it's moot now,
but I'm not sure.

Correct, with bool as char this would have fallen afoul of
-Wchar-subscripts, which is included in -Wall.
I could remove these casts as a separate patch so that the reason can be
documented more clearly.

Either that or just explain it in a para of the commit message.

Since we've dropped pre-C11 support, I wonder why we'd include this
upstream workaround for that.

I figured it would be better to reduce divergences from upstream, even
if we don't need all the code.

[ shrug... ] Okay.

No further comments.

regards, tom lane

#7Peter Eisentraut
peter_e@gmx.net
In reply to: Tom Lane (#6)
Re: Update timezone to C99

On 19.11.25 16:48, Tom Lane wrote:

No further comments.

This has been committed.

FWIW, I don't have any plans currently to work on upgrading the code to
newer upstream versions.