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.
This commit is contained in:
António Fernandes 2020-09-23 18:28:40 +01:00 committed by Philip Withnall
parent 7124b91e21
commit 19c5e13fc4

View File

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