Define and use cleanup helpers for libarchive

This should fix some of the ASAN leaks around libarchive usage,
and is generally better.

Closes: #609
Approved by: jlebon

7 files changed, 61 insertions, 22 deletions

diff --git a/Makefile-libostree.am b/Makefile-libostree.am
index 0be60a18..d6ae6023 100644
--- a/Makefile-libostree.am
+++ b/Makefile-libostree.am
@@ -134,6 +134,7 @@ libostree_1_la_SOURCES = \
 if USE_LIBARCHIVE
 libostree_1_la_SOURCES += src/libostree/ostree-libarchive-input-stream.h \
 	src/libostree/ostree-libarchive-input-stream.c \
+	src/libostree/ostree-libarchive-private.h \
 	$(NULL)
 endif
 if HAVE_LIBSOUP_CLIENT_CERTS
diff --git a/src/libostree/ostree-libarchive-input-stream.h b/src/libostree/ostree-libarchive-input-stream.h
index bcfc7bd2..7f88693e 100644
--- a/src/libostree/ostree-libarchive-input-stream.h
+++ b/src/libostree/ostree-libarchive-input-stream.h
@@ -23,6 +23,7 @@
 #pragma once
 
 #include <gio/gio.h>
+#include "ostree-libarchive-private.h"
 
 G_BEGIN_DECLS
 
diff --git a/src/libostree/ostree-libarchive-private.h b/src/libostree/ostree-libarchive-private.h
new file mode 100644
index 00000000..177c6568
--- /dev/null
+++ b/src/libostree/ostree-libarchive-private.h
@@ -0,0 +1,42 @@
+/* -*- mode: C; c-file-style: "gnu"; indent-tabs-mode: nil; -*-
+ *
+ * Copyright (C) 2016 Colin Walters <walters@verbum.org>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General
+ * Public License along with this library; if not, write to the
+ * Free Software Foundation, Inc., 59 Temple Place, Suite 330,
+ * Boston, MA 02111-1307, USA.
+ *
+ * Author: Alexander Larsson <alexl@redhat.com>
+ */
+
+#pragma once
+
+#include <gio/gio.h>
+#include "libglnx.h"
+#ifdef HAVE_LIBARCHIVE
+#include <archive.h>
+#include <archive_entry.h>
+#endif
+
+G_BEGIN_DECLS
+
+#ifdef HAVE_LIBARCHIVE
+GLNX_DEFINE_CLEANUP_FUNCTION (void *, flatpak_local_free_write_archive, archive_write_free)
+#define ot_cleanup_write_archive __attribute__((cleanup (flatpak_local_free_write_archive)))
+
+GLNX_DEFINE_CLEANUP_FUNCTION (void *, flatpak_local_free_read_archive, archive_read_free)
+#define ot_cleanup_read_archive __attribute__((cleanup (flatpak_local_free_read_archive)))
+#endif
+
+G_END_DECLS
diff --git a/src/libostree/ostree-repo-libarchive.c b/src/libostree/ostree-repo-libarchive.c
index 02a1180a..01982894 100644
--- a/src/libostree/ostree-repo-libarchive.c
+++ b/src/libostree/ostree-repo-libarchive.c
@@ -913,10 +913,9 @@ ostree_repo_write_archive_to_mtree (OstreeRepo                *self,
 {
 #ifdef HAVE_LIBARCHIVE
   gboolean ret = FALSE;
-  struct archive *a = NULL;
+  ot_cleanup_read_archive struct archive *a = archive_read_new ();
   OstreeRepoImportArchiveOptions opts = { 0, };
 
-  a = archive_read_new ();
 #ifdef HAVE_ARCHIVE_READ_SUPPORT_FILTER_ALL
   archive_read_support_filter_all (a);
 #else
@@ -945,7 +944,6 @@ ostree_repo_write_archive_to_mtree (OstreeRepo                *self,
   if (a)
     {
       (void)archive_read_close (a);
-      (void)archive_read_free (a);
     }
   return ret;
 #else
diff --git a/src/ostree/ot-builtin-export.c b/src/ostree/ot-builtin-export.c
index db4dbc31..b0bb40fa 100644
--- a/src/ostree/ot-builtin-export.c
+++ b/src/ostree/ot-builtin-export.c
@@ -24,6 +24,7 @@
 #include "ot-builtins.h"
 #include "ostree.h"
 #include "ostree-repo-file.h"
+#include "ostree-libarchive-private.h"
 #include "otutil.h"
 
 #ifdef HAVE_LIBARCHIVE
@@ -67,7 +68,7 @@ ostree_builtin_export (int argc, char **argv, GCancellable *cancellable, GError
   g_autoptr(GFile) subtree = NULL;
   g_autofree char *commit = NULL;
   g_autoptr(GVariant) commit_data = NULL;
-  struct archive *a = NULL;
+  ot_cleanup_write_archive struct archive *a = NULL;
   OstreeRepoExportArchiveOptions opts = { 0, };
 
   context = g_option_context_new ("COMMIT - Stream COMMIT to stdout in tar format");
@@ -154,10 +155,6 @@ ostree_builtin_export (int argc, char **argv, GCancellable *cancellable, GError
   
   ret = TRUE;
  out:
-#ifdef HAVE_LIBARCHIVE  
-  if (a)
-    archive_write_free (a);
-#endif
   if (context)
     g_option_context_free (context);
   return ret;
diff --git a/tests/test-libarchive-import.c b/tests/test-libarchive-import.c
index 254d4143..e9be7f66 100644
--- a/tests/test-libarchive-import.c
+++ b/tests/test-libarchive-import.c
@@ -26,6 +26,7 @@
 #include <string.h>
 
 #include <ostree.h>
+#include "ostree-libarchive-private.h"
 #include <archive.h>
 #include <archive_entry.h>
 
@@ -40,7 +41,7 @@ static void
 test_data_init (TestData *td)
 {
   GError *error = NULL;
-  struct archive *a = archive_write_new ();
+  ot_cleanup_write_archive struct archive *a = archive_write_new ();
   struct archive_entry *ae;
   uid_t uid = getuid ();
   gid_t gid = getgid ();
@@ -115,12 +116,12 @@ test_data_init (TestData *td)
   archive_entry_free (ae);
 
   g_assert_cmpint (ARCHIVE_OK, ==, archive_write_close (a));
-  g_assert_cmpint (ARCHIVE_OK, ==, archive_write_free (a));
 
   td->fd_empty = openat (AT_FDCWD, "empty.tar.gz", O_CREAT | O_EXCL | O_RDWR | O_CLOEXEC, 0644);
   g_assert (td->fd_empty >= 0);
   (void) unlink ("empty.tar.gz");
 
+  g_assert_cmpint (ARCHIVE_OK, ==, archive_write_free (a));
   a = archive_write_new ();
   g_assert (a);
 
@@ -128,7 +129,6 @@ test_data_init (TestData *td)
   g_assert_cmpint (0, ==, archive_write_add_filter_gzip (a));
   g_assert_cmpint (0, ==, archive_write_open_fd (a, td->fd_empty));
   g_assert_cmpint (ARCHIVE_OK, ==, archive_write_close (a));
-  g_assert_cmpint (ARCHIVE_OK, ==, archive_write_free (a));
 
   { g_autoptr(GFile) repopath = g_file_new_for_path ("repo");
     td->repo = ostree_repo_new (repopath);
@@ -165,7 +165,7 @@ test_libarchive_noautocreate_empty (gconstpointer data)
 {
   TestData *td = (void*)data;
   GError *error = NULL;
-  struct archive *a = archive_read_new ();
+  ot_cleanup_read_archive struct archive *a = archive_read_new ();
   OstreeRepoImportArchiveOptions opts = { 0, };
   glnx_unref_object OstreeMutableTree *mtree = ostree_mutable_tree_new ();
 
@@ -181,7 +181,7 @@ test_libarchive_autocreate_empty (gconstpointer data)
 {
   TestData *td = (void*)data;
   g_autoptr(GError) error = NULL;
-  struct archive *a = archive_read_new ();
+  ot_cleanup_read_archive struct archive *a = archive_read_new ();
   OstreeRepoImportArchiveOptions opts = { 0, };
   glnx_unref_object OstreeMutableTree *mtree = ostree_mutable_tree_new ();
 
@@ -199,7 +199,7 @@ test_libarchive_error_device_file (gconstpointer data)
 {
   TestData *td = (void*)data;
   g_autoptr(GError) error = NULL;
-  struct archive *a = archive_read_new ();
+  ot_cleanup_read_archive struct archive *a = archive_read_new ();
   OstreeRepoImportArchiveOptions opts = { 0, };
   glnx_unref_object OstreeMutableTree *mtree = ostree_mutable_tree_new ();
 
@@ -207,6 +207,7 @@ test_libarchive_error_device_file (gconstpointer data)
 
   (void)ostree_repo_import_archive_to_mtree (td->repo, &opts, a, mtree, NULL, NULL, &error);
   g_assert (error != NULL);
+  g_clear_error (&error);
 }
 
 static gboolean
@@ -268,8 +269,8 @@ static void
 test_libarchive_ignore_device_file (gconstpointer data)
 {
   TestData *td = (void*)data;
-  GError *error = NULL;
-  struct archive *a = archive_read_new ();
+  g_autoptr(GError) error = NULL;
+  ot_cleanup_read_archive struct archive *a = archive_read_new ();
   OstreeRepoImportArchiveOptions opts = { 0, };
 
   if (skip_if_no_xattr (td))
@@ -331,7 +332,7 @@ test_libarchive_ostree_convention (gconstpointer data)
 {
   TestData *td = (void*)data;
   GError *error = NULL;
-  struct archive *a = archive_read_new ();
+  ot_cleanup_read_archive struct archive *a = archive_read_new ();
   OstreeRepoImportArchiveOptions opts = { 0, };
 
   if (skip_if_no_xattr (td))
@@ -373,7 +374,7 @@ test_libarchive_xattr_callback (gconstpointer data)
 {
   TestData *td = (void*)data;
   GError *error = NULL;
-  struct archive *a = archive_read_new ();
+  ot_cleanup_read_archive struct archive *a = archive_read_new ();
   OstreeRepoImportArchiveOptions opts = { 0 };
   OstreeRepoCommitModifier *modifier = NULL;
   char buf[7] = { 0 };
@@ -428,7 +429,7 @@ static void
 entry_pathname_test_helper (gconstpointer data, gboolean on)
 {
   TestData *td = (void*)data; GError *error = NULL;
-  struct archive *a = archive_read_new ();
+  ot_cleanup_read_archive struct archive *a = archive_read_new ();
   OstreeRepoImportArchiveOptions opts = { 0, };
   OstreeRepoCommitModifier *modifier = NULL;
   gboolean met_etc_file = FALSE;
@@ -468,7 +469,6 @@ entry_pathname_test_helper (gconstpointer data, gboolean on)
       goto out;
     }
 
-  archive_read_free (a);
   ostree_repo_commit_modifier_unref (modifier);
  out:
   g_assert_no_error (error);
@@ -491,7 +491,7 @@ test_libarchive_selinux (gconstpointer data)
 {
   TestData *td = (void*)data;
   GError *error = NULL;
-  struct archive *a = archive_read_new ();
+  ot_cleanup_read_archive struct archive *a = archive_read_new ();
   OstreeRepoImportArchiveOptions opts = { 0 };
   glnx_unref_object OstreeSePolicy *sepol = NULL;
   OstreeRepoCommitModifier *modifier = NULL;
@@ -536,7 +536,6 @@ test_libarchive_selinux (gconstpointer data)
   g_assert_cmpstr (buf, ==, "system_u:object_r:etc_t:s0");
 
  out:
-  archive_read_free (a);
   if (modifier)
     ostree_repo_commit_modifier_unref (modifier);
   g_assert_no_error (error);
diff --git a/tests/test-ot-opt-utils.c b/tests/test-ot-opt-utils.c
index 7f334c36..1a7230b3 100644
--- a/tests/test-ot-opt-utils.c
+++ b/tests/test-ot-opt-utils.c
@@ -25,6 +25,7 @@
 #include <gio/gio.h>
 #include <string.h>
 #include "ot-opt-utils.h"
+#include "libglnx.h"
 
 static GString *printerr_str = NULL;