lib: Hoist unlinkat() cleanup API to fsutil, use in pull

The pull code also could make use of this in both the metadata and content
paths. I changed it to own the tempfile malloc (just like `GLnxTmpFile`), since
there's no reason to have different lifetimes for the filename and the file, and
that way we only have one variable rather than two.

The content path turns out to be a special case though, where
at least for mirroring archives, we directly pass the file *path*
down into `_ostree_repo_commit_loose_final()`.

This is prep for `GLnxTmpFile` porting.

Closes: #957
Approved by: jlebon

diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c
index 37e3d7e5613a3386bea039488e5ba063201ef809..32170ac7d510d6820e216174abd15117ff07f30f 100644 (file)
--- a/src/libostree/ostree-repo-commit.c
+++ b/src/libostree/ostree-repo-commit.c
@@ -567,23 +567,6 @@ create_regular_tmpfile_linkable_with_content (OstreeRepo *self,
   return TRUE;
 }
 
-/* A little helper to call unlinkat() as a cleanup
- * function.  Mostly only necessary to handle
- * deletion of temporary symlinks.
- */
-typedef struct {
-  int dfd;
-  const char *path;
-} CleanupUnlinkat;
-
-static void
-cleanup_unlinkat (CleanupUnlinkat *cleanup)
-{
-  if (cleanup->path)
-    (void) unlinkat (cleanup->dfd, cleanup->path, 0);
-}
-G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(CleanupUnlinkat, cleanup_unlinkat);
-
 /* Write a content object. */
 static gboolean
 write_content_object (OstreeRepo         *self,
@@ -651,9 +634,8 @@ write_content_object (OstreeRepo         *self,
    * temp_filename might also be a symlink.  Hence the CleanupUnlinkat
    * which handles that case.
    */
-  g_auto(CleanupUnlinkat) tmp_unlinker = { self->tmp_dir_fd, NULL };
+  g_auto(OtCleanupUnlinkat) tmp_unlinker = { self->tmp_dir_fd, NULL };
   glnx_fd_close int temp_fd = -1;
-  g_autofree char *temp_filename = NULL;
   gssize unpacked_size = 0;
   gboolean indexable = FALSE;
   if ((_ostree_repo_mode_is_bare (repo_mode)) && !phys_object_is_symlink)
@@ -661,10 +643,9 @@ write_content_object (OstreeRepo         *self,
       guint64 size = g_file_info_get_size (file_info);
 
       if (!create_regular_tmpfile_linkable_with_content (self, size, file_input,
-                                                         &temp_fd, &temp_filename,
+                                                         &temp_fd, &tmp_unlinker.path,
                                                          cancellable, error))
         return FALSE;
-      tmp_unlinker.path = temp_filename;
     }
   else if (_ostree_repo_mode_is_bare (repo_mode) && phys_object_is_symlink)
     {
@@ -672,10 +653,9 @@ write_content_object (OstreeRepo         *self,
          regular file and take the branch above */
       if (!_ostree_make_temporary_symlink_at (self->tmp_dir_fd,
                                               g_file_info_get_symlink_target (file_info),
-                                              &temp_filename,
+                                              &tmp_unlinker.path,
                                               cancellable, error))
         return FALSE;
-      tmp_unlinker.path = temp_filename;
     }
   else if (repo_mode == OSTREE_REPO_MODE_ARCHIVE_Z2)
     {
@@ -688,10 +668,9 @@ write_content_object (OstreeRepo         *self,
         indexable = TRUE;
 
       if (!glnx_open_tmpfile_linkable_at (self->tmp_dir_fd, ".", O_WRONLY|O_CLOEXEC,
-                                          &temp_fd, &temp_filename,
+                                          &temp_fd, &tmp_unlinker.path,
                                           error))
         return FALSE;
-      tmp_unlinker.path = temp_filename;
       temp_out = g_unix_output_stream_new (temp_fd, FALSE);
 
       file_meta = _ostree_zlib_file_header_new (file_info, xattrs);
@@ -758,15 +737,15 @@ write_content_object (OstreeRepo         *self,
   const guint32 gid = g_file_info_get_attribute_uint32 (file_info, "unix::gid");
   const guint32 mode = g_file_info_get_attribute_uint32 (file_info, "unix::mode");
   if (!commit_loose_content_object (self, actual_checksum,
-                                    temp_filename,
+                                    tmp_unlinker.path,
                                     object_file_type == G_FILE_TYPE_SYMBOLIC_LINK,
                                     uid, gid, mode,
                                     xattrs, temp_fd,
                                     cancellable, error))
     return glnx_prefix_error (error, "Writing object %s.%s", actual_checksum,
                               ostree_object_type_to_string (OSTREE_OBJECT_TYPE_FILE));
-  /* Clear the unlinker path, it was consumed */
-  tmp_unlinker.path = NULL;
+  /* Clear the unlinker, it was consumed */
+  ot_cleanup_unlinkat_clear (&tmp_unlinker);
 
   /* Update size metadata if configured */
   if (indexable && object_file_type == G_FILE_TYPE_REGULAR)

diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c
index 4c87199adbefeae47b7d2cb2d3fe49d7e75aa0f4..edfd8d4f62428fddcd249c8e4048d4f7ddf4548e 100644 (file)
--- a/src/libostree/ostree-repo-pull.c
+++ b/src/libostree/ostree-repo-pull.c
@@ -910,13 +910,13 @@ content_fetch_on_complete (GObject        *object,
   g_autoptr(GVariant) xattrs = NULL;
   g_autoptr(GInputStream) file_in = NULL;
   g_autoptr(GInputStream) object_input = NULL;
-  g_autofree char *temp_path = NULL;
+  g_auto(OtCleanupUnlinkat) tmp_unlinker = { _ostree_fetcher_get_dfd (fetcher), NULL };
   const char *checksum;
   g_autofree char *checksum_obj = NULL;
   OstreeObjectType objtype;
   gboolean free_fetch_data = TRUE;
 
-  if (!_ostree_fetcher_request_to_tmpfile_finish (fetcher, result, &temp_path, error))
+  if (!_ostree_fetcher_request_to_tmpfile_finish (fetcher, result, &tmp_unlinker.path, error))
     goto out;
 
   ostree_object_name_deserialize (fetch_data->object, &checksum, &objtype);
@@ -941,9 +941,11 @@ content_fetch_on_complete (GObject        *object,
       if (!have_object)
         {
           if (!_ostree_repo_commit_loose_final (pull_data->repo, checksum, OSTREE_OBJECT_TYPE_FILE,
-                                                _ostree_fetcher_get_dfd (fetcher), -1, temp_path,
+                                                tmp_unlinker.dfd, -1, tmp_unlinker.path,
                                                 cancellable, error))
             goto out;
+          /* The path was consumed */
+          ot_cleanup_unlinkat_clear (&tmp_unlinker);
         }
       pull_data->n_fetched_content++;
     }
@@ -952,20 +954,16 @@ content_fetch_on_complete (GObject        *object,
       /* Non-mirroring path */
 
       if (!ostree_content_file_parse_at (TRUE, _ostree_fetcher_get_dfd (fetcher),
-                                         temp_path, FALSE,
+                                         tmp_unlinker.path, FALSE,
                                          &file_in, &file_info, &xattrs,
                                          cancellable, error))
-        {
-          /* If it appears corrupted, delete it */
-          (void) unlinkat (_ostree_fetcher_get_dfd (fetcher), temp_path, 0);
-          goto out;
-        }
+        goto out;
 
       /* Also, delete it now that we've opened it, we'll hold
        * a reference to the fd.  If we fail to validate or write, then
        * the temp space will be cleaned up.
        */
-      (void) unlinkat (_ostree_fetcher_get_dfd (fetcher), temp_path, 0);
+      ot_cleanup_unlinkat (&tmp_unlinker);
 
       if (!validate_bareuseronly_mode (pull_data,
                                        checksum,
@@ -1046,7 +1044,7 @@ meta_fetch_on_complete (GObject           *object,
   FetchObjectData *fetch_data = user_data;
   OtPullData *pull_data = fetch_data->pull_data;
   g_autoptr(GVariant) metadata = NULL;
-  g_autofree char *temp_path = NULL;
+  g_auto(OtCleanupUnlinkat) tmp_unlinker = { _ostree_fetcher_get_dfd (fetcher), NULL };
   const char *checksum;
   g_autofree char *checksum_obj = NULL;
   OstreeObjectType objtype;
@@ -1060,7 +1058,7 @@ meta_fetch_on_complete (GObject           *object,
   g_debug ("fetch of %s%s complete", checksum_obj,
            fetch_data->is_detached_meta ? " (detached)" : "");
 
-  if (!_ostree_fetcher_request_to_tmpfile_finish (fetcher, result, &temp_path, error))
+  if (!_ostree_fetcher_request_to_tmpfile_finish (fetcher, result, &tmp_unlinker.path, error))
     {
       if (g_error_matches (local_error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND))
         {
@@ -1103,22 +1101,24 @@ meta_fetch_on_complete (GObject           *object,
   if (objtype == OSTREE_OBJECT_TYPE_TOMBSTONE_COMMIT)
     goto out;
 
-  fd = openat (_ostree_fetcher_get_dfd (fetcher), temp_path, O_RDONLY | O_CLOEXEC);
+  fd = openat (_ostree_fetcher_get_dfd (fetcher), tmp_unlinker.path, O_RDONLY | O_CLOEXEC);
   if (fd == -1)
     {
       glnx_set_error_from_errno (error);
       goto out;
     }
 
+  /* Now delete it, keeping the fd open as the last reference); see comment in
+   * corresponding content fetch path.
+   */
+  ot_cleanup_unlinkat (&tmp_unlinker);
+
   if (fetch_data->is_detached_meta)
     {
       if (!ot_util_variant_map_fd (fd, 0, G_VARIANT_TYPE ("a{sv}"),
                                    FALSE, &metadata, error))
         goto out;
 
-      /* Now delete it, see comment in corresponding content fetch path */
-      (void) unlinkat (_ostree_fetcher_get_dfd (fetcher), temp_path, 0);
-
       if (!ostree_repo_write_commit_detached_metadata (pull_data->repo, checksum, metadata,
                                                        pull_data->cancellable, error))
         goto out;
@@ -1136,8 +1136,6 @@ meta_fetch_on_complete (GObject           *object,
                                    FALSE, &metadata, error))
         goto out;
 
-      (void) unlinkat (_ostree_fetcher_get_dfd (fetcher), temp_path, 0);
-
       /* Write the commitpartial file now while we're still fetching data */
       if (objtype == OSTREE_OBJECT_TYPE_COMMIT)
         {

diff --git a/src/libotutil/ot-fs-utils.h b/src/libotutil/ot-fs-utils.h
index 26c5a49978a02b3c84c2fe9efb4b717bcb99c192..0f62cb7fe1cb13901188864a07f85137f85bd572 100644 (file)
--- a/src/libotutil/ot-fs-utils.h
+++ b/src/libotutil/ot-fs-utils.h
@@ -49,6 +49,34 @@ ot_link_tmpfile_at (OtTmpfile *tmpf,
                     const char *target,
                     GError **error);
 
+
+/* A little helper to call unlinkat() as a cleanup
+ * function.  Mostly only necessary to handle
+ * deletion of temporary symlinks.
+ */
+typedef struct {
+  int dfd;
+  char *path;
+} OtCleanupUnlinkat;
+
+static inline void
+ot_cleanup_unlinkat_clear (OtCleanupUnlinkat *cleanup)
+{
+  g_clear_pointer (&cleanup->path, g_free);
+}
+
+static inline void
+ot_cleanup_unlinkat (OtCleanupUnlinkat *cleanup)
+{
+  if (cleanup->path)
+    {
+      (void) unlinkat (cleanup->dfd, cleanup->path, 0);
+      ot_cleanup_unlinkat_clear (cleanup);
+    }
+}
+G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(OtCleanupUnlinkat, ot_cleanup_unlinkat);
+
+
 GFile * ot_fdrel_to_gfile (int dfd, const char *path);
 
 gboolean ot_readlinkat_gfile_info (int             dfd,