[glib/wip/antoniof/fallback-timezone-cache-lookup: 3/5] gtimezone: Set resolved_identifier earlier

[António Fernandes <antoniof src gnome org>]

commit 3fff198a3704164e07ba535526411ef434ae133f
Author: António Fernandes <antoniof gnome org>
Date:   Wed Sep 23 18:28:40 2020 +0100

    gtimezone: Set resolved_identifier earlier
    
    We have been passing a &resolved_identifier address around for multiple
    functions to set it. Each function may either:
    
        1.  leaving it for the next function to set, if returning early;
        2.  set it to a duplicate of the passed identifier, if not NULL;
        3.  get a fallback value and set it, otherwise.
    
    This can be simplified by setting it early to either:
    
        1.  a duplicate of the passed identifier, if not NULL;
        2.  a fallback value, otherwise.
    
    This way we can avoid some unnecessary string duplication and freeing.
    Also, on Windows, we avoid calling windows_default_tzname() twice.
    
    But the main motivation for this change is enabling the performance
    optimization in the next commit.

 glib/gtimezone.c | 76 +++++++++++++++++++-------------------------------------
 1 file changed, 25 insertions(+), 51 deletions(-)
---
diff --git a/glib/gtimezone.c b/glib/gtimezone.c
index d90a9bb73..da9c3fe65 100644
--- a/glib/gtimezone.c
+++ b/glib/gtimezone.c
@@ -508,13 +508,12 @@ out:
 }
 
 static GBytes*
-zone_info_unix (const gchar  *identifier,
-                gchar       **out_identifier)
+zone_info_unix (const gchar *identifier,
+                const gchar *resolved_identifier)
 {
   gchar *filename = NULL;
   GMappedFile *file = NULL;
   GBytes *zoneinfo = NULL;
-  gchar *resolved_identifier = NULL;
   const gchar *tzdir;
 
   tzdir = g_getenv ("TZDIR");
@@ -527,8 +526,6 @@ zone_info_unix (const gchar  *identifier,
      glibc allows both syntaxes, so we should too */
   if (identifier != NULL)
     {
-      resolved_identifier = g_strdup (identifier);
-
       if (*identifier == ':')
         identifier ++;
 
@@ -539,7 +536,6 @@ zone_info_unix (const gchar  *identifier,
     }
   else
     {
-      resolved_identifier = zone_identifier_unix ();
       if (resolved_identifier == NULL)
         goto out;
 
@@ -559,10 +555,6 @@ zone_info_unix (const gchar  *identifier,
   g_assert (resolved_identifier != NULL);
 
 out:
-  if (out_identifier != NULL)
-    *out_identifier = g_steal_pointer (&resolved_identifier);
-
-  g_free (resolved_identifier);
   g_free (filename);
 
   return zoneinfo;
@@ -834,14 +826,13 @@ register_tzi_to_tzi (RegTZI *reg, TIME_ZONE_INFORMATION *tzi)
 
 static guint
 rules_from_windows_time_zone (const gchar   *identifier,
-                              gchar        **out_identifier,
-                              TimeZoneRule **rules,
-                              gboolean       copy_identifier)
+                              const gchar   *resolved_identifier,
+                              TimeZoneRule **rules)
 {
   HKEY key;
   gchar *subkey = NULL;
   gchar *subkey_dynamic = NULL;
-  gchar *key_name = NULL;
+  gchar *key_name;
   const gchar *reg_key =
     "SOFTWARE\\Microsoft\\Windows NT\\CurrentVersion\\Time Zones\\";
   TIME_ZONE_INFORMATION tzi;
@@ -856,19 +847,15 @@ rules_from_windows_time_zone (const gchar   *identifier,
   if (GetSystemDirectoryW (winsyspath, MAX_PATH) == 0)
     return 0;
 
-  g_assert (copy_identifier == FALSE || out_identifier != NULL);
   g_assert (rules != NULL);
 
-  if (copy_identifier)
-    *out_identifier = NULL;
-
   *rules = NULL;
   key_name = NULL;
 
   if (!identifier)
-    key_name = windows_default_tzname ();
+    key_name = resolved_identifier;
   else
-    key_name = g_strdup (identifier);
+    key_name = identifier;
 
   if (!key_name)
     return 0;
@@ -1011,16 +998,9 @@ utf16_conv_failed:
       else
         (*rules)[rules_num - 1].start_year = (*rules)[rules_num - 2].start_year + 1;
 
-      if (copy_identifier)
-        *out_identifier = g_steal_pointer (&key_name);
-      else
-        g_free (key_name);
-
       return rules_num;
     }
 
-  g_free (key_name);
-
   return 0;
 }
 
@@ -1521,16 +1501,13 @@ parse_identifier_boundaries (gchar **pos, TimeZoneRule *tzr)
  */
 static guint
 rules_from_identifier (const gchar   *identifier,
-                       gchar        **out_identifier,
                        TimeZoneRule **rules)
 {
   gchar *pos;
   TimeZoneRule tzr;
 
-  g_assert (out_identifier != NULL);
   g_assert (rules != NULL);
 
-  *out_identifier = NULL;
   *rules = NULL;
 
   if (!identifier)
@@ -1545,7 +1522,6 @@ rules_from_identifier (const gchar   *identifier,
 
   if (*pos == 0)
     {
-      *out_identifier = g_strdup (identifier);
       return create_ruleset_from_rule (rules, &tzr);
     }
 
@@ -1566,14 +1542,8 @@ rules_from_identifier (const gchar   *identifier,
       /* Use US rules, Windows' default is Pacific Standard Time */
       if ((rules_num = rules_from_windows_time_zone ("Pacific Standard Time",
                                                      NULL,
-                                                     rules,
-                                                     FALSE)))
+                                                     rules)))
         {
-          /* We don't want to hardcode our identifier here as
-           * "Pacific Standard Time", use what was passed in
-           */
-          *out_identifier = g_strdup (identifier);
-
           for (i = 0; i < rules_num - 1; i++)
             {
               (*rules)[i].std_offset = - tzr.std_offset;
@@ -1594,7 +1564,6 @@ rules_from_identifier (const gchar   *identifier,
   if (!parse_identifier_boundaries (&pos, &tzr))
     return 0;
 
-  *out_identifier = g_strdup (identifier);
   return create_ruleset_from_rule (rules, &tzr);
 }
 
@@ -1605,17 +1574,13 @@ parse_footertz (const gchar *footer, size_t footerlen)
   gchar *tzstring = g_strndup (footer + 1, footerlen - 2);
   GTimeZone *footertz = NULL;
 
-  /* FIXME: it might make sense to modify rules_from_identifier to
-     allow NULL to be passed instead of &ident, saving the strdup/free
-     pair.  The allocation for tzstring could also be avoided by
+  /* FIXME: The allocation for tzstring could be avoided by
      passing a gsize identifier_len argument to rules_from_identifier
      and changing the code in that function to stop assuming that
      identifier is nul-terminated.  */
-  gchar *ident;
   TimeZoneRule *rules;
-  guint rules_num = rules_from_identifier (tzstring, &ident, &rules);
+  guint rules_num = rules_from_identifier (tzstring, &rules);
 
-  g_free (ident);
   g_free (tzstring);
   if (rules_num > 1)
     {
@@ -1723,6 +1688,16 @@ g_time_zone_new (const gchar *identifier)
           G_UNLOCK (time_zones);
           return tz;
         }
+      else
+        resolved_identifier = g_strdup (identifier);
+    }
+  else
+    {
+#ifdef G_OS_UNIX
+      resolved_identifier = zone_identifier_unix ();
+#elif defined (G_OS_WIN32)
+      resolved_identifier = windows_default_tzname ();
+#endif
     }
 
   tz = g_slice_new0 (GTimeZone);
@@ -1731,7 +1706,7 @@ g_time_zone_new (const gchar *identifier)
   zone_for_constant_offset (tz, identifier);
 
   if (tz->t_info == NULL &&
-      (rules_num = rules_from_identifier (identifier, &resolved_identifier, &rules)))
+      (rules_num = rules_from_identifier (identifier, &rules)))
     {
       init_zone_from_rules (tz, rules, rules_num, g_steal_pointer (&resolved_identifier));
       g_free (rules);
@@ -1740,7 +1715,7 @@ g_time_zone_new (const gchar *identifier)
   if (tz->t_info == NULL)
     {
 #ifdef G_OS_UNIX
-      GBytes *zoneinfo = zone_info_unix (identifier, &resolved_identifier);
+      GBytes *zoneinfo = zone_info_unix (identifier, resolved_identifier);
       if (zoneinfo != NULL)
         {
           init_zone_from_iana_info (tz, zoneinfo, g_steal_pointer (&resolved_identifier));
@@ -1748,9 +1723,8 @@ g_time_zone_new (const gchar *identifier)
         }
 #elif defined (G_OS_WIN32)
       if ((rules_num = rules_from_windows_time_zone (identifier,
-                                                     &resolved_identifier,
-                                                     &rules,
-                                                     TRUE)))
+                                                     resolved_identifier,
+                                                     &rules)))
         {
           init_zone_from_rules (tz, rules, rules_num, g_steal_pointer (&resolved_identifier));
           g_free (rules);
@@ -1777,7 +1751,7 @@ g_time_zone_new (const gchar *identifier)
                   rules[0].start_year = MIN_TZYEAR;
                   rules[1].start_year = MAX_TZYEAR;
 
-                  init_zone_from_rules (tz, rules, 2, windows_default_tzname ());
+                  init_zone_from_rules (tz, rules, 2, g_steal_pointer (&resolved_identifier));
                 }
 
               g_free (rules);