Horton

Thread

Commits

  1. Update timezone to C99

  2. Remove obsolete cast

  1. Update timezone to C99

    Peter Eisentraut <peter@eisentraut.org> — 2025-11-12T16:18:45Z

    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.

  2. Re: Update timezone to C99

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-11-12T18:02:03Z

    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

  3. Re: Update timezone to C99

    Peter Eisentraut <peter@eisentraut.org> — 2025-11-14T08:11:48Z

    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.

  4. Re: Update timezone to C99

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-11-14T21:11:21Z

    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

  5. Re: Update timezone to C99

    Peter Eisentraut <peter@eisentraut.org> — 2025-11-19T09:31:10Z

    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.

  6. Re: Update timezone to C99

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-11-19T15:48:06Z

    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

  7. Re: Update timezone to C99

    Peter Eisentraut <peter@eisentraut.org> — 2025-11-21T13:20:59Z

    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.