[PATCH v2] tst-tzset: output reason when creating 4GiB file fails

[PATCH v2] tst-tzset: output reason when creating 4GiB file fails

[Stafford Horne]

Currently, if the temporary file creation fails the create_tz_file
function returns NULL.  The NULL pointer is then passed to setenv which
causes a SIGSEGV.  Rather than failing with a SIGSEGV print a warning
and exit.

Cc: Adhemerval Zanella <adhemerval.zanella@linaro.org>
---
Changes since v1:
 - Use PRId64 printf formatting, suggested by Adhemerval

 timezone/tst-tzset.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/timezone/tst-tzset.c b/timezone/tst-tzset.c
index d6da2932bb..3dad42e041 100644
--- a/timezone/tst-tzset.c
+++ b/timezone/tst-tzset.c
@@ -25,6 +25,7 @@
 #include <time.h>
 #include <unistd.h>
 #include <support/check.h>
+#include <inttypes.h>
 
 static int do_test (void);
 #define TEST_FUNCTION do_test ()
@@ -103,6 +104,13 @@ static void
 test_tz_file (off64_t size)
 {
   char *path = create_tz_file (size);
+  if (path == NULL)
+   {
+     printf ("creating timezone file of size: %" PRId64 "MiB failed.\n",
+	     size / (1024 * 1024));
+     exit (1);
+   }
+
   if (setenv ("TZ", path, 1) < 0)
     {
       printf ("setenv failed: %m\n");
-- 
2.31.1

Re: [PATCH v2] tst-tzset: output reason when creating 4GiB file fails

[Adhemerval Zanella]

On 02/11/2021 20:31, Stafford Horne wrote:
> Currently, if the temporary file creation fails the create_tz_file
> function returns NULL.  The NULL pointer is then passed to setenv which
> causes a SIGSEGV.  Rather than failing with a SIGSEGV print a warning
> and exit.
> 
> Cc: Adhemerval Zanella <adhemerval.zanella@linaro.org>

LGTM, thanks.

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

> ---
> Changes since v1:
>  - Use PRId64 printf formatting, suggested by Adhemerval
> 
>  timezone/tst-tzset.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/timezone/tst-tzset.c b/timezone/tst-tzset.c
> index d6da2932bb..3dad42e041 100644
> --- a/timezone/tst-tzset.c
> +++ b/timezone/tst-tzset.c
> @@ -25,6 +25,7 @@
>  #include <time.h>
>  #include <unistd.h>
>  #include <support/check.h>
> +#include <inttypes.h>
>  
>  static int do_test (void);
>  #define TEST_FUNCTION do_test ()
> @@ -103,6 +104,13 @@ static void
>  test_tz_file (off64_t size)
>  {
>    char *path = create_tz_file (size);
> +  if (path == NULL)
> +   {
> +     printf ("creating timezone file of size: %" PRId64 "MiB failed.\n",
> +	     size / (1024 * 1024));
> +     exit (1);
> +   }
> +
>    if (setenv ("TZ", path, 1) < 0)
>      {
>        printf ("setenv failed: %m\n");
> 

Re: [PATCH v2] tst-tzset: output reason when creating 4GiB file fails

[Paul Eggert]

On 11/2/21 16:31, Stafford Horne via Libc-alpha wrote:
> +   {
> +     printf ("creating timezone file of size: %" PRId64 "MiB failed.\n",
> +	     size / (1024 * 1024));

Why not simply print the size in bytes? Converting to MiB outputs less 
information due to rounding, and is hardly worth the trouble here. Other 
than that, looks fine.

Re: [PATCH v2] tst-tzset: output reason when creating 4GiB file fails

[Stafford Horne]

On Wed, Nov 03, 2021 at 12:14:52PM -0700, Paul Eggert wrote:
> On 11/2/21 16:31, Stafford Horne via Libc-alpha wrote:
> > +   {
> > +     printf ("creating timezone file of size: %" PRId64 "MiB failed.\n",
> > +	     size / (1024 * 1024));
> 
> Why not simply print the size in bytes? Converting to MiB outputs less
> information due to rounding, and is hardly worth the trouble here. Other
> than that, looks fine.

Hi Paul,

The reason for the divide by (1024 * 1024) it to make it easier to see where the
failure is at a glance.

The call to test_tz_file is done two times:

  test_tz_file (64 * 1024 * 1024);

  test_tz_file (4LL * 1024 * 1024 * 1024 - 6);

For me, it's a lot easier to tell the different between 2 and 4 digits (64MiB
and 4096MiB) than the bigger numbers.  The information lost is not too important
considering the test, in my opinion.

-Stafford

Re: [PATCH v2] tst-tzset: output reason when creating 4GiB file fails

[Stafford Horne]

On Thu, Nov 04, 2021 at 05:30:27AM +0900, Stafford Horne wrote:
> On Wed, Nov 03, 2021 at 12:14:52PM -0700, Paul Eggert wrote:
> > On 11/2/21 16:31, Stafford Horne via Libc-alpha wrote:
> > > +   {
> > > +     printf ("creating timezone file of size: %" PRId64 "MiB failed.\n",
> > > +	     size / (1024 * 1024));
> > 
> > Why not simply print the size in bytes? Converting to MiB outputs less
> > information due to rounding, and is hardly worth the trouble here. Other
> > than that, looks fine.
> 
> Hi Paul,
> 
> The reason for the divide by (1024 * 1024) it to make it easier to see where the
> failure is at a glance.
> 
> The call to test_tz_file is done two times:
> 
>   test_tz_file (64 * 1024 * 1024);
> 
>   test_tz_file (4LL * 1024 * 1024 * 1024 - 6);
> 
> For me, it's a lot easier to tell the different between 2 and 4 digits (64MiB
> and 4096MiB) than the bigger numbers.  The information lost is not too important
> considering the test, in my opinion.

Hi,

I have pushed this as is.

-Stafford