[PATCH] time: Remove assert in reading of tz file

[PATCH] time: Remove assert in reading of tz file

[Christopher Wong]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: 0001-time-Remove-assert-in-reading-of-tz-file.patch --]
[-- Type: text/x-patch; name="0001-time-Remove-assert-in-reading-of-tz-file.patch", Size: 1393 bytes --]

From d1f0f7a09765ce7bb80d7d6bdd00e39bf7ca5d54 Mon Sep 17 00:00:00 2001
From: Christopher Wong <christopher.wong@axis.com>
Date: Mon, 6 Dec 2021 14:48:33 +0100
Subject: [PATCH] time: Remove assert in reading of tz file
To: libc-alpha@sourceware.org

The assumption of "__tzname[0] == NULL" then there must be no transition
and "num_types == 1" is obsolete. The case when the tz file is truncated
then "__tzname[0] == NULL" happens even when there is one transition.

The "num_types == 1" is kept up to version 2021c of the tzdb. Starting
from version 2021d of the tzdb the truncation introduces "-00" time zone
abbreviations for intervals with Universal Time (UT) offsets that are
unspecified. In other words, it means "num_types == 2".
---
 time/tzfile.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/time/tzfile.c b/time/tzfile.c
index 190a777152..874e10c9c7 100644
--- a/time/tzfile.c
+++ b/time/tzfile.c
@@ -429,12 +429,7 @@ __tzfile_read (const char *file, size_t extra, char **extrap)
 	}
     }
   if (__tzname[0] == NULL)
-    {
-      /* This should only happen if there are no transition rules.
-	 In this case there should be only one single type.  */
-      assert (num_types == 1);
-      __tzname[0] = __tzstring (zone_names);
-    }
+    __tzname[0] = __tzstring (zone_names);
   if (__tzname[1] == NULL)
     __tzname[1] = __tzname[0];
 
-- 
2.20.1

Re: [PATCH] time: Remove assert in reading of tz file

[Florian Weimer]

* Christopher Wong via Libc-alpha:

> diff --git a/time/tzfile.c b/time/tzfile.c
> index 190a777152..874e10c9c7 100644
> --- a/time/tzfile.c
> +++ b/time/tzfile.c
> @@ -429,12 +429,7 @@ __tzfile_read (const char *file, size_t extra, char **extrap)
>  	}
>      }
>    if (__tzname[0] == NULL)
> -    {
> -      /* This should only happen if there are no transition rules.
> -	 In this case there should be only one single type.  */
> -      assert (num_types == 1);
> -      __tzname[0] = __tzstring (zone_names);
> -    }
> +    __tzname[0] = __tzstring (zone_names);
>    if (__tzname[1] == NULL)
>      __tzname[1] = __tzname[0];

Is this patch required to avoid an assert with 2021d and later?

Would you be able to add a test case using a precomputed zone file?

Thanks,
Florian

Re: [PATCH] time: Remove assert in reading of tz file

[Christopher Wong]

This patch is needed if the tzfile is compiled using the truncated option -r in 2021d and later.


I know how to create the zone file, but I haven't run the test case before. Would probably take me some time to put that up.


Best Regards,

Christopher Wong
________________________________
From: Florian Weimer <fweimer@redhat.com>
Sent: Monday, December 6, 2021 4:10:16 PM
To: Christopher Wong via Libc-alpha
Cc: Christopher Wong
Subject: Re: [PATCH] time: Remove assert in reading of tz file

* Christopher Wong via Libc-alpha:

> diff --git a/time/tzfile.c b/time/tzfile.c
> index 190a777152..874e10c9c7 100644
> --- a/time/tzfile.c
> +++ b/time/tzfile.c
> @@ -429,12 +429,7 @@ __tzfile_read (const char *file, size_t extra, char **extrap)
>        }
>      }
>    if (__tzname[0] == NULL)
> -    {
> -      /* This should only happen if there are no transition rules.
> -      In this case there should be only one single type.  */
> -      assert (num_types == 1);
> -      __tzname[0] = __tzstring (zone_names);
> -    }
> +    __tzname[0] = __tzstring (zone_names);
>    if (__tzname[1] == NULL)
>      __tzname[1] = __tzname[0];

Is this patch required to avoid an assert with 2021d and later?

Would you be able to add a test case using a precomputed zone file?

Thanks,
Florian

[PATCH v2] timezone: handle truncated timezones from tzcode-2021d and later (BZ #28707)

[Hans-Peter Nilsson]

I'm assisting Christopher, who sent the first version; this
one includes a test-case and a bugzilla report reference.
We're both covered by Axis' copyright assignment.  Tested on
x86_64-linux (Debian 9).

brgds, H-P
8< ------------------------------------------------------ >8

When using a timezone file generated by the zic in IANA
tzcode-2021d a.k.a. tzlib-2021d (also in tzlib-2021e; current as
of this writing), glibc asserts in __tzfile_read (on
e.g. tzset() for this file) and you may find lines matching
"tzfile.c:435: __tzfile_read: Assertion `num_types == 1' failed"
in your syslog.

Attached is a test-case including the tzfile for Asuncion
generated by tzlib-2021e as follows, using the tzlib-2021e zic:
"zic -d DEST -r @1546300800 -L /dev/null -b slim
SOURCE/southamerica".  Note that in its type 2 header, it has
two entries in its "time-types" array (types), but only one
entry in its "transition types" array (type_idxs).

This is valid and expected already in the published RFC8536, and
not even frowned upon: "Local time for timestamps before the
first transition is specified by the first time type (time type
0)" ... "every nonzero local time type index SHOULD appear at
least once in the transition type array".  Note the "nonzero ...
index".  Until the 2021d zic, index 0 has been shared by the
first valid transition but with 2021d it's separate, set apart
as a placeholder and only "implicitly" indexed.  (A draft update
of the RFC mandates that the entry at index 0 is a placeholder
in this case, hence can no longer be shared.)

	* time/tzfile.c: Don't assert when no transitions are found.
	* timezone/Makefile, timezone/tst-pr28707.c,
	timezone/testdata/XT5: New test.

Co-authored-by: Christopher Wong <Christopher.Wong@axis.com>
---
 time/tzfile.c           |   4 ++--
 timezone/Makefile       |   4 +++-
 timezone/testdata/XT5   | Bin 0 -> 156 bytes
 timezone/tst-bz28707.c |  46 ++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 51 insertions(+), 3 deletions(-)
 create mode 100644 timezone/testdata/XT5
 create mode 100644 timezone/tst-bz28707.c

diff --git a/time/tzfile.c b/time/tzfile.c
index 190a777152b3..8668392ad387 100644
--- a/time/tzfile.c
+++ b/time/tzfile.c
@@ -431,8 +431,8 @@ __tzfile_read (const char *file, size_t extra, char **extrap)
   if (__tzname[0] == NULL)
     {
       /* This should only happen if there are no transition rules.
-	 In this case there should be only one single type.  */
-      assert (num_types == 1);
+	 In this case there's usually only one single type, unless
+	 e.g. the data file has a truncated time-range.  */
       __tzname[0] = __tzstring (zone_names);
     }
   if (__tzname[1] == NULL)
diff --git a/timezone/Makefile b/timezone/Makefile
index c624a189b322..369b3e1698ba 100644
--- a/timezone/Makefile
+++ b/timezone/Makefile
@@ -23,7 +23,7 @@ subdir	:= timezone
 include ../Makeconfig
 
 others	:= zdump zic
-tests	:= test-tz tst-timezone tst-tzset
+tests	:= test-tz tst-timezone tst-tzset tst-bz28707
 
 generated-dirs += testdata
 
@@ -85,10 +85,12 @@ $(objpfx)tst-timezone.out: $(addprefix $(testdata)/, \
 				       America/Sao_Paulo Asia/Tokyo \
 				       Europe/London)
 $(objpfx)tst-tzset.out: $(addprefix $(testdata)/XT, 1 2 3 4)
+$(objpfx)tst-bz28707.out: $(testdata)/XT5
 
 test-tz-ENV = TZDIR=$(testdata)
 tst-timezone-ENV = TZDIR=$(testdata)
 tst-tzset-ENV = TZDIR=$(testdata)
+tst-bz28707-ENV = TZDIR=$(testdata)
 
 # Note this must come second in the deps list for $(built-program-cmd) to work.
 zic-deps = $(objpfx)zic $(leapseconds) yearistype
diff --git a/timezone/testdata/XT5 b/timezone/testdata/XT5
new file mode 100644
index 0000000000000000000000000000000000000000..aadcd6dccab37d1177d2c0ca725ea56c5dc7ca48
GIT binary patch
literal 156
zcmWHE%1kq2AP5+NDnJ+nLI`UCDP;m;4v_j7t+fqMz5oATy}-z#Yhb{jYhcX4Wut3g
cVrK#*jqP-N4Gr`R^$he4bbO8VOh61S07fYg0{{R3

literal 0
HcmV?d00001

diff --git a/timezone/tst-bz28707.c b/timezone/tst-bz28707.c
new file mode 100644
index 000000000000..0a9df1e9a094
--- /dev/null
+++ b/timezone/tst-bz28707.c
@@ -0,0 +1,46 @@
+/* Copyright (C) 2021 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <time.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+/* Test that we can use a truncated timezone-file, where the time-type
+   at index 0 is not indexed by the transition-types array (and the
+   transition-types array does not contain at least both one DST and one
+   normal time members).  */
+
+static int
+do_test (void)
+{
+  if (setenv ("TZ", "XT5", 1))
+    {
+      puts ("setenv failed.");
+      return 1;
+    }
+
+  tzset ();
+
+  return
+    /* Sanity-check that we got the right timezone-name for DST.  For
+       normal time, we're likely to get "-00" (the "unspecified" marker),
+       even though the POSIX timezone string says "-04".  Let's not test
+       that.  */
+    !(strcmp (tzname[1], "-03") == 0);
+}
+#include <support/test-driver.c>
-- 
2.11.0

Re: [PATCH v2] timezone: handle truncated timezones from tzcode-2021d and later (BZ #28707)

[Paul Eggert]

Thanks for the patch. The basic idea looks right, but:

On 12/16/21 16:57, Hans-Peter Nilsson wrote:
> diff --git a/timezone/testdata/XT5 b/timezone/testdata/XT5
> new file mode 100644
> index 0000000000000000000000000000000000000000..aadcd6dccab37d1177d2c0ca725ea56c5dc7ca48
> GIT binary patch
> literal 156
> zcmWHE%1kq2AP5+NDnJ+nLI`UCDP;m;4v_j7t+fqMz5oATy}-z#Yhb{jYhcX4Wut3g
> cVrK#*jqP-N4Gr`R^$he4bbO8VOh61S07fYg0{{R3
> 
> literal 0

Instead of putting that binary file directly in the repository it might 
be better to use the 'zic' command that you mentioned, to create the 
binary file from text source, as binary files in the repository are a 
pain for the usual reasons.

If it's too much trouble to invoke zic, then a shell 'printf' like the 
following should be OK.

# Put a comment here explaining how this file was derived.
printf >XT5 \
'TZif2\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0'\
'\0\0\0\0\0\0\0\1\0\0\0\1\0\0\0\0\0\0\0TZif2\0\0\0\0\0\0\0\0'\
'\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\1\0\0\0\2\0\0\0\b\0'\
'\0\0\0\*\255\200\1\0\0\0\0\0\0\377\377\325\320\1\4-00\0-03\0\n'\
'<-04>4<-03>,M10.1.0/0,M3.4.0/0\n'

Re: [PATCH v2] timezone: handle truncated timezones from tzcode-2021d and later (BZ #28707)

[Carlos O'Donell]

On 12/16/21 21:51, Paul Eggert wrote:
> Thanks for the patch. The basic idea looks right, but:
> 
> On 12/16/21 16:57, Hans-Peter Nilsson wrote:
>> diff --git a/timezone/testdata/XT5 b/timezone/testdata/XT5
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..aadcd6dccab37d1177d2c0ca725ea56c5dc7ca48
>> GIT binary patch
>> literal 156
>> zcmWHE%1kq2AP5+NDnJ+nLI`UCDP;m;4v_j7t+fqMz5oATy}-z#Yhb{jYhcX4Wut3g
>> cVrK#*jqP-N4Gr`R^$he4bbO8VOh61S07fYg0{{R3
>>
>> literal 0
> 
> Instead of putting that binary file directly in the repository it might be better to use the 'zic' command that you mentioned, to create the binary file from text source, as binary files in the repository are a pain for the usual reasons.
> 
> If it's too much trouble to invoke zic, then a shell 'printf' like the following should be OK.
> 
> # Put a comment here explaining how this file was derived.
> printf >XT5 \
> 'TZif2\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0'\
> '\0\0\0\0\0\0\0\1\0\0\0\1\0\0\0\0\0\0\0TZif2\0\0\0\0\0\0\0\0'\
> '\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\1\0\0\0\2\0\0\0\b\0'\
> '\0\0\0\*\255\200\1\0\0\0\0\0\0\377\377\325\320\1\4-00\0-03\0\n'\
> '<-04>4<-03>,M10.1.0/0,M3.4.0/0\n'
 
DJ,

Could you please review the interaction between CI and this patch?

32-bit i686 CI/CD fell over because of the binary patch:
https://patchwork.sourceware.org/project/glibc/patch/20211217005744.E099F203C8@pchp3.se.axis.com/

It should be entirely possible to apply this, and it does
apply with git-pw patch apply.

-- 
Cheers,
Carlos.

Re: [PATCH v2] timezone: handle truncated timezones from tzcode-2021d and later (BZ #28707)

[Hans-Peter Nilsson]

> From: Carlos O'Donell <carlos@redhat.com>
> Date: Fri, 17 Dec 2021 15:05:57 +0100

> On 12/16/21 21:51, Paul Eggert wrote:
> > Thanks for the patch. The basic idea looks right, but:
> > 
> > On 12/16/21 16:57, Hans-Peter Nilsson wrote:
> >> diff --git a/timezone/testdata/XT5 b/timezone/testdata/XT5
> >> new file mode 100644
> >> index 0000000000000000000000000000000000000000..aadcd6dccab37d1177d2c0ca725ea56c5dc7ca48
> >> GIT binary patch
> >> literal 156
> >> zcmWHE%1kq2AP5+NDnJ+nLI`UCDP;m;4v_j7t+fqMz5oATy}-z#Yhb{jYhcX4Wut3g
> >> cVrK#*jqP-N4Gr`R^$he4bbO8VOh61S07fYg0{{R3
> >>
> >> literal 0
> > 
> > Instead of putting that binary file directly in the
> > repository it might be better to use the 'zic' command
> > that you mentioned, to create the binary file from text
> > source, as binary files in the repository are a pain for
> > the usual reasons.

So, here I was thinking:
"That kind of thinking was appropriate for CVS, but git
(should) handle the files just fine.  Also, see the existing
XT1..XT4!"

> > If it's too much trouble to invoke zic,

...and exactly version 2021d or 2021e: yes, I think it is.

> > then a shell 'printf' like the following should be OK.

Nice, thanks.

> DJ,
> 
> Could you please review the interaction between CI and this patch?
> 
> 32-bit i686 CI/CD fell over because of the binary patch:
> https://patchwork.sourceware.org/project/glibc/patch/20211217005744.E099F203C8@pchp3.se.axis.com/

Hah!  I stand corrected; QED binary files (in patches) are
*still* causing problems!

New version coming up.

brgds, H-P

Re: [PATCH v2] timezone: handle truncated timezones from tzcode-2021d and later (BZ #28707)

[Carlos O'Donell]

On 12/16/21 21:51, Paul Eggert wrote:
> Thanks for the patch. The basic idea looks right, but:

Paul,

What is the impact of this for all of the downstream distributions?

I would like to avoid having every distribution scrambling to patch
glibc to handle new tdata binary files, but if we need to do this
patch I would like to get it into all of the active release branches.

-- 
Cheers,
Carlos.

Re: [PATCH v2] timezone: handle truncated timezones from tzcode-2021d and later (BZ #28707)

[Paul Eggert]

On 12/17/21 06:24, Carlos O'Donell wrote:
> What is the impact of this for all of the downstream distributions?

For tzdb Zones, this should affect only people who use zic's -r option 
to limit the size of the TZif binary files by supporting only a 
particular time range. For example, 'zic -r@1609459200 ...' means "omit 
all data for timestamps before the year 2021 UTC". Although full distros 
don't do that, I suppose some stripped-down installions might, for 
embedded applications that don't deal with timestamps in the past. zic's 
-r option was introduced in tzdb 2019a (2019-03-25), was propagated into 
glibc 2.32 (2020-08-05), and is the basis for BZ#28707.

If people are using glibc zic, they need to also specify '-b slim' to 
get the slimmed-down TZif files that have the problem, as in the 
original BZ#28707 bug report. However, I expect that people who use -r 
to generate small TZif files will also either use '-b slim', or will use 
upstream zic where '-b slim' is already the default (this is as of tzdb 
2020b).

> I would like to avoid having every distribution scrambling to patch
> glibc to handle new tdata binary files, but if we need to do this
> patch I would like to get it into all of the active release branches.

That depends on how many people trim down TZif files in the 
abovementioned ways. It shouldn't be a problem on mainstream distros. It 
might be a problem if people use these newer RFC 8536-inspired zic 
options for installations on embedded devices or whatever.

Re: [PATCH v2] timezone: handle truncated timezones from tzcode-2021d and later (BZ #28707)

[Hans-Peter Nilsson]

> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Fri, 17 Dec 2021 18:57:52 +0100

> > I would like to avoid having every distribution scrambling to patch
> > glibc to handle new tdata binary files, but if we need to do this
> > patch I would like to get it into all of the active release branches.
> 
> That depends on how many people trim down TZif files in the 
> abovementioned ways. It shouldn't be a problem on mainstream distros. It 
> might be a problem if people use these newer RFC 8536-inspired zic 
> options for installations on embedded devices or whatever.

Just adding that this is exactly the case here.

For some reason the zic from tzcode is preferred over
glibc's, and usually the tzdata and tzcode (run only at
build time) is updated together.  (Not our own invention or
mis/conception, but using a truncated time-range apparently
is, which is the reason we're lucky #1.)

brgds, H-P

Re: [PATCH v2] timezone: handle truncated timezones from tzcode-2021d and later (BZ #28707)

[Paul Eggert]

On 12/17/21 10:11, Hans-Peter Nilsson wrote:

> For some reason the zic from tzcode is preferred over
> glibc's

If you're using zic's -r and/or -bslim options, it is indeed better to 
use zic 2021d or later, as recent tzcode fixes several bugs relating to 
-r and -bslim.

This raises the issue of when it'd be a good time to sync glibc's copy 
of tzcode source files from upstream. I don't know when (and can't 
guarantee that) Internet RFC 8536's draft successor will be published as 
an RFC. That being said, I doubt whether substantial technical changes 
will be made to the draft between now and then. So partly it boils down 
to whether glibc wants to be a bit ahead of TZif standardization, or a 
bit behind it.

A significant portability implication of syncing from tzcode is that 
this would make '-b slim' the default for glibc zic. This change should 
work OK with existing glibc (so long as you don't also use zic -r) but 
it may affect other programs that read TZif files directly, and that 
don't yet support RFC 8536.

[PATCH v3 0/2] timezone: BZ #28707

[Hans-Peter Nilsson]

I'm splitting up the patch into respectively the fix itself
and the test-case, as it seemed the methods in the test-case
were subject to discussion and tools indigestion, despite
following what appeared to be existing practice.  Hopefully
this will speed up getting the fix itself accepted and
committed, as that step is gating downstream acceptance.
Also fixing an error in the ChangeLog entry, FWIW.

brgds, H-P

Re: [PATCH v3 0/2] timezone: BZ #28707

[Paul Eggert]

Thanks, I reviewed this set of patches by hand, and they look good to me.

Re: [PATCH v3 0/2] timezone: BZ #28707

[Hans-Peter Nilsson]

> From: Hans-Peter Nilsson <hp@axis.com>
> Date: Fri, 17 Dec 2021 21:36:55 +0100

> I'm splitting up the patch into respectively the fix itself
> and the test-case, as it seemed the methods in the test-case
> were subject to discussion and tools indigestion, despite
> following what appeared to be existing practice.  Hopefully
> this will speed up getting the fix itself accepted and
> committed, as that step is gating downstream acceptance.
> Also fixing an error in the ChangeLog entry, FWIW.

Ping.

This set was ok'd by Paul Eggert in
https://sourceware.org/pipermail/libc-alpha/2021-December/134379.html

brgds, H-P

Re: [PATCH v3 0/2] timezone: BZ #28707

[Adhemerval Zanella]

On 28/12/2021 21:50, Hans-Peter Nilsson via Libc-alpha wrote:
>> From: Hans-Peter Nilsson <hp@axis.com>
>> Date: Fri, 17 Dec 2021 21:36:55 +0100
> 
>> I'm splitting up the patch into respectively the fix itself
>> and the test-case, as it seemed the methods in the test-case
>> were subject to discussion and tools indigestion, despite
>> following what appeared to be existing practice.  Hopefully
>> this will speed up getting the fix itself accepted and
>> committed, as that step is gating downstream acceptance.
>> Also fixing an error in the ChangeLog entry, FWIW.
> 
> Ping.
> 
> This set was ok'd by Paul Eggert in
> https://sourceware.org/pipermail/libc-alpha/2021-December/134379.html
> 
> brgds, H-P

I will take care of it.

[PATCH v3 1/2] timezone: handle truncated timezones from tzcode-2021d and later (BZ #28707)

[Hans-Peter Nilsson]

When using a timezone file with a truncated starting time,
generated by the zic in IANA tzcode-2021d a.k.a. tzlib-2021d
(also in tzlib-2021e; current as of this writing), glibc
asserts in __tzfile_read (on e.g. tzset() for this file) and
you may find lines matching "tzfile.c:435: __tzfile_read:
Assertion `num_types == 1' failed" in your syslog.

One example of such a file is the tzfile for Asuncion
generated by tzlib-2021e as follows, using the tzlib-2021e zic:
"zic -d DEST -r @1546300800 -L /dev/null -b slim
SOURCE/southamerica".  Note that in its type 2 header, it has
two entries in its "time-types" array (types), but only one
entry in its "transition types" array (type_idxs).

This is valid and expected already in the published RFC8536, and
not even frowned upon: "Local time for timestamps before the
first transition is specified by the first time type (time type
0)" ... "every nonzero local time type index SHOULD appear at
least once in the transition type array".  Note the "nonzero ...
index".  Until the 2021d zic, index 0 has been shared by the
first valid transition but with 2021d it's separate, set apart
as a placeholder and only "implicitly" indexed.  (A draft update
of the RFC mandates that the entry at index 0 is a placeholder
in this case, hence can no longer be shared.)

	* time/tzfile.c (__tzfile_read): Don't assert when no transitions
	are found.

Co-authored-by: Christopher Wong <Christopher.Wong@axis.com>
---
 time/tzfile.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/time/tzfile.c b/time/tzfile.c
index 190a777152b3..8668392ad387 100644
--- a/time/tzfile.c
+++ b/time/tzfile.c
@@ -431,8 +431,8 @@ __tzfile_read (const char *file, size_t extra, char **extrap)
   if (__tzname[0] == NULL)
     {
       /* This should only happen if there are no transition rules.
-	 In this case there should be only one single type.  */
-      assert (num_types == 1);
+	 In this case there's usually only one single type, unless
+	 e.g. the data file has a truncated time-range.  */
       __tzname[0] = __tzstring (zone_names);
     }
   if (__tzname[1] == NULL)
-- 
2.11.0

[PATCH v3 2/2] timezone: test-case for BZ #28707

[Hans-Peter Nilsson]

Here's an alternative for the test-case, trivially
generating the binary tzfile from a source.  At first I
though re-sending just the first version but split up as the
first alternative, but on second thought that would just be
undesired and confusing.

Generating the file using the specific zic version *for the
purpose of covering the fixed bug* would add a dependency to
install exactly 2021d or 2021e only to be used by this test,
and I don't think that's appropriate.  Better use a
shell-script and printf as suggested.  Hopefully digestible
for both maintainers and CI tools.

Happy Holidays in advance.

8< ------------------------------------------------------ >8

This test-case is the tzfile for Asuncion generated by
tzlib-2021e as follows, using the tzlib-2021e zic: "zic -d
DEST -r @1546300800 -L /dev/null -b slim
SOURCE/southamerica".  Note that in its type 2 header, it
has two entries in its "time-types" array (types), but only
one entry in its "transition types" array (type_idxs).

	* timezone/Makefile, timezone/tst-pr28707.c,
	timezone/testdata/gen-XT5.sh: New test.

Change-Id: I75122962b8e682b00c8f7241cb5bda56adb3d3a3
Co-authored-by: Christopher Wong <Christopher.Wong@axis.com>
---
 timezone/Makefile            |  8 +++++++-
 timezone/testdata/gen-XT5.sh | 16 +++++++++++++++
 timezone/tst-bz28707.c       | 46 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 69 insertions(+), 1 deletion(-)
 create mode 100755 timezone/testdata/gen-XT5.sh
 create mode 100644 timezone/tst-bz28707.c

diff --git a/timezone/Makefile b/timezone/Makefile
index c624a189b322..f091663b8bbb 100644
--- a/timezone/Makefile
+++ b/timezone/Makefile
@@ -23,7 +23,7 @@ subdir	:= timezone
 include ../Makeconfig
 
 others	:= zdump zic
-tests	:= test-tz tst-timezone tst-tzset
+tests	:= test-tz tst-timezone tst-tzset tst-bz28707
 
 generated-dirs += testdata
 
@@ -85,10 +85,12 @@ $(objpfx)tst-timezone.out: $(addprefix $(testdata)/, \
 				       America/Sao_Paulo Asia/Tokyo \
 				       Europe/London)
 $(objpfx)tst-tzset.out: $(addprefix $(testdata)/XT, 1 2 3 4)
+$(objpfx)tst-bz28707.out: $(testdata)/XT5
 
 test-tz-ENV = TZDIR=$(testdata)
 tst-timezone-ENV = TZDIR=$(testdata)
 tst-tzset-ENV = TZDIR=$(testdata)
+tst-bz28707-ENV = TZDIR=$(testdata)
 
 # Note this must come second in the deps list for $(built-program-cmd) to work.
 zic-deps = $(objpfx)zic $(leapseconds) yearistype
@@ -122,6 +124,10 @@ $(testdata)/XT%: testdata/XT%
 	$(make-target-directory)
 	cp $< $@
 
+$(testdata)/XT%: testdata/gen-XT%.sh
+	$(SHELL) $< > $@.tmp
+	mv $@.tmp $@
+
 $(objpfx)tzselect: tzselect.ksh $(common-objpfx)config.make
 	sed -e 's|TZDIR=[^}]*|TZDIR=$(zonedir)|' \
 	    -e '/TZVERSION=/s|see_Makefile|"$(version)"|' \
diff --git a/timezone/testdata/gen-XT5.sh b/timezone/testdata/gen-XT5.sh
new file mode 100755
index 000000000000..3cea0569eb5a
--- /dev/null
+++ b/timezone/testdata/gen-XT5.sh
@@ -0,0 +1,16 @@
+#! /bin/sh
+
+# This test-case is the tzfile for America/Asuncion
+# generated by tzlib-2021e as follows, using the tzlib-2021e
+# zic: "zic -d DEST -r @1546300800 -L /dev/null -b slim
+# SOURCE/southamerica".  Note that in its type 2 header, it
+# has two entries in its "time-types" array (types), but
+# only one entry in its "transition types" array
+# (type_idxs).
+
+printf \
+'TZif2\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0'\
+'\0\0\0\0\0\0\0\1\0\0\0\1\0\0\0\0\0\0\0TZif2\0\0\0\0\0\0\0\0'\
+'\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\1\0\0\0\2\0\0\0\b\0'\
+'\0\0\0\*\255\200\1\0\0\0\0\0\0\377\377\325\320\1\4-00\0-03\0\n'\
+'<-04>4<-03>,M10.1.0/0,M3.4.0/0\n'
diff --git a/timezone/tst-bz28707.c b/timezone/tst-bz28707.c
new file mode 100644
index 000000000000..0a9df1e9a094
--- /dev/null
+++ b/timezone/tst-bz28707.c
@@ -0,0 +1,46 @@
+/* Copyright (C) 2021 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <time.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+/* Test that we can use a truncated timezone-file, where the time-type
+   at index 0 is not indexed by the transition-types array (and the
+   transition-types array does not contain at least both one DST and one
+   normal time members).  */
+
+static int
+do_test (void)
+{
+  if (setenv ("TZ", "XT5", 1))
+    {
+      puts ("setenv failed.");
+      return 1;
+    }
+
+  tzset ();
+
+  return
+    /* Sanity-check that we got the right timezone-name for DST.  For
+       normal time, we're likely to get "-00" (the "unspecified" marker),
+       even though the POSIX timezone string says "-04".  Let's not test
+       that.  */
+    !(strcmp (tzname[1], "-03") == 0);
+}
+#include <support/test-driver.c>
-- 
2.11.0

Re: [PATCH v3 2/2] timezone: test-case for BZ #28707

[Hans-Peter Nilsson]

> From: Hans-Peter Nilsson <hp@axis.com>
> Date: Fri, 17 Dec 2021 21:45:54 +0100

> Change-Id: I75122962b8e682b00c8f7241cb5bda56adb3d3a3

Oops, sorry about that line.  Mishap from internal
git-hooks...  Please delete this when applying!
(Or do I need to send a new version?)

brgds, H-P