From 19c5e13fc4d162c30ee3a67672b7bf2642be23fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ant=C3=B3nio=20Fernandes?= Date: Wed, 23 Sep 2020 18:28:40 +0100 Subject: [PATCH] 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..40064c9ab 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; + const 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);