[PATCH] Fix tzfile.c

[PATCH] Fix tzfile.c

[Jakub Jelinek]

Hi!

The recent tzfile.c changes broke any program which calls tzset or strftime*
or mktime* more than once.
Although the second __tzfile_read set __use_tzfile to 1, transitions was
NULL.
The patch below fixes this.

Still, I wonder if it is a good idea to fopen /etc/localtime every time
strfime, strftime_l or mktime is called or if it wouldn't be enough to
use the old behaviour of tzset (where if $TZ is unset and was previously
unset too, nothing will happen) when called from these functions and
only do the slow path if tzset () is called by the user program.

E.g. rpm -qia call results in almost 15000 openings of /etc/localtime,
which is IMHO a lot.

2004-08-11  Jakub Jelinek  <jakub@redhat.com>

	* time/tzfile.c (__tzfile_read): Free transitions only if it will
	not be reused.

--- libc/time/tzfile.c.jj	2004-08-11 17:27:33.000000000 +0200
+++ libc/time/tzfile.c	2004-08-11 17:47:20.654952554 +0200
@@ -104,16 +104,12 @@ __tzfile_read (const char *file, size_t 
 
   __use_tzfile = 0;
 
-  if (transitions != NULL)
-    free ((void *) transitions);
-  transitions = NULL;
-
   if (file == NULL)
     /* No user specification; use the site-wide default.  */
     file = TZDEFAULT;
   else if (*file == '\0')
     /* User specified the empty string; use UTC with no leap seconds.  */
-    return;
+    goto ret_free_transitions;
   else
     {
       /* We must not allow to read an arbitrary file in a setuid
@@ -127,7 +123,7 @@ __tzfile_read (const char *file, size_t 
 	      || strstr (file, "../") != NULL))
 	/* This test is certainly a bit too restrictive but it should
 	   catch all critical cases.  */
-	return;
+	goto ret_free_transitions;
     }
 
   if (*file != '/')
@@ -156,14 +152,14 @@ __tzfile_read (const char *file, size_t 
      disabled.  */
   f = fopen (file, "rc");
   if (f == NULL)
-    return;
+    goto ret_free_transitions;
 
   /* Get information about the file.  */
   struct stat64 st;
   if (fstat64 (fileno (f), &st) != 0)
     {
       fclose (f);
-      return;
+      goto ret_free_transitions;
     }
   if (was_using_tzfile && tzfile_ino == st.st_ino && tzfile_dev == st.st_dev)
     {
@@ -173,6 +169,9 @@ __tzfile_read (const char *file, size_t 
       return;
     }
 
+  free ((void *) transitions);
+  transitions = NULL;
+
   /* Remember the inode and device number.  */
   tzfile_dev = st.st_dev;
   tzfile_ino = st.st_ino;
@@ -381,6 +380,9 @@ __tzfile_read (const char *file, size_t 
 
  lose:
   fclose (f);
+ ret_free_transitions:
+  free ((void *) transitions);
+  transitions = NULL;
 }
 \f
 /* The user specified a hand-made timezone, but not its DST rules.

	Jakub

Re: [PATCH] Fix tzfile.c

[Roland McGrath]

> Still, I wonder if it is a good idea to fopen /etc/localtime every time
> strfime, strftime_l or mktime is called or if it wouldn't be enough to
> use the old behaviour of tzset (where if $TZ is unset and was previously
> unset too, nothing will happen) when called from these functions and
> only do the slow path if tzset () is called by the user program.

This is why tzset exists.  I don't thik mktime et al ought to be rechecking
this.

Re: [PATCH] Fix tzfile.c

[Ulrich Drepper]

Jakub Jelinek wrote:

> Still, I wonder if it is a good idea to fopen /etc/localtime every time
> strfime, strftime_l or mktime is called

I've checked in a patch for this.  And your's as well.

-- 
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖