[libgweather/benzea/wip-variant-backend: 4/15] gweather: Change API to return references




commit 5de66123a8670ff88a7c5667ebc6c195d4fe3029
Author: Benjamin Berg <bberg redhat com>
Date:   Fri Jan 8 14:41:29 2021 +0100

    gweather: Change API to return references
    
    The old API effectively requires the whole world to be kept in memory at
    all times. However, this is inefficient, so change the API in order to
    permit changing the underlying implementation.

 libgweather/gweather-location-entry.c |  2 +-
 libgweather/gweather-location.c       | 44 ++++++++++++++++++++++++-----------
 libgweather/gweather-timezone-menu.c  |  3 ++-
 libgweather/gweather-timezone.c       | 11 +++++----
 libgweather/gweather-weather.c        |  4 +---
 libgweather/test_libgweather.c        | 38 +++++++++++++++++-------------
 6 files changed, 63 insertions(+), 39 deletions(-)
---
diff --git a/libgweather/gweather-location-entry.c b/libgweather/gweather-location-entry.c
index 92c3ce63..b429b186 100644
--- a/libgweather/gweather-location-entry.c
+++ b/libgweather/gweather-location-entry.c
@@ -175,7 +175,7 @@ constructed (GObject *object)
     entry = GWEATHER_LOCATION_ENTRY (object);
 
     if (!entry->priv->top)
-       entry->priv->top = gweather_location_ref (gweather_location_get_world ());
+       entry->priv->top = gweather_location_get_world ();
 
     store = gtk_tree_store_new (4, G_TYPE_STRING, GWEATHER_TYPE_LOCATION, G_TYPE_STRING, G_TYPE_STRING);
     fill_location_entry_model (store, entry->priv->top, NULL, NULL, NULL, entry->priv->show_named_timezones);
diff --git a/libgweather/gweather-location.c b/libgweather/gweather-location.c
index d0b924ab..85cd9a99 100644
--- a/libgweather/gweather-location.c
+++ b/libgweather/gweather-location.c
@@ -439,9 +439,10 @@ _gweather_location_reset_world (void)
  * representing a hierarchy containing all of the locations from
  * Locations.xml.
  *
- * Return value: (allow-none) (transfer none): a %GWEATHER_LOCATION_WORLD
+ * Prior to version 40 no reference was returned.
+ *
+ * Return value: (allow-none) (transfer full): a %GWEATHER_LOCATION_WORLD
  * location, or %NULL if Locations.xml could not be found or could not be parsed.
- * The return value is owned by libgweather and should not be modified or freed.
  **/
 GWeatherLocation *
 gweather_location_get_world (void)
@@ -471,7 +472,7 @@ gweather_location_get_world (void)
        _gweather_parser_free (parser);
     }
 
-    return global_world;
+    return gweather_location_ref (global_world);
 }
 
 /**
@@ -545,7 +546,7 @@ void
 gweather_location_unref (GWeatherLocation *loc)
 {
     g_return_if_fail (loc != NULL);
-    g_return_if_fail (loc->level != GWEATHER_LOCATION_WORLD || loc->ref_count > 1);
+    g_return_if_fail (loc != global_world || loc->ref_count > 1);
 
     _gweather_location_unref_no_check (loc);
 }
@@ -694,14 +695,17 @@ gweather_location_level_to_string (GWeatherLocationLevel level)
  *
  * Gets @loc's parent location.
  *
- * Return value: (transfer none) (allow-none): @loc's parent, or %NULL
+ * Prior to version 40 no reference was returned.
+ *
+ * Return value: (transfer full) (allow-none): @loc's parent, or %NULL
  * if @loc is a %GWEATHER_LOCATION_WORLD node.
  **/
 GWeatherLocation *
 gweather_location_get_parent (GWeatherLocation *loc)
 {
     g_return_val_if_fail (loc != NULL, NULL);
-    return loc->parent;
+
+    return loc->parent ? gweather_location_ref (loc->parent) : NULL;
 }
 
 /**
@@ -824,6 +828,7 @@ gweather_location_find_nearest_city (GWeatherLocation *loc,
                                     double            lat,
                                     double            lon)
 {
+    g_autoptr(GWeatherLocation) world = NULL;
     /* The data set really isn't too big. Don't concern ourselves
      * with a proper nearest neighbors search. Instead, just do
      * an O(n) search. */
@@ -832,7 +837,7 @@ gweather_location_find_nearest_city (GWeatherLocation *loc,
     g_return_val_if_fail (loc == NULL || loc->level < GWEATHER_LOCATION_CITY, NULL);
 
     if (loc == NULL)
-       loc = gweather_location_get_world ();
+       loc = world = gweather_location_get_world ();
 
     lat = lat * M_PI / 180.0;
     lon = lon * M_PI / 180.0;
@@ -879,6 +884,7 @@ gweather_location_find_nearest_city_full (GWeatherLocation  *loc,
                                          gpointer           user_data,
                                          GDestroyNotify     destroy)
 {
+    g_autoptr(GWeatherLocation) world = NULL;
     /* The data set really isn't too big. Don't concern ourselves
      * with a proper nearest neighbors search. Instead, just do
      * an O(n) search. */
@@ -888,7 +894,7 @@ gweather_location_find_nearest_city_full (GWeatherLocation  *loc,
                          loc->level == GWEATHER_LOCATION_NAMED_TIMEZONE, NULL);
 
     if (loc == NULL)
-        loc = gweather_location_get_world ();
+        loc = world = gweather_location_get_world ();
 
     lat = lat * M_PI / 180.0;
     lon = lon * M_PI / 180.0;
@@ -972,6 +978,7 @@ gweather_location_detect_nearest_city (GWeatherLocation    *loc,
                                        GAsyncReadyCallback callback,
                                        gpointer            user_data)
 {
+    g_autoptr(GWeatherLocation) world = NULL;
     ArgData *data;
     GeocodeLocation *location;
     GeocodeReverse *reverse;
@@ -981,7 +988,7 @@ gweather_location_detect_nearest_city (GWeatherLocation    *loc,
                      loc->level == GWEATHER_LOCATION_NAMED_TIMEZONE);
 
     if (loc == NULL)
-        loc = gweather_location_get_world ();
+        loc = world = gweather_location_get_world ();
 
     location = geocode_location_new (lat, lon, GEOCODE_LOCATION_ACCURACY_CITY);
     reverse = geocode_reverse_new_for_location (location);
@@ -1361,7 +1368,9 @@ _gweather_location_update_weather_location (GWeatherLocation *gloc,
  *
  * See gweather_location_deserialize() to recover a stored #GWeatherLocation.
  *
- * Returns: (transfer none): a weather station level #GWeatherLocation for @station_code,
+ * Prior to version 40 no reference was returned.
+ *
+ * Returns: (transfer full): a weather station level #GWeatherLocation for @station_code,
  *          or %NULL if none exists in the database.
  */
 GWeatherLocation *
@@ -1371,7 +1380,7 @@ gweather_location_find_by_station_code (GWeatherLocation *world,
     GList *l;
 
     l = g_hash_table_lookup (world->metar_code_cache, station_code);
-    return l ? l->data : NULL;
+    return l ? gweather_location_ref (l->data) : NULL;
 }
 
 /**
@@ -1382,13 +1391,19 @@ gweather_location_find_by_station_code (GWeatherLocation *world,
  * Retrieves the country identified by the specified ISO 3166 code,
  * if present in the database.
  *
- * Returns: (transfer none): a country level #GWeatherLocation, or %NULL.
+ * Prior to version 40 no reference was returned.
+ *
+ * Returns: (transfer full): a country level #GWeatherLocation, or %NULL.
  */
 GWeatherLocation *
 gweather_location_find_by_country_code (GWeatherLocation *world,
                                         const gchar      *country_code)
 {
-       return g_hash_table_lookup (world->country_code_cache, country_code);
+       GWeatherLocation *res;
+
+       res = g_hash_table_lookup (world->country_code_cache, country_code);
+
+       return res ? gweather_location_ref (res) : NULL;
 }
 
 /**
@@ -1787,7 +1802,8 @@ gweather_location_new_detached (const char *name,
                                gdouble     latitude,
                                gdouble     longitude)
 {
-    GWeatherLocation *world, *city;
+    g_autoptr(GWeatherLocation) world = NULL;
+    g_autoptr(GWeatherLocation) city = NULL;
 
     g_return_val_if_fail (name != NULL, NULL);
 
diff --git a/libgweather/gweather-timezone-menu.c b/libgweather/gweather-timezone-menu.c
index 1914caff..c4559bba 100644
--- a/libgweather/gweather-timezone-menu.c
+++ b/libgweather/gweather-timezone-menu.c
@@ -262,6 +262,7 @@ insert_locations (GtkTreeStore *store, GWeatherLocation *loc)
 static GtkTreeModel *
 gweather_timezone_model_new (GWeatherLocation *top)
 {
+    g_autoptr(GWeatherLocation) world = NULL;
     GtkTreeStore *store;
     GtkTreeModel *model;
     GtkTreeIter iter;
@@ -290,7 +291,7 @@ gweather_timezone_model_new (GWeatherLocation *top)
     g_free (unknown);
 
     if (!top)
-       top = gweather_location_get_world ();
+       top = world = gweather_location_get_world ();
 
     insert_locations (store, top);
 
diff --git a/libgweather/gweather-timezone.c b/libgweather/gweather-timezone.c
index 3916dc57..daa5ed85 100644
--- a/libgweather/gweather-timezone.c
+++ b/libgweather/gweather-timezone.c
@@ -155,21 +155,24 @@ parse_tzdata (const char *tz_name, time_t start, time_t end,
  *
  * Get the #GWeatherTimezone for @tzid.
  *
- * Returns: (transfer none): A #GWeatherTimezone. This object
- * belongs to GWeather, do not unref it.
+ * Prior to version 40 no reference was returned.
+ *
+ * Returns: (transfer full): A #GWeatherTimezone.
  *
  * Since: 3.12
  */
 GWeatherTimezone *
 gweather_timezone_get_by_tzid (const char *tzid)
 {
-    GWeatherLocation *world;
+    g_autoptr(GWeatherLocation) world = NULL;
+    GWeatherTimezone *res;
 
     g_return_val_if_fail (tzid != NULL, NULL);
 
     world = gweather_location_get_world ();
+    res = g_hash_table_lookup (world->timezone_cache, tzid);
 
-    return g_hash_table_lookup (world->timezone_cache, tzid);
+    return res ? gweather_timezone_ref (res) : NULL;
 }
 
 static GWeatherTimezone *
diff --git a/libgweather/gweather-weather.c b/libgweather/gweather-weather.c
index 5f9230c9..8b8122c7 100644
--- a/libgweather/gweather-weather.c
+++ b/libgweather/gweather-weather.c
@@ -2153,7 +2153,7 @@ gweather_info_set_location_internal (GWeatherInfo     *info,
     if (priv->glocation) {
         gweather_location_ref (location);
     } else {
-        GWeatherLocation *world;
+        g_autoptr(GWeatherLocation) world = NULL;
         const gchar *station_code;
 
         default_loc = g_settings_get_value (priv->settings, DEFAULT_LOCATION);
@@ -2165,8 +2165,6 @@ gweather_info_set_location_internal (GWeatherInfo     *info,
 
        world = gweather_location_get_world ();
        priv->glocation = gweather_location_find_by_station_code (world, station_code);
-       if (priv->glocation)
-           gweather_location_ref (priv->glocation);
     }
 
     if (priv->glocation) {
diff --git a/libgweather/test_libgweather.c b/libgweather/test_libgweather.c
index f482fcbc..10c70753 100644
--- a/libgweather/test_libgweather.c
+++ b/libgweather/test_libgweather.c
@@ -39,7 +39,8 @@ static double max_distance = 0.0;
 static void
 test_named_timezones (void)
 {
-    GWeatherLocation *world, **children;
+    g_autoptr(GWeatherLocation) world = NULL;
+    GWeatherLocation **children;
     guint i;
 
     world = gweather_location_get_world ();
@@ -128,7 +129,7 @@ static void test_timezones (void);
 static void
 test_named_timezones_deserialized (void)
 {
-    GWeatherLocation *world;
+    g_autoptr(GWeatherLocation) world = NULL;
     GList *list, *l;
 
     world = gweather_location_get_world ();
@@ -164,7 +165,9 @@ static void
 test_no_code_serialize (void)
 {
     GVariant *variant;
-    GWeatherLocation *world, *loc, *new_loc;
+    g_autoptr(GWeatherLocation) world = NULL;
+    g_autoptr(GWeatherLocation) loc = NULL;
+    g_autoptr(GWeatherLocation) new_loc = NULL;
     GString *str;
 
     world = gweather_location_get_world ();
@@ -186,9 +189,6 @@ test_no_code_serialize (void)
     g_assert_nonnull (new_loc);
     g_assert_cmpstr (gweather_location_get_name (loc), ==, gweather_location_get_name (new_loc));
     g_assert_true (gweather_location_equal (loc, new_loc));
-    gweather_location_unref (new_loc);
-
-    gweather_location_unref (loc);
 
     _gweather_location_reset_world ();
 }
@@ -196,7 +196,7 @@ test_no_code_serialize (void)
 static void
 test_timezone (GWeatherLocation *location)
 {
-    GWeatherTimezone *gtz;
+    g_autoptr(GWeatherTimezone) gtz = NULL;
     const char *tz;
 
     tz = gweather_location_get_timezone_str (location);
@@ -244,7 +244,7 @@ test_timezones_children (GWeatherLocation *location)
 static void
 test_timezones (void)
 {
-    GWeatherLocation *world;
+    g_autoptr(GWeatherLocation) world = NULL;
 
     world = gweather_location_get_world ();
     g_assert_nonnull (world);
@@ -257,7 +257,7 @@ test_timezones (void)
 static void
 test_distance (GWeatherLocation *location)
 {
-    GWeatherLocation *parent;
+    g_autoptr(GWeatherLocation) parent = NULL;
     double distance;
 
     parent = gweather_location_get_parent (location);
@@ -293,7 +293,7 @@ test_airport_distance_children (GWeatherLocation *location)
 static void
 test_airport_distance_sanity (void)
 {
-    GWeatherLocation *world;
+    g_autoptr(GWeatherLocation) world = NULL;
 
     world = gweather_location_get_world ();
     g_assert_nonnull (world);
@@ -411,7 +411,7 @@ test_metar_weather_stations_children (GWeatherLocation *location,
 static void
 test_metar_weather_stations (void)
 {
-    GWeatherLocation *world;
+    g_autoptr(GWeatherLocation) world = NULL;
     SoupMessage *msg;
     SoupSession *session;
     GHashTable *stations_ht;
@@ -483,7 +483,8 @@ set_gsettings (void)
 static void
 test_utc_sunset (void)
 {
-       GWeatherLocation *world, *utc;
+       g_autoptr(GWeatherLocation) world = NULL;
+       g_autoptr(GWeatherLocation) utc = NULL;
        GWeatherInfo *info;
        char *sunset;
        GWeatherMoonPhase phase;
@@ -575,7 +576,7 @@ test_bad_duplicate_weather_stations_children (GWeatherLocation *location,
 static void
 test_bad_duplicate_weather_stations (void)
 {
-    GWeatherLocation *world;
+    g_autoptr(GWeatherLocation) world = NULL;
     GHashTable *stations_ht;
 
     g_setenv ("LIBGWEATHER_LOCATIONS_NO_NEAREST", "1", TRUE);
@@ -638,7 +639,7 @@ test_duplicate_weather_stations_children (GWeatherLocation *location)
 static void
 test_duplicate_weather_stations (void)
 {
-    GWeatherLocation *world;
+    g_autoptr(GWeatherLocation) world = NULL;
 
     g_setenv ("LIBGWEATHER_LOCATIONS_NO_NEAREST", "1", TRUE);
     world = gweather_location_get_world ();
@@ -653,7 +654,8 @@ test_duplicate_weather_stations (void)
 static void
 test_location_names (void)
 {
-    GWeatherLocation *world, *brussels;
+    g_autoptr(GWeatherLocation) world = NULL;
+    g_autoptr(GWeatherLocation) brussels = NULL;
 
     world = gweather_location_get_world ();
     g_assert_nonnull (world);
@@ -666,6 +668,9 @@ test_location_names (void)
     gweather_location_unref (brussels);
 
     setlocale (LC_ALL, "fr_FR.UTF-8");
+
+    g_clear_pointer (&world, gweather_location_unref);
+    g_clear_pointer (&brussels, gweather_location_unref);
     _gweather_location_reset_world ();
 
     world = gweather_location_get_world ();
@@ -737,7 +742,8 @@ static void
 test_weather_loop_use_after_free (void)
 {
     GMainLoop *loop;
-    GWeatherLocation *world, *loc;
+    g_autoptr(GWeatherLocation) world = NULL;
+    GWeatherLocation *loc;
     GWeatherInfo *info;
     const char *search_str = "LFLL";
 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]