[gnome-photos] Don't steal the SPARQL string passed to photos_query_new

[Debarshi Ray <debarshir src gnome org>]

commit 6df0ff815a69d9cfae85b631ed7fd9fd2b481282
Author: Debarshi Ray <debarshir gnome org>
Date:   Wed Sep 13 18:57:28 2017 +0200

    Don't steal the SPARQL string passed to photos_query_new
    
    It is not idiomatic for strings to transfer ownership when passed as
    input arguments to functions, unless the function is explitly named
    as such. eg., "take" functions.
    
    Transferring the ownership of the SPARQL string to the PhotosQuery
    struct did afford us some conveniences. eg., not having to free the
    string. However, in a following patch, PhotosQuery is going to be
    converted into a GObject. It will be even more non-idiomatic to
    retain the current behaviour, and a future port to g_auto* will take
    care of the extra lines of code needed to free the string.
    
    That leaves the possibility of memory fragmentation caused by the
    extra copy. However, one can't say if that's a real issue without
    actual measurements. We don't really create that many PhotosQuery
    instances. In any case, this will be assuaged in the following patch
    where PhotosQuery instances will be made reference counted. That will
    remove the need to copy the sparql and tag fields, and reduce
    fragmentation.

 src/photos-query-builder.c          |   94 +++++++++++++++++++++++++----------
 src/photos-query.c                  |    4 +-
 src/photos-query.h                  |    2 +-
 src/photos-tracker-change-monitor.c |    2 +-
 src/photos-utils.c                  |    2 -
 5 files changed, 72 insertions(+), 32 deletions(-)
---
diff --git a/src/photos-query-builder.c b/src/photos-query-builder.c
index 781043f..d995a0f 100644
--- a/src/photos-query-builder.c
+++ b/src/photos-query-builder.c
@@ -204,7 +204,8 @@ PhotosQuery *
 photos_query_builder_create_collection_query (PhotosSearchContextState *state, const gchar *name)
 {
   GTimeVal tv;
-  gchar *sparql;
+  PhotosQuery *query;
+  gchar *sparql = NULL;
   gchar *time;
   gint64 timestamp;
 
@@ -221,69 +222,92 @@ photos_query_builder_create_collection_query (PhotosSearchContextState *state, c
                             name,
                             PHOTOS_QUERY_LOCAL_COLLECTIONS_IDENTIFIER,
                             name);
-  g_free (time);
 
-  return photos_query_new (state, sparql);
+  query = photos_query_new (state, sparql);
+
+  g_free (sparql);
+  g_free (time);
+  return query;
 }
 
 
 PhotosQuery *
 photos_query_builder_collection_icon_query (PhotosSearchContextState *state, const gchar *resource)
 {
-  gchar *sparql;
+  PhotosQuery *query;
+  gchar *sparql = NULL;
 
   sparql = g_strdup_printf ("SELECT ?urn "
                             "tracker:coalesce(nfo:fileLastModified(?urn), nie:contentLastModified(?urn)) AS 
?mtime "
                             "WHERE { ?urn nie:isPartOf <%s> } "
                             "ORDER BY DESC (?mtime) LIMIT 4",
                             resource);
-  return photos_query_new (state, sparql);
+
+  query = photos_query_new (state, sparql);
+
+  g_free (sparql);
+  return query;
 }
 
 
 PhotosQuery *
 photos_query_builder_count_query (PhotosSearchContextState *state, gint flags)
 {
-  gchar *sparql;
+  PhotosQuery *query;
+  gchar *sparql = NULL;
   gchar *where_sparql;
 
   where_sparql = photos_query_builder_where (state, TRUE, flags);
   sparql = g_strconcat ("SELECT DISTINCT COUNT(?urn) ", where_sparql, NULL);
-  g_free (where_sparql);
+  query = photos_query_new (state, sparql);
 
-  return photos_query_new (state, sparql);
+  g_free (sparql);
+  g_free (where_sparql);
+  return query;
 }
 
 
 PhotosQuery *
 photos_query_builder_delete_resource_query (PhotosSearchContextState *state, const gchar *resource)
 {
-  gchar *sparql;
+  PhotosQuery *query;
+  gchar *sparql = NULL;
 
   sparql = g_strdup_printf ("DELETE { <%s> a rdfs:Resource }", resource);
-  return photos_query_new (state, sparql);
+  query = photos_query_new (state, sparql);
+
+  g_free (sparql);
+  return query;
 }
 
 
 PhotosQuery *
 photos_query_builder_equipment_query (PhotosSearchContextState *state, GQuark equipment)
 {
+  PhotosQuery *query;
   const gchar *resource;
-  gchar *sparql;
+  gchar *sparql = NULL;
 
   resource = g_quark_to_string (equipment);
   sparql = g_strdup_printf ("SELECT nfo:manufacturer (<%s>) nfo:model (<%s>) WHERE {}", resource, resource);
-  return photos_query_new (state, sparql);
+  query = photos_query_new (state, sparql);
+
+  g_free (sparql);
+  return query;
 }
 
 
 PhotosQuery *
 photos_query_builder_fetch_collections_query (PhotosSearchContextState *state, const gchar *resource)
 {
-  gchar *sparql;
+  PhotosQuery *query;
+  gchar *sparql = NULL;
 
   sparql = g_strdup_printf ("SELECT ?urn WHERE { ?urn a nfo:DataContainer . <%s> nie:isPartOf ?urn }", 
resource);
-  return photos_query_new (state, sparql);
+  query = photos_query_new (state, sparql);
+
+  g_free (sparql);
+  return query;
 }
 
 
@@ -292,20 +316,28 @@ photos_query_builder_global_query (PhotosSearchContextState *state,
                                    gint flags,
                                    PhotosOffsetController *offset_cntrlr)
 {
-  gchar *sparql;
+  PhotosQuery *query;
+  gchar *sparql = NULL;
 
   sparql = photos_query_builder_query (state, TRUE, flags, offset_cntrlr);
-  return photos_query_new (state, sparql);
+  query = photos_query_new (state, sparql);
+
+  g_free (sparql);
+  return query;
 }
 
 
 PhotosQuery *
 photos_query_builder_location_query (PhotosSearchContextState *state, const gchar *location_urn)
 {
-  gchar *sparql;
+  PhotosQuery *query;
+  gchar *sparql = NULL;
 
   sparql = g_strdup_printf ("SELECT slo:latitude (<%s>) slo:longitude (<%s>) WHERE {}", location_urn, 
location_urn);
-  return photos_query_new (state, sparql);
+  query = photos_query_new (state, sparql);
+
+  g_free (sparql);
+  return query;
 }
 
 
@@ -315,13 +347,17 @@ photos_query_builder_set_collection_query (PhotosSearchContextState *state,
                                            const gchar *collection_urn,
                                            gboolean setting)
 {
-  gchar *sparql;
+  PhotosQuery *query;
+  gchar *sparql = NULL;
 
   sparql = g_strdup_printf ("%s { <%s> nie:isPartOf <%s> }",
                             setting ? "INSERT" : "DELETE",
                             item_urn,
                             collection_urn);
-  return photos_query_new (state, sparql);
+  query = photos_query_new (state, sparql);
+
+  g_free (sparql);
+  return query;
 }
 
 
@@ -329,8 +365,9 @@ PhotosQuery *
 photos_query_builder_single_query (PhotosSearchContextState *state, gint flags, const gchar *resource)
 {
   GRegex *regex;
+  PhotosQuery *query;
   gchar *replacement;
-  gchar *sparql;
+  gchar *sparql = NULL;
   gchar *tmp;
 
   tmp = photos_query_builder_query (state, FALSE, flags, NULL);
@@ -338,11 +375,13 @@ photos_query_builder_single_query (PhotosSearchContextState *state, gint flags,
   regex = g_regex_new ("\\?urn", 0, 0, NULL);
   replacement = g_strconcat ("<", resource, ">", NULL);
   sparql = g_regex_replace (regex, tmp, -1, 0, replacement, 0, NULL);
+  query = photos_query_new (state, sparql);
+
+  g_free (sparql);
   g_free (replacement);
   g_free (tmp);
   g_regex_unref (regex);
-
-  return photos_query_new (state, sparql);
+  return query;
 }
 
 
@@ -350,7 +389,8 @@ PhotosQuery *
 photos_query_builder_update_mtime_query (PhotosSearchContextState *state, const gchar *resource)
 {
   GTimeVal tv;
-  gchar *sparql;
+  PhotosQuery *query;
+  gchar *sparql = NULL;
   gchar *time;
   gint64 timestamp;
 
@@ -360,9 +400,11 @@ photos_query_builder_update_mtime_query (PhotosSearchContextState *state, const
   time = g_time_val_to_iso8601 (&tv);
 
   sparql = g_strdup_printf ("INSERT OR REPLACE { <%s> nie:contentLastModified '%s' }", resource, time);
-  g_free (time);
+  query = photos_query_new (state, sparql);
 
-  return photos_query_new (state, sparql);
+  g_free (sparql);
+  g_free (time);
+  return query;
 }
 
 
diff --git a/src/photos-query.c b/src/photos-query.c
index 9e4132f..869fef0 100644
--- a/src/photos-query.c
+++ b/src/photos-query.c
@@ -35,7 +35,7 @@ const gchar *PHOTOS_QUERY_LOCAL_COLLECTIONS_IDENTIFIER = "photos:collection:loca
 
 
 PhotosQuery *
-photos_query_new (PhotosSearchContextState *state, gchar *sparql)
+photos_query_new (PhotosSearchContextState *state, const gchar *sparql)
 {
   PhotosQuery *query;
 
@@ -50,7 +50,7 @@ photos_query_new (PhotosSearchContextState *state, gchar *sparql)
         query->source = PHOTOS_SOURCE (g_object_ref (active_object));
     }
 
-  query->sparql = sparql;
+  query->sparql = g_strdup (sparql);
 
   return query;
 }
diff --git a/src/photos-query.h b/src/photos-query.h
index 0a29e90..6bc14e2 100644
--- a/src/photos-query.h
+++ b/src/photos-query.h
@@ -81,7 +81,7 @@ struct _PhotosQuery
   gchar *tag;
 };
 
-PhotosQuery    *photos_query_new     (PhotosSearchContextState *state, gchar *sparql);
+PhotosQuery    *photos_query_new     (PhotosSearchContextState *state, const gchar *sparql);
 
 void            photos_query_set_tag (PhotosQuery *query, const gchar *tag);
 
diff --git a/src/photos-tracker-change-monitor.c b/src/photos-tracker-change-monitor.c
index 92a84d4..9cacd43 100644
--- a/src/photos-tracker-change-monitor.c
+++ b/src/photos-tracker-change-monitor.c
@@ -278,7 +278,7 @@ photos_tracker_change_monitor_process_events (PhotosTrackerChangeMonitor *self)
 
   g_string_append (sparql, " {}");
 
-  query = photos_query_new (NULL, g_strdup (sparql->str));
+  query = photos_query_new (NULL, sparql->str);
 
   data = photos_tracker_change_monitor_query_data_new (self, id_table, events);
   photos_tracker_queue_select (self->queue,
diff --git a/src/photos-utils.c b/src/photos-utils.c
index a6e2fe2..b5afbe8 100644
--- a/src/photos-utils.c
+++ b/src/photos-utils.c
@@ -1321,7 +1321,6 @@ photos_utils_set_edited_name (const gchar *urn, const gchar *title)
 
   sparql = g_strdup_printf ("INSERT OR REPLACE { <%s> nie:title \"%s\" }", urn, title);
   query = photos_query_new (NULL, sparql);
-  sparql = NULL;
 
   error = NULL;
   queue = photos_tracker_queue_dup_singleton (NULL, &error);
@@ -1353,7 +1352,6 @@ photos_utils_set_favorite (const gchar *urn, gboolean is_favorite)
                             (is_favorite) ? "INSERT OR REPLACE" : "DELETE",
                             urn);
   query = photos_query_new (NULL, sparql);
-  sparql = NULL;
 
   error = NULL;
   queue = photos_tracker_queue_dup_singleton (NULL, &error);