Message ID | 20211102233159.4094554-1-shorne@gmail.com |
---|---|
State | New |
Headers | show |
Series | [v2] tst-tzset: output reason when creating 4GiB file fails | expand |
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"); >
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.
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
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
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");