[gcab] Use g_autoptr() to fix several memory leaks on error in the library

[Richard Hughes <rhughes src gnome org>]

commit 01d27fc1db4c7e8727ecd837f9ad748a059ec679
Author: Richard Hughes <richard hughsie com>
Date:   Thu Dec 14 12:11:23 2017 +0000

    Use g_autoptr() to fix several memory leaks on error in the library

 libgcab/gcab-cabinet.c |   77 ++++++++++++++++++-----------------------------
 libgcab/gcab-folder.c  |   62 +++++++++++++-------------------------
 2 files changed, 52 insertions(+), 87 deletions(-)
---
diff --git a/libgcab/gcab-cabinet.c b/libgcab/gcab-cabinet.c
index b9a21f1..a1a64c4 100644
--- a/libgcab/gcab-cabinet.c
+++ b/libgcab/gcab-cabinet.c
@@ -1,6 +1,7 @@
 /*
  * LibGCab
  * Copyright (c) 2012, Marc-André Lureau <marcandre lureau gmail com>
+ * Copyright (c) 2017, Richard Hughes <richard hughsie com>
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -76,7 +77,8 @@ gcab_cabinet_finalize (GObject *object)
         g_byte_array_unref (self->reserved);
     if (self->signature)
         g_byte_array_unref (self->signature);
-    g_clear_object (&self->stream);
+    if (self->stream)
+        g_object_unref (self->stream);
 
     G_OBJECT_CLASS (gcab_cabinet_parent_class)->finalize (object);
 }
@@ -237,15 +239,13 @@ gcab_cabinet_write (GCabCabinet *self,
 
     GCabFolder *cabfolder = g_ptr_array_index (self->folders, 0);
     gsize nfiles = gcab_folder_get_nfiles (cabfolder);
-    GInputStream *in = NULL;
-    GDataOutputStream *dstream = NULL;
-    gboolean success = FALSE;
+    g_autoptr(GDataOutputStream) dstream = NULL;
     gssize len, offset = 0;
     cdata_t block = { 0, };
     guint8 data[DATABLOCKSIZE];
     gsize written;
     size_t sumstr = 0;
-    GSList *files;
+    g_autoptr(GSList) files = NULL;
     GCabFile *prevf = NULL;
 
     dstream = g_data_output_stream_new (out);
@@ -273,51 +273,52 @@ gcab_cabinet_write (GCabCabinet *self,
     /* avoid seeking to allow growing output streams */
     for (guint i = 0; i < folder.offsetdata; i++)
         if (!g_data_output_stream_put_byte (dstream, 0, cancellable, error))
-            goto end;
+            return FALSE;
 
     for (GSList *l = files; l != NULL; l = l->next) {
+        g_autoptr(GInputStream) in = NULL;
+
         GCabFile *file = GCAB_FILE (l->data);
         if (file_callback)
             file_callback (file, user_data);
 
-        g_clear_object (&in);
         in = G_INPUT_STREAM (g_file_read (gcab_file_get_gfile (file), cancellable, error));
         if (in == NULL)
-            goto end;
+            return FALSE;
 
         while ((len = g_input_stream_read (in,
                                            &data[offset], DATABLOCKSIZE - offset,
                                            cancellable, error)) == (DATABLOCKSIZE - offset)) {
             if (!cdata_write (&block, dstream, folder.typecomp, data, DATABLOCKSIZE, &written, cancellable, 
error))
-                goto end;
+                return FALSE;
             header.size += written;
             offset = 0;
         }
 
         if (len == -1)
-            goto end;
+            return FALSE;
 
         offset += len;
     }
     if (offset != 0) {
         if (!cdata_write (&block, dstream, folder.typecomp, data, offset, &written, cancellable, error))
-            goto end;
+            return FALSE;
         header.size += written;
     }
 
     if (!g_seekable_seek (G_SEEKABLE (out), 0,
                           G_SEEK_SET, cancellable, error))
-        goto end;
+        return FALSE;
 
     header.nfiles = nfiles;
     header.size += header.offsetfiles + nfiles * 16; /* 1st part cfile struct = 16 bytes */
     header.size += sumstr;
 
     if (!cheader_write (&header, dstream, cancellable, error))
-        goto end;
+        return FALSE;
 
     if (!cfolder_write (&folder, dstream, cancellable, error))
-        goto end;
+        return FALSE;
 
     for (GSList *l = files; l != NULL; l = l->next) {
         GCabFile *file = GCAB_FILE (l->data);
@@ -329,17 +330,10 @@ gcab_cabinet_write (GCabCabinet *self,
             gcab_file_add_attribute (file, GCAB_FILE_ATTRIBUTE_NAME_IS_UTF);
 
         if (!cfile_write (gcab_file_get_cfile (file), dstream, cancellable, error))
-            goto end;
+            return FALSE;
     }
 
-    success = TRUE;
-
-end:
-    g_clear_object (&dstream);
-    g_clear_object (&in);
-    g_slist_free (files);
-
-    return success;
+    return TRUE;
 }
 
 /**
@@ -408,16 +402,13 @@ gcab_cabinet_load (GCabCabinet *self,
 
     self->stream = g_object_ref (stream);
 
-    gboolean success = FALSE;
     cheader_t cheader;
-    GDataInputStream *in = g_data_input_stream_new (stream);
+    g_autoptr(GDataInputStream) in = g_data_input_stream_new (stream);
     g_filter_input_stream_set_close_base_stream (G_FILTER_INPUT_STREAM (in), FALSE);
     g_data_input_stream_set_byte_order (in, G_DATA_STREAM_BYTE_ORDER_LITTLE_ENDIAN);
 
-    GPtrArray *folders = self->folders;
-
     if (!cheader_read (&cheader, in, cancellable, error))
-        goto end;
+        return FALSE;
 
     if (cheader.reserved)
         g_object_set (self, "reserved",
@@ -427,48 +418,40 @@ gcab_cabinet_load (GCabCabinet *self,
     for (guint i = 0; i < cheader.nfolders; i++) {
         cfolder_t cfolder = { 0, };
         if (!cfolder_read (&cfolder, cheader.res_folder, in, cancellable, error))
-            goto end;
+            return FALSE;
 
         GCabFolder *folder = gcab_folder_new_with_cfolder (&cfolder, stream);
         if (cfolder.reserved)
             g_object_set (folder, "reserved",
                           g_byte_array_new_take (cfolder.reserved, cheader.res_folder),
                           NULL);
-        g_ptr_array_add (folders, folder);
+        g_ptr_array_add (self->folders, folder);
         cfolder.reserved = NULL;
     }
 
     for (guint i = 0; i < cheader.nfiles; i++) {
         cfile_t cfile = { 0, };
         if (!cfile_read (&cfile, in, cancellable, error))
-            goto end;
+            return FALSE;
 
-        if (cfile.index >= folders->len) {
+        if (cfile.index >= self->folders->len) {
             g_set_error (error, GCAB_ERROR, GCAB_ERROR_FORMAT,
                          "Invalid folder index");
-            goto end;
+            return FALSE;
         }
 
-        GCabFolder *folder = g_ptr_array_index (folders, cfile.index);
+        GCabFolder *folder = g_ptr_array_index (self->folders, cfile.index);
         if (folder == NULL) {
             g_set_error (error, GCAB_ERROR, GCAB_ERROR_FORMAT,
                          "Invalid folder pointer");
-            goto end;
+            return FALSE;
         }
 
-        GCabFile *file = gcab_file_new_with_cfile (&cfile);
-        if (!gcab_folder_add_file (folder, file, FALSE, cancellable, error)) {
-            g_object_unref (file);
-            goto end;
-        }
+        g_autoptr(GCabFile) file = gcab_file_new_with_cfile (&cfile);
+        if (!gcab_folder_add_file (folder, file, FALSE, cancellable, error))
+            return FALSE;
     }
-
-    success = TRUE;
-
-end:
-    if (in)
-        g_object_unref (in);
-    return success;
+    return TRUE;
 }
 
 /**
diff --git a/libgcab/gcab-folder.c b/libgcab/gcab-folder.c
index 8932a14..b790178 100644
--- a/libgcab/gcab-folder.c
+++ b/libgcab/gcab-folder.c
@@ -1,6 +1,7 @@
 /*
  * LibGCab
  * Copyright (c) 2012, Marc-André Lureau <marcandre lureau gmail com>
+ * Copyright (c) 2017, Richard Hughes <richard hughsie com>
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -208,33 +209,25 @@ add_file_info (GCabFolder *self, GCabFile *file, GFileInfo *info,
         if (!recurse)
             return TRUE;
 
-        GFileEnumerator *dir = g_file_enumerate_children (gcab_file_get_gfile (file), FILE_ATTRS, 0, NULL, 
error);
-        if (*error) {
+        g_autoptr(GFileEnumerator) dir = g_file_enumerate_children (gcab_file_get_gfile (file), FILE_ATTRS, 
0, NULL, error);
+        if (dir == NULL) {
             g_warning ("Couldn't enumerate directory %s: %s", name, (*error)->message);
             g_clear_error (error);
             return TRUE;
         }
 
         while ((info = g_file_enumerator_next_file (dir, NULL, error)) != NULL) {
-            GFile *child = g_file_get_child (gcab_file_get_gfile (file), g_file_info_get_name (info));
-            gchar *child_name = g_build_path ("\\", name, g_file_info_get_name (info), NULL);
-            GCabFile *child_file = gcab_file_new_with_file (child_name, child);
-
-            add_file_info (self, child_file, info, child_name, recurse, error);
-            if (*error) {
+            g_autoptr(GFile) child = g_file_get_child (gcab_file_get_gfile (file), g_file_info_get_name 
(info));
+            g_autofree gchar *child_name = g_build_path ("\\", name, g_file_info_get_name (info), NULL);
+            g_autoptr(GCabFile) child_file = gcab_file_new_with_file (child_name, child);
+            if (!add_file_info (self, child_file, info, child_name, recurse, error)) {
                 g_warning ("Couldn't add file %s: %s",
                            child_name, (*error)->message);
                 g_clear_error (error);
             }
-
-            g_object_unref (child_file);
-            g_free (child_name);
-            g_object_unref (child);
             g_object_unref (info);
         }
 
-        g_object_unref (dir);
-
     } else if (file_type == G_FILE_TYPE_REGULAR) {
         gcab_file_update_info (file, info);
         if (!add_file (self, file, error))
@@ -277,13 +270,12 @@ gcab_folder_add_file (GCabFolder *self, GCabFile *file,
     if (gfile) {
         g_return_val_if_fail (G_IS_FILE (gfile), FALSE);
 
-        GFileInfo *info = g_file_query_info (gfile, FILE_ATTRS, 0, NULL, error);
+        g_autoptr(GFileInfo) info = g_file_query_info (gfile, FILE_ATTRS, 0, NULL, error);
         if (info == NULL)
             return FALSE;
 
         success = add_file_info (self, file, info,
                                  gcab_file_get_name (file), recurse, error);
-        g_object_unref (info);
     } else {
         success = add_file (self, file, error);
     }
@@ -375,9 +367,10 @@ gcab_folder_extract (GCabFolder *self,
 {
     GError *my_error = NULL;
     gboolean success = FALSE;
-    GDataInputStream *data = NULL;
-    GFileOutputStream *out = NULL;
-    GSList *f, *files = NULL;
+    g_autoptr(GDataInputStream) data = NULL;
+    g_autoptr(GFileOutputStream) out = NULL;
+    GSList *f = NULL;
+    g_autoptr(GSList) files = NULL;
     cdata_t cdata = { 0, };
     guint32 nubytes = 0;
 
@@ -396,48 +389,41 @@ gcab_folder_extract (GCabFolder *self,
         if (file_callback && !file_callback (file, callback_data))
             continue;
 
-        gchar *fname = g_strdup (gcab_file_get_extract_name (file));
+        g_autofree gchar *fname = g_strdup (gcab_file_get_extract_name (file));
         int i = 0, len = strlen (fname);
         for (i = 0; i < len; i++)
             if (fname[i] == '\\')
                 fname[i] = '/';
 
-        GFile *gfile = g_file_resolve_relative_path (path, fname);
-        g_free (fname);
+        g_autoptr(GFile) gfile = g_file_resolve_relative_path (path, fname);
 
         if (!g_file_has_prefix (gfile, path)) {
             // "Rebase" the file in the given path, to ensure we never escape it
-            char *rawpath = g_file_get_path (gfile);
+            g_autofree gchar *rawpath = g_file_get_path (gfile);
             if (rawpath != NULL) {
                 char *newpath = rawpath;
                 while (*newpath != 0 && *newpath == G_DIR_SEPARATOR) {
                     newpath++;
                 }
-                GFile *newgfile = g_file_resolve_relative_path (path, newpath);
-                g_free (rawpath);
-                g_object_unref (gfile);
-                gfile = newgfile;
+                g_autoptr(GFile) newgfile = g_file_resolve_relative_path (path, newpath);
+                g_set_object (&gfile, newgfile);
             }
         }
 
-        GFile *parent = g_file_get_parent (gfile);
+        g_autoptr(GFile) parent = g_file_get_parent (gfile);
 
         if (!g_file_make_directory_with_parents (parent, cancellable, &my_error)) {
             if (g_error_matches (my_error, G_IO_ERROR, G_IO_ERROR_EXISTS))
                 g_clear_error (&my_error);
             else {
-                g_object_unref (gfile);
-                g_object_unref (parent);
                 g_propagate_error (error, my_error);
                 goto end;
             }
         }
-        g_object_unref (parent);
 
-        g_clear_object (&out);
-        out = g_file_replace (gfile, NULL, FALSE, G_FILE_CREATE_REPLACE_DESTINATION, cancellable, error);
-        g_object_unref (gfile);
-        if (!out)
+        g_autoptr(GFileOutputStream) out2 = NULL;
+        out2 = g_file_replace (gfile, NULL, FALSE, G_FILE_CREATE_REPLACE_DESTINATION, cancellable, error);
+        if (!out2)
             goto end;
 
         guint32 usize = gcab_file_get_usize (file);
@@ -464,7 +450,7 @@ gcab_folder_extract (GCabFolder *self,
                     gcab_file_get_uoffset (file) - nubytes : 0;
                 const void *p = &cdata.out[offset];
                 gsize count = MIN (usize, cdata.nubytes - offset);
-                if (!g_output_stream_write_all (G_OUTPUT_STREAM (out), p, count,
+                if (!g_output_stream_write_all (G_OUTPUT_STREAM (out2), p, count,
                                                 NULL, cancellable, error))
                     goto end;
                 usize -= count;
@@ -476,10 +462,6 @@ gcab_folder_extract (GCabFolder *self,
     success = TRUE;
 
 end:
-    g_slist_free (files);
-
-    g_clear_object (&data);
-    g_clear_object (&out);
     cdata_finish (&cdata, NULL);
 
     return success;