Merge lp:~dshea/timezonemap/memory-problems into lp:timezonemap

[David Shea]

This fixes a memory leak and an access of freed memory, and works around a call to setenv which is not thread-safe. tz_location_set_locally could probably be removed but it's kind of part of the public interface so I didn't want to mess with that.

[lp:~dshea/timezonemap/memory-problems]

62. By David Shea on 2016-06-28

Fix the calculation of time zone offsets.

g_get_real_time returns microseconds, not milliseconds.

Unmerged revisions

62. By David Shea on 2016-06-28

Fix the calculation of time zone offsets.

g_get_real_time returns microseconds, not milliseconds.

61. By David Shea on 2016-05-24

Do not use tz_location_get_utc_offset

The function in tz.c to fetch the timezone offset for a given location
sets the TZ environment variable which, besides being potentially
surprising to the rest of the process, is not thread-safe. Instead,
skip the function entirely and use the glib functions for querying time
zone data.

Increase the required glib version to 2.26 for the GTimeZone functions.

60. By David Shea on 2016-05-10

Fix an invalid memory access

When truncating the list for the distance-based location search, do not
attempt to read the rest of the list after it has been freed.

59. By David Shea on 2016-05-10

Fix memory leaks in tz.c

Use a stack-allocated block for the user_data struct passed to
parse_cities15000, since the struct itself is not needed after
parse_file, and no one can forget to free it if it's not dynamically
allocated the first place.

Use const strings in tz_data_file_get so that the return value does not
need to be duplicated and freed.

=== modified file 'debian/control'
--- debian/control	2015-05-07 13:47:16 +0000
+++ debian/control	2016-06-28 17:25:08 +0000
@@ -9,7 +9,7 @@
                gir1.2-glib-2.0,
                gir1.2-gtk-3.0,
                intltool (>= 0.35.0),
-               libglib2.0-dev (>= 2.25.0),
+               libglib2.0-dev (>= 2.26.0),
                libgtk-3-dev (>= 3.1.4),
                libcairo2-dev (>= 1.10),
                libjson-glib-dev,
@@ -57,7 +57,7 @@
 Depends: ${shlibs:Depends}, 
          ${misc:Depends},
          libtimezonemap1 (= ${binary:Version}),
-         libglib2.0-dev (>= 2.25.0),
+         libglib2.0-dev (>= 2.26.0),
          libgtk-3-dev (>= 3.1.4),
          libjson-glib-dev
 Replaces: gir1.2-timezonemap-1.0 (<< 0.3)

=== modified file 'src/cc-timezone-map.c'
--- src/cc-timezone-map.c	2015-05-07 16:43:32 +0000
+++ src/cc-timezone-map.c	2016-06-28 17:25:08 +0000
@@ -843,6 +843,39 @@
   return 0;
 }
 
+/* Return the UTC offset (in hours) for the standard (winter) time at a location */
+static gdouble
+get_location_offset (CcTimezoneLocation *location)
+{
+  const gchar *zone_name;
+  GTimeZone *zone;
+  gint interval;
+  gint64 curtime;
+  gint32 offset;
+
+  g_return_val_if_fail (location != NULL, 0);
+  zone_name = cc_timezone_location_get_zone (location);
+  g_return_val_if_fail (zone_name != NULL, 0);
+
+  zone = g_time_zone_new (zone_name);
+
+  /* Query the zone based on the current time, since otherwise the data
+   * may not make sense. */
+  curtime = g_get_real_time () / 1000000;    /* convert to seconds */
+  interval = g_time_zone_find_interval (zone, G_TIME_TYPE_UNIVERSAL, curtime);
+
+  offset = g_time_zone_get_offset (zone, interval);
+  if (g_time_zone_is_dst (zone, interval))
+    {
+      /* Subtract an hour's worth of seconds to get the standard time offset */
+      offset -= (60 * 60);
+    }
+
+  g_time_zone_unref (zone);
+
+  return offset / (60.0 * 60.0);
+}
+
 static void
 set_location (CcTimezoneMap *map,
               CcTimezoneLocation    *location)
@@ -854,11 +887,8 @@
 
   if (priv->location)
   {
-    info = tz_info_from_location (priv->location);
-    priv->selected_offset = tz_location_get_utc_offset (priv->location)
-        / (60.0*60.0) + ((info->daylight) ? -1.0 : 0.0);
+    priv->selected_offset = get_location_offset (priv->location);
     priv->show_offset = TRUE;
-    tz_info_free (info);
   }
   else
   {
@@ -940,6 +970,7 @@
               /* Cut the list off here */
               node->prev->next = NULL;
               g_list_free(node);
+              break;
             }
 
           node = g_list_next(node);

=== modified file 'src/tz.c'
--- src/tz.c	2013-12-16 17:01:37 +0000
+++ src/tz.c	2016-06-28 17:25:08 +0000
@@ -41,7 +41,7 @@
 #endif
 static int compare_country_names (const void *a, const void *b);
 static void sort_locations_by_country (GPtrArray *locations);
-static gchar * tz_data_file_get (gchar *env, gchar *defaultfile);
+static const gchar * tz_data_file_get (const gchar *env, const gchar *defaultfile);
 
 void parse_file (const char * filename,
                  const guint ncolumns,
@@ -179,7 +179,7 @@
 TzDB *
 tz_load_db (void)
 {
-    gchar *tz_data_file, *admin1_file, *country_file;
+    const gchar *tz_data_file, *admin1_file, *country_file;
     TzDB *tz_db;
     char buf[4096];
 
@@ -217,23 +217,19 @@
     tz_db = g_new0 (TzDB, 1);
     tz_db->locations = g_ptr_array_new ();
 
-    Triple * triple = g_new (Triple, 1);
-    triple->first = tz_db->locations;
-    triple->second = stateHash;
-    triple->third = countryHash;
+    Triple triple;
+    triple.first = tz_db->locations;
+    triple.second = stateHash;
+    triple.third = countryHash;
 
-    parse_file (tz_data_file, 19, parse_cities15000, triple);
+    parse_file (tz_data_file, 19, parse_cities15000, &triple);
 
     g_hash_table_destroy (stateHash);
     g_hash_table_destroy (countryHash);
-    triple->second = NULL;
-    triple->third = NULL;
 
     /* now sort by country */
     sort_locations_by_country (tz_db->locations);
 
-    g_free (tz_data_file);
-
     return tz_db;
 }
 
@@ -391,14 +387,14 @@
  * Private functions *
  * ----------------- */
 
-static gchar *
-tz_data_file_get (gchar *env, gchar *defaultfile)
+static const gchar *
+tz_data_file_get (const gchar *env, const gchar *defaultfile)
 {
     /* Allow passing this in at runtime, to support loading it from the build
      * tree during tests. */
     const gchar * filename = g_getenv (env);
 
-    return filename ? g_strdup (filename) : g_strdup (defaultfile);
+    return filename ? filename : defaultfile;
 }
 
 #ifdef __sun