[v2] timezone: Enhance tst-bz28707 diagnostics

Message ID 87tt74dx33.fsf@oldenburg.str.redhat.com (mailing list archive)
State Committed
Headers
Series [v2] timezone: Enhance tst-bz28707 diagnostics |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_glibc_build--master-arm success Build passed
redhat-pt-bot/TryBot-32bit success Build for i686
linaro-tcwg-bot/tcwg_glibc_check--master-arm success Test passed
redhat-pt-bot/TryBot-still_applies warning Patch no longer applies to master

Commit Message

Florian Weimer April 4, 2025, 10:45 a.m. UTC
  This hopefully provides additional information about why the
test failed, in case the fix in commit 62db87ab24f9ca483f97f
("timezone: Fix tst-bz28707 Makefile rule") turns out to be
insufficient.

---
v2: Fix subject/commit message (sorry).
 timezone/tst-bz28707.c | 29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)


base-commit: 95b780c1d0549678c0a244c6e2112ec97edf0839
  

Comments

Adhemerval Zanella Netto April 8, 2025, 6:16 p.m. UTC | #1
On 04/04/25 07:45, Florian Weimer wrote:
> This hopefully provides additional information about why the
> test failed, in case the fix in commit 62db87ab24f9ca483f97f
> ("timezone: Fix tst-bz28707 Makefile rule") turns out to be
> insufficient.
> 

LGTM, just a question below.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> ---
> v2: Fix subject/commit message (sorry).
>  timezone/tst-bz28707.c | 29 ++++++++++++++++-------------
>  1 file changed, 16 insertions(+), 13 deletions(-)
> 
> diff --git a/timezone/tst-bz28707.c b/timezone/tst-bz28707.c
> index a5723f4b8a..bb81ad1bd6 100644
> --- a/timezone/tst-bz28707.c
> +++ b/timezone/tst-bz28707.c
> @@ -15,10 +15,11 @@
>     License along with the GNU C Library; if not, see
>     <https://www.gnu.org/licenses/>.  */
>  
> -#include <time.h>
> +#include <errno.h>
>  #include <stdio.h>
>  #include <stdlib.h>
> -#include <string.h>
> +#include <support/check.h>
> +#include <time.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
> @@ -28,19 +29,21 @@
>  static int
>  do_test (void)
>  {
> -  if (setenv ("TZ", "XT5", 1))
> -    {
> -      puts ("setenv failed.");
> -      return 1;
> -    }
> +  if (setenv ("TZ", "XT5", 1) != 0)
> +    FAIL_EXIT1 ("setenv: %m");
>  
> +  errno = 0;
>    tzset ();
> +  if (errno != 0)
> +    printf ("warning: tzset set errno to %d (%m)", errno);

Shouldn't it call support_record_failure() in this case?

>  
> -  return
> -    /* Sanity-check that we got the right abbreviation 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);
> +  /* Sanity-check that we got the right abbreviation 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.  */
> +  TEST_COMPARE_STRING (tzname[1], "-03");
> +
> +  return 0;
>  }
> +
>  #include <support/test-driver.c>
> 
> base-commit: 95b780c1d0549678c0a244c6e2112ec97edf0839
>
  
Florian Weimer April 8, 2025, 6:38 p.m. UTC | #2
* Adhemerval Zanella Netto:

>> +  errno = 0;
>>    tzset ();
>> +  if (errno != 0)
>> +    printf ("warning: tzset set errno to %d (%m)", errno);
>
> Shouldn't it call support_record_failure() in this case?

It's not a documented way for checking for tzset failures.  I don't want
to increase the number of test failures we see further. 8-)

Thanks,
Florian
  
Paul Eggert April 8, 2025, 6:40 p.m. UTC | #3
On 2025-04-04 03:45, Florian Weimer wrote:
> +  errno = 0;
>     tzset ();
> +  if (errno != 0)
> +    printf ("warning: tzset set errno to %d (%m)", errno);

I don't know why this part of the patch is useful, as tzset's effect on 
errno is unspecified. If this part is useful, please add an explanatory 
comment.

Otherwise the patch looks good, thanks.
  
Adhemerval Zanella Netto April 8, 2025, 6:40 p.m. UTC | #4
On 08/04/25 15:38, Florian Weimer wrote:
> * Adhemerval Zanella Netto:
> 
>>> +  errno = 0;
>>>    tzset ();
>>> +  if (errno != 0)
>>> +    printf ("warning: tzset set errno to %d (%m)", errno);
>>
>> Shouldn't it call support_record_failure() in this case?
> 
> It's not a documented way for checking for tzset failures.  I don't want
> to increase the number of test failures we see further. 8-)

Fair enough.
  

Patch

diff --git a/timezone/tst-bz28707.c b/timezone/tst-bz28707.c
index a5723f4b8a..bb81ad1bd6 100644
--- a/timezone/tst-bz28707.c
+++ b/timezone/tst-bz28707.c
@@ -15,10 +15,11 @@ 
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
-#include <time.h>
+#include <errno.h>
 #include <stdio.h>
 #include <stdlib.h>
-#include <string.h>
+#include <support/check.h>
+#include <time.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
@@ -28,19 +29,21 @@ 
 static int
 do_test (void)
 {
-  if (setenv ("TZ", "XT5", 1))
-    {
-      puts ("setenv failed.");
-      return 1;
-    }
+  if (setenv ("TZ", "XT5", 1) != 0)
+    FAIL_EXIT1 ("setenv: %m");
 
+  errno = 0;
   tzset ();
+  if (errno != 0)
+    printf ("warning: tzset set errno to %d (%m)", errno);
 
-  return
-    /* Sanity-check that we got the right abbreviation 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);
+  /* Sanity-check that we got the right abbreviation 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.  */
+  TEST_COMPARE_STRING (tzname[1], "-03");
+
+  return 0;
 }
+
 #include <support/test-driver.c>