GNOME.org
 

[libgweather/benzea/wip-variant-backend: 1/9] Change API to return references


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

    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       | 48 +++++++++++++++++++++++------------
 libgweather/gweather-location.h       |  2 ++
 libgweather/gweather-timezone-menu.c  |  3 ++-
 libgweather/gweather-timezone.c       | 11 +++++---
 libgweather/gweather-timezone.h       |  2 ++
 libgweather/gweather-weather.c        |  4 +--
 libgweather/test_libgweather.c        | 38 +++++++++++++++------------
 8 files changed, 69 insertions(+), 41 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 c472f274..78dfa71c 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
+ * Since version 40 a reference is 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
+ * Since version 40 a reference is 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,7 +828,8 @@ gweather_location_find_nearest_city (GWeatherLocation *loc,
                                     double            lat,
                                     double            lon)
 {
-   /* The data set really isn't too big. Don't concern ourselves
+    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. */
     struct FindNearestCityData data;
@@ -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,7 +884,8 @@ gweather_location_find_nearest_city_full (GWeatherLocation  *loc,
                                          gpointer           user_data,
                                          GDestroyNotify     destroy)
 {
-   /* The data set really isn't too big. Don't concern ourselves
+    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. */
     struct FindNearestCityData data;
@@ -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,
+ * Since version 40 a reference is 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.
+ * Since version 40 a reference is 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-location.h b/libgweather/gweather-location.h
index 701862d8..badf2226 100644
--- a/libgweather/gweather-location.h
+++ b/libgweather/gweather-location.h
@@ -58,6 +58,8 @@ GWeatherLocation      *gweather_location_ref            (GWeatherLocation  *loc)
 GWEATHER_EXTERN
 void                   gweather_location_unref          (GWeatherLocation  *loc);
 
+G_DEFINE_AUTOPTR_CLEANUP_FUNC (GWeatherLocation, gweather_location_unref);
+
 GWEATHER_EXTERN
 const char            *gweather_location_get_name       (GWeatherLocation  *loc);
 GWEATHER_EXTERN
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..fef7a1e2 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.
+ * Since version 40 a reference is 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-timezone.h b/libgweather/gweather-timezone.h
index 260a2fca..2368470f 100644
--- a/libgweather/gweather-timezone.h
+++ b/libgweather/gweather-timezone.h
@@ -51,6 +51,8 @@ GWeatherTimezone *gweather_timezone_ref            (GWeatherTimezone *zone);
 GWEATHER_EXTERN
 void              gweather_timezone_unref          (GWeatherTimezone *zone);
 
+G_DEFINE_AUTOPTR_CLEANUP_FUNC (GWeatherTimezone, gweather_timezone_unref);
+
 GWEATHER_EXTERN
 GWeatherTimezone *gweather_timezone_get_utc        (void);
 GWEATHER_EXTERN
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]