[sushi] libsushi: use g_autoptr/g_autofree for memory management

[Cosimo Cecchi <cosimoc src gnome org>]

commit e761e37ad3a6293e77eccb2f10e345fcf6f8a037
Author: Cosimo Cecchi <cosimoc gnome org>
Date:   Sat Jul 6 09:37:19 2019 -0700

    libsushi: use g_autoptr/g_autofree for memory management
    
    Makes us more robust to leaks and simplifies the code.

 src/libsushi/sushi-font-loader.c | 52 +++++++++------------------
 src/libsushi/sushi-font-widget.c | 54 +++++++++++-----------------
 src/libsushi/sushi-utils.c       | 78 +++++++++++++---------------------------
 3 files changed, 62 insertions(+), 122 deletions(-)
---
diff --git a/src/libsushi/sushi-font-loader.c b/src/libsushi/sushi-font-loader.c
index c612b41..f7cf1de 100644
--- a/src/libsushi/sushi-font-loader.c
+++ b/src/libsushi/sushi-font-loader.c
@@ -60,10 +60,13 @@ static void
 font_load_job_free (FontLoadJob *job)
 {
   g_clear_object (&job->file);
+  g_free (job->face_contents);
 
   g_slice_free (FontLoadJob, job);
 }
 
+G_DEFINE_AUTOPTR_CLEANUP_FUNC (FontLoadJob, font_load_job_free)
+
 static FT_Face
 create_face_from_contents (FontLoadJob *job,
                            gchar **contents,
@@ -79,34 +82,23 @@ create_face_from_contents (FontLoadJob *job,
                                  &retval);
 
   if (ft_error != 0) {
-    gchar *uri;
-    uri = g_file_get_uri (job->file);
+    g_autofree gchar *uri = g_file_get_uri (job->file);
     g_set_error (error, G_IO_ERROR, 0,
                  "Unable to read the font face file '%s'", uri);
-    retval = NULL;
-    g_free (job->face_contents);
-    g_free (uri);
-  } else {
-    *contents = job->face_contents;
+    return NULL;
   }
 
+  *contents = g_steal_pointer (&job->face_contents);
   return retval;
 }
 
-static void
+static gboolean
 font_load_job_do_load (FontLoadJob *job,
                        GError **error)
 {
-  gchar *contents;
-  gsize length;
-
-  g_file_load_contents (job->file, NULL,
-                        &contents, &length, NULL, error);
-
-  if ((error != NULL) && (*error == NULL)) {
-    job->face_contents = contents;
-    job->face_length = length;
-  }
+  return g_file_load_contents (job->file, NULL,
+                               &job->face_contents, &job->face_length,
+                               NULL, error);
 }
 
 static void
@@ -116,12 +108,12 @@ font_load_job (GTask *task,
                GCancellable *cancellable)
 {
   FontLoadJob *job = user_data;
-  GError *error = NULL;
+  g_autoptr(GError) error = NULL;
 
   font_load_job_do_load (job, &error);
 
   if (error != NULL)
-    g_task_return_error (task, error);
+    g_task_return_error (task, g_steal_pointer (&error));
   else
     g_task_return_boolean (task, TRUE);
 }
@@ -137,21 +129,11 @@ sushi_new_ft_face_from_uri (FT_Library library,
                             gchar **contents,
                             GError **error)
 {
-  FontLoadJob *job = NULL;
-  FT_Face face;
-
-  job = font_load_job_new (library, uri, face_index, NULL, NULL);
-  font_load_job_do_load (job, error);
-
-  if ((error != NULL) && (*error != NULL)) {
-    font_load_job_free (job);
+  g_autoptr(FontLoadJob) job = font_load_job_new (library, uri, face_index, NULL, NULL);
+  if (!font_load_job_do_load (job, error))
     return NULL;
-  }
 
-  face = create_face_from_contents (job, contents, error);
-  font_load_job_free (job);
-
-  return face;
+  return create_face_from_contents (job, contents, error);
 }
 
 /**
@@ -166,12 +148,10 @@ sushi_new_ft_face_from_uri_async (FT_Library library,
                                   gpointer user_data)
 {
   FontLoadJob *job = font_load_job_new (library, uri, face_index, callback, user_data);
-  GTask *task;
+  g_autoptr(GTask) task = g_task_new (NULL, NULL, callback, user_data);
 
-  task = g_task_new (NULL, NULL, callback, user_data);
   g_task_set_task_data (task, job, (GDestroyNotify) font_load_job_free);
   g_task_run_in_thread (task, font_load_job);
-  g_object_unref (task);
 }
 
 /**
diff --git a/src/libsushi/sushi-font-widget.c b/src/libsushi/sushi-font-widget.c
index 43b5a0c..144a3d6 100644
--- a/src/libsushi/sushi-font-widget.c
+++ b/src/libsushi/sushi-font-widget.c
@@ -173,11 +173,11 @@ text_extents (cairo_t *cr,
               const char *text,
               cairo_text_extents_t *extents)
 {
-  cairo_glyph_t *glyphs;
+  g_autofree cairo_glyph_t *glyphs = NULL;
   gint num_glyphs;
+
   text_to_glyphs (cr, text, &glyphs, &num_glyphs);
   cairo_glyph_extents (cr, glyphs, num_glyphs, extents);
-  g_free (glyphs);
 }
 
 /* adapted from gnome-utils:font-viewer/font-view.c
@@ -194,9 +194,9 @@ draw_string (SushiFontWidget *self,
             const gchar *text,
             gint *pos_y)
 {
+  g_autofree cairo_glyph_t *glyphs = NULL;
   cairo_font_extents_t font_extents;
   cairo_text_extents_t extents;
-  cairo_glyph_t *glyphs;
   GtkTextDirection text_dir;
   gint pos_x;
   gint num_glyphs;
@@ -227,8 +227,6 @@ draw_string (SushiFontWidget *self,
   cairo_move_to (cr, pos_x, *pos_y);
   cairo_show_glyphs (cr, glyphs, num_glyphs);
 
-  g_free (glyphs);
-
   *pos_y += LINE_SPACING / 2;
 }
 
@@ -236,30 +234,25 @@ static gboolean
 check_font_contain_text (FT_Face face,
                          const gchar *text)
 {
-  gunichar *string;
+  g_autofree gunichar *string = NULL;
   glong len, idx;
-  gboolean retval = TRUE;
 
   string = g_utf8_to_ucs4_fast (text, -1, &len);
   for (idx = 0; idx < len; idx++) {
     gunichar c = string[idx];
 
-    if (!FT_Get_Char_Index (face, c)) {
-      retval = FALSE;
-      break;
-    }
+    if (!FT_Get_Char_Index (face, c))
+      return FALSE;
   }
 
-  g_free (string);
-
-  return retval;
+  return TRUE;
 }
 
 static gchar *
 build_charlist_for_face (FT_Face face,
                          gint *length)
 {
-  GString *string;
+  g_autoptr(GString) string = NULL;
   gulong c;
   guint glyph;
   gint total_chars = 0;
@@ -277,16 +270,16 @@ build_charlist_for_face (FT_Face face,
   if (length)
     *length = total_chars;
 
-  return g_string_free (string, FALSE);
+  return g_strdup (string->str);
 }
 
 static gchar *
 random_string_from_available_chars (FT_Face face,
                                     gint n_chars)
 {
-  gchar *chars;
+  g_autofree gchar *chars = NULL;
+  g_autoptr(GString) retval = NULL;
   gint idx, rand, total_chars;
-  GString *retval;
   gchar *ptr, *end;
 
   idx = 0;
@@ -296,7 +289,7 @@ random_string_from_available_chars (FT_Face face,
     return NULL;
 
   if (total_chars <= n_chars)
-    return chars;
+    return g_steal_pointer (&chars);
 
   retval = g_string_new (NULL);
 
@@ -310,7 +303,7 @@ random_string_from_available_chars (FT_Face face,
     idx++;
   }
 
-  return g_string_free (retval, FALSE);
+  return g_strdup (retval->str);
 }
 
 static gboolean
@@ -456,7 +449,8 @@ sushi_font_widget_size_request (GtkWidget *drawing_area,
   cairo_text_extents_t extents;
   cairo_font_extents_t font_extents;
   cairo_font_face_t *font;
-  gint *sizes = NULL, n_sizes, alpha_size, title_size;
+  g_autofree gint *sizes = NULL;
+  gint n_sizes, alpha_size, title_size;
   cairo_t *cr;
   cairo_surface_t *surface;
   FT_Face face = self->face;
@@ -562,7 +556,6 @@ sushi_font_widget_size_request (GtkWidget *drawing_area,
   cairo_destroy (cr);
   cairo_font_face_destroy (font);
   cairo_surface_destroy (surface);
-  g_free (sizes);
 }
 
 static void
@@ -596,7 +589,8 @@ sushi_font_widget_draw (GtkWidget *drawing_area,
                         cairo_t *cr)
 {
   SushiFontWidget *self = SUSHI_FONT_WIDGET (drawing_area);
-  gint *sizes = NULL, n_sizes, alpha_size, title_size, pos_y = 0, i;
+  g_autofree gint *sizes = NULL;
+  gint n_sizes, alpha_size, title_size, pos_y = 0, i;
   cairo_font_face_t *font = NULL;
   FT_Face face = self->face;
   GtkStyleContext *context;
@@ -606,7 +600,7 @@ sushi_font_widget_draw (GtkWidget *drawing_area,
   gint allocated_width, allocated_height;
 
   if (face == NULL)
-    goto end;
+    return FALSE;
 
   context = gtk_widget_get_style_context (drawing_area);
   state = gtk_style_context_get_state (context);
@@ -668,9 +662,7 @@ sushi_font_widget_draw (GtkWidget *drawing_area,
   }
 
  end:
-  if (font != NULL)
-    cairo_font_face_destroy (font);
-  g_free (sizes);
+  cairo_font_face_destroy (font);
 
   return FALSE;
 }
@@ -681,7 +673,7 @@ font_face_async_ready_cb (GObject *object,
                           gpointer user_data)
 {
   SushiFontWidget *self = user_data;
-  GError *error = NULL;
+  g_autoptr(GError) error = NULL;
 
   self->face =
     sushi_new_ft_face_from_uri_finish (result,
@@ -691,7 +683,6 @@ font_face_async_ready_cb (GObject *object,
   if (error != NULL) {
     g_signal_emit (self, signals[ERROR], 0, error);
     g_print ("Can't load the font face: %s\n", error->message);
-    g_error_free (error);
 
     return;
   }
@@ -715,10 +706,7 @@ sushi_font_widget_load (SushiFontWidget *self)
 static void
 sushi_font_widget_init (SushiFontWidget *self)
 {
-  FT_Error err;
-
-  self->face = NULL;
-  err = FT_Init_FreeType (&self->library);
+  FT_Error err = FT_Init_FreeType (&self->library);
 
   if (err != FT_Err_Ok)
     g_error ("Unable to initialize FreeType");
diff --git a/src/libsushi/sushi-utils.c b/src/libsushi/sushi-utils.c
index 89e5c94..e215846 100644
--- a/src/libsushi/sushi-utils.c
+++ b/src/libsushi/sushi-utils.c
@@ -136,14 +136,13 @@ libreoffice_missing_ready_cb (GObject *source,
                               GAsyncResult *res,
                               gpointer user_data)
 {
-  GTask *task = user_data;
-  GError *error = NULL;
+  g_autoptr(GTask) task = user_data;
+  g_autoptr(GError) error = NULL;
 
   g_dbus_connection_call_finish (G_DBUS_CONNECTION (source), res, &error);
   if (error != NULL) {
     /* can't install libreoffice with packagekit - nothing else we can do */
-    g_task_return_error (task, error);
-    g_object_unref (task);
+    g_task_return_error (task, g_steal_pointer (&error));
     return;
   }
 
@@ -182,7 +181,7 @@ libreoffice_missing (GTask *task)
                           NULL, G_DBUS_CALL_FLAGS_NONE,
                           G_MAXINT, NULL,
                           libreoffice_missing_ready_cb,
-                          task);
+                          g_object_ref (task));
 }
 
 static void
@@ -190,16 +189,13 @@ libreoffice_child_watch_cb (GPid pid,
                             gint status,
                             gpointer user_data)
 {
-  GTask *task = user_data;
+  g_autoptr(GTask) task = user_data;
   TaskData *data = g_task_get_task_data (task);
-  GFile *file;
 
   g_spawn_close_pid (pid);
   data->libreoffice_pid = -1;
 
-  file = g_file_new_for_path (data->pdf_path);
-  g_task_return_pointer (task, file, g_object_unref);
-  g_object_unref (task);
+  g_task_return_pointer (task, g_file_new_for_path (data->pdf_path), g_object_unref);
 }
 
 #define LIBREOFFICE_FLATPAK "org.libreoffice.LibreOffice"
@@ -209,9 +205,9 @@ check_libreoffice_flatpak (GTask       *task,
                            const gchar *flatpak_path)
 {
   const gchar *check_argv[] = { flatpak_path, "info", LIBREOFFICE_FLATPAK, NULL };
+  g_autoptr(GError) error = NULL;
   gboolean ret;
   gint exit_status = -1;
-  GError *error = NULL;
   TaskData *data = g_task_get_task_data (task);
 
   if (data->checked_libreoffice_flatpak)
@@ -228,19 +224,17 @@ check_libreoffice_flatpak (GTask       *task,
                       &exit_status, &error);
 
   if (ret) {
-    GError *child_error = NULL;
+    g_autoptr(GError) child_error = NULL;
     if (g_spawn_check_exit_status (exit_status, &child_error)) {
       g_debug ("Found LibreOffice flatpak!");
       data->have_libreoffice_flatpak = TRUE;
     } else {
       g_debug ("LibreOffice flatpak not found, flatpak info returned %i (%s)",
                exit_status, child_error->message);
-      g_clear_error (&child_error);
     }
   } else {
     g_warning ("Error while checking for LibreOffice flatpak: %s",
                error->message);
-    g_clear_error (&error);
   }
 
   return data->have_libreoffice_flatpak;
@@ -249,14 +243,14 @@ check_libreoffice_flatpak (GTask       *task,
 static void
 load_libreoffice (GTask *task)
 {
-  gchar *flatpak_path, *libreoffice_path = NULL;
+  g_autofree gchar *flatpak_path = NULL, *libreoffice_path = NULL;
+  g_autofree gchar *doc_path = NULL, *doc_name = NULL, *tmp_name = NULL;
+  g_autofree gchar *command = NULL, *pdf_dir = NULL;
+  g_auto(GStrv) argv = NULL;
+  g_autoptr(GError) error = NULL;
   gboolean use_flatpak = FALSE;
-  gchar *doc_path, *doc_name, *tmp_name, *pdf_dir;
-  gchar *flatpak_doc = NULL, *flatpak_dir = NULL;
   gboolean res;
   GPid pid;
-  GError *error = NULL;
-  gchar **argv = NULL;
   TaskData *data = g_task_get_task_data (task);
 
   flatpak_path = g_find_program_in_path ("flatpak");
@@ -267,7 +261,6 @@ load_libreoffice (GTask *task)
     libreoffice_path = g_find_program_in_path ("libreoffice");
     if (libreoffice_path == NULL) {
       libreoffice_missing (task);
-      g_free (flatpak_path);
       return;
     }
   }
@@ -280,17 +273,14 @@ load_libreoffice (GTask *task)
   if (tmp_name)
     *tmp_name = '\0';
   tmp_name = g_strdup_printf ("%s.pdf", doc_name);
-  g_free (doc_name);
 
   pdf_dir = g_build_filename (g_get_user_cache_dir (), "sushi", NULL);
   data->pdf_path = g_build_filename (pdf_dir, tmp_name, NULL);
   g_mkdir_with_parents (pdf_dir, 0700);
 
-  g_free (tmp_name);
-
   if (use_flatpak) {
-    flatpak_doc = g_strdup_printf ("--filesystem=%s:ro", doc_path);
-    flatpak_dir = g_strdup_printf ("--filesystem=%s", pdf_dir);
+    g_autofree gchar *flatpak_doc = g_strdup_printf ("--filesystem=%s:ro", doc_path);
+    g_autofree gchar *flatpak_dir = g_strdup_printf ("--filesystem=%s", pdf_dir);
 
     const gchar *flatpak_argv[] = {
       NULL, /* to be replaced with flatpak binary */
@@ -328,32 +318,20 @@ load_libreoffice (GTask *task)
     argv = g_strdupv ((gchar **) libreoffice_argv);
   }
 
-  tmp_name = g_strjoinv (" ", (gchar **) argv);
-  g_debug ("Executing LibreOffice command: %s", tmp_name);
-  g_free (tmp_name);
+  command = g_strjoinv (" ", (gchar **) argv);
+  g_debug ("Executing LibreOffice command: %s", command);
 
   res = g_spawn_async (NULL, (gchar **) argv, NULL,
                        G_SPAWN_DO_NOT_REAP_CHILD,
                        NULL, NULL,
                        &pid, &error);
 
-  g_free (pdf_dir);
-  g_free (doc_path);
-  g_free (libreoffice_path);
-  g_free (flatpak_path);
-  g_free (flatpak_doc);
-  g_free (flatpak_dir);
-  g_strfreev (argv);
-
   if (!res) {
-    g_warning ("Error while spawning libreoffice: %s",
-               error->message);
-    g_error_free (error);
-
+    g_warning ("Error while spawning libreoffice: %s", error->message);
     return;
   }
 
-  g_child_watch_add (pid, libreoffice_child_watch_cb, task);
+  g_child_watch_add (pid, libreoffice_child_watch_cb, g_object_ref (task));
   data->libreoffice_pid = pid;
 }
 
@@ -396,9 +374,9 @@ GdkPixbuf *
 sushi_pixbuf_from_gst_sample (GstSample *sample,
                               GError   **error)
 {
+  g_autoptr(GdkPixbufLoader) loader = NULL;
   GstBuffer *buffer = gst_sample_get_buffer (sample);
   GdkPixbuf *pixbuf = NULL;
-  GdkPixbufLoader *loader;
   GstMapInfo info;
 
   if (!gst_buffer_map (buffer, &info, GST_MAP_READ)) {
@@ -416,7 +394,6 @@ sushi_pixbuf_from_gst_sample (GstSample *sample,
   if (pixbuf)
     g_object_ref (pixbuf);
 
-  g_object_unref (loader);
   gst_buffer_unmap (buffer, &info);
 
   return pixbuf;
@@ -458,13 +435,12 @@ fetch_uri_job (GTask *task,
                GCancellable *cancellable)
 {
   FetchUriTaskData *data = task_data;
+  g_autofree gchar *retval = NULL;
+  g_auto(GStrv) param_names = NULL, param_values = NULL;
   Mb5Metadata metadata;
   Mb5Query query;
   Mb5Release release;
   Mb5ReleaseList release_list;
-  gchar *retval = NULL;
-  gchar **param_names = NULL;
-  gchar **param_values = NULL;
 
   query = mb5_query_new ("sushi", NULL, 0);
 
@@ -509,10 +485,7 @@ fetch_uri_job (GTask *task,
                              G_IO_ERROR_NOT_FOUND, "%s",
                              "Error getting the ASIN from MusicBrainz");
   else
-    g_task_return_pointer (task, retval, g_free);
-
-  g_strfreev (param_names);
-  g_strfreev (param_values);
+    g_task_return_pointer (task, g_steal_pointer (&retval), g_free);
 }
 
 gchar *
@@ -528,10 +501,9 @@ sushi_get_asin_for_track (const gchar *artist,
                           GAsyncReadyCallback callback,
                           gpointer user_data)
 {
+  g_autoptr(GTask) task = g_task_new (NULL, NULL, callback, user_data);
   FetchUriTaskData *data = fetch_uri_task_data_new (artist, album);
-  GTask *task = g_task_new (NULL, NULL, callback, user_data);
-  g_task_set_task_data (task, data, fetch_uri_task_data_free);
 
+  g_task_set_task_data (task, data, fetch_uri_task_data_free);
   g_task_run_in_thread (task, fetch_uri_job);
-  g_object_unref (task);
 }