lib: Use OtTmpFile for static delta processing

The `OstreeRepoContentBareCommit` struct was basically an `OtTmpFile`, so let's
make it one. I moved the "convert to `GOutputStream`" logic into the callers,
since that bit can't fail; it makes the implementation much simpler since we can
just return the result of `ot_open_tmpfile_linkable_at()`.

Prep for `GLnxTmpfile` porting.

Closes: #957
Approved by: jlebon

src/libostree/ostree-repo-commit.c

@@ -446,57 +446,39 @@ ot_fallocate (int fd, goffset size, GError **error)
   return TRUE;
 }
 
+/* Combines a check for whether or not we already have the object with
+ * allocating a tempfile if we don't.  Used by the static delta code.
+ */
 gboolean
 _ostree_repo_open_content_bare (OstreeRepo          *self,
                                 const char          *checksum,
                                 guint64              content_len,
-                                OstreeRepoContentBareCommit *out_state,
+                                OtTmpfile           *out_tmpf,
-                                GOutputStream      **out_stream,
                                 gboolean            *out_have_object,
                                 GCancellable        *cancellable,
                                 GError             **error)
 {
-  gboolean ret = FALSE;
-  g_autofree char *temp_filename = NULL;
-  g_autoptr(GOutputStream) ret_stream = NULL;
   gboolean have_obj;
-
   if (!_ostree_repo_has_loose_object (self, checksum, OSTREE_OBJECT_TYPE_FILE, &have_obj,
                                       cancellable, error))
-    goto out;
+    return FALSE;
-
+  /* Do we already have this object? */
-  if (!have_obj)
-    {
-      int fd;
-
-      if (!glnx_open_tmpfile_linkable_at (self->tmp_dir_fd, ".", O_WRONLY|O_CLOEXEC,
-                                          &fd, &temp_filename, error))
-        goto out;
-
-      if (!ot_fallocate (fd, content_len, error))
-        goto out;
-
-      ret_stream = g_unix_output_stream_new (fd, TRUE);
-    }
-
-  ret = TRUE;
-  if (!have_obj)
-    {
-      out_state->temp_filename = temp_filename;
-      temp_filename = NULL;
-      out_state->fd = g_file_descriptor_based_get_fd ((GFileDescriptorBased*)ret_stream);
-      if (out_stream)
-        *out_stream = g_steal_pointer (&ret_stream);
-    }
   *out_have_object = have_obj;
- out:
+  if (have_obj)
-  return ret;
+    {
+      /* Make sure the tempfile is unset */
+      out_tmpf->initialized = 0;
+      return TRUE;
+    }
+
+  return ot_open_tmpfile_linkable_at (self->tmp_dir_fd, ".", O_WRONLY|O_CLOEXEC,
+                                      out_tmpf, error);
 }
 
 gboolean
 _ostree_repo_commit_trusted_content_bare (OstreeRepo          *self,
                                           const char          *checksum,
-                                          OstreeRepoContentBareCommit *state,
+                                          OtTmpfile           *tmpf,
                                           guint32              uid,
                                           guint32              gid,
                                           guint32              mode,
@@ -504,25 +486,22 @@ _ostree_repo_commit_trusted_content_bare (OstreeRepo          *self,
                                           GCancellable        *cancellable,
                                           GError             **error)
 {
-  gboolean ret = FALSE;
+  /* I don't think this is necessary, but a similar check was here previously,
+   * keeping it for extra redundancy.
+   */
+  if (!tmpf->initialized || tmpf->fd == -1)
+    return TRUE;
 
-  if (state->fd != -1)
+  if (!commit_loose_content_object (self, checksum,
-    {
+                                    tmpf->path,
-      if (!commit_loose_content_object (self, checksum,
+                                    FALSE, uid, gid, mode,
-                                        state->temp_filename,
+                                    xattrs, tmpf->fd,
-                                        FALSE, uid, gid, mode,
+                                    cancellable, error))
-                                        xattrs, state->fd,
+    return glnx_prefix_error (error, "Writing object %s.%s", checksum, ostree_object_type_to_string (OSTREE_OBJECT_TYPE_FILE));
-                                        cancellable, error))
+  /* The path was consumed */
-        {
+  g_clear_pointer (&tmpf->path, g_free);
-          g_prefix_error (error, "Writing object %s.%s: ", checksum, ostree_object_type_to_string (OSTREE_OBJECT_TYPE_FILE));
+  tmpf->initialized = FALSE;
-          goto out;
+  return TRUE;
-        }
-    }
-
-  ret = TRUE;
- out:
-  g_free (state->temp_filename);
-  return ret;
 }
 
 static gboolean

src/libostree/ostree-repo-private.h

@@ -23,7 +23,7 @@
 #include "ostree-ref.h"
 #include "ostree-repo.h"
 #include "ostree-remote-private.h"
-#include "libglnx.h"
+#include "otutil.h"
 
 G_BEGIN_DECLS
 
@@ -305,17 +305,11 @@ _ostree_repo_commit_loose_final (OstreeRepo        *self,
                                  GCancellable      *cancellable,
                                  GError           **error);
 
-typedef struct {
-  int fd;
-  char *temp_filename;
-} OstreeRepoContentBareCommit;
-
 gboolean
 _ostree_repo_open_content_bare (OstreeRepo          *self,
                                 const char          *checksum,
                                 guint64              content_len,
-                                OstreeRepoContentBareCommit *out_state,
+                                OtTmpfile           *out_tmpf,
-                                GOutputStream      **out_stream,
                                 gboolean            *out_have_object,
                                 GCancellable        *cancellable,
                                 GError             **error);
@@ -323,7 +317,7 @@ _ostree_repo_open_content_bare (OstreeRepo          *self,
 gboolean
 _ostree_repo_commit_trusted_content_bare (OstreeRepo          *self,
                                           const char          *checksum,
-                                          OstreeRepoContentBareCommit *state,
+                                          OtTmpfile           *tmpf,
                                           guint32              uid,
                                           guint32              gid,
                                           guint32              mode,

src/libostree/ostree-repo-pull.c

@@ -1108,7 +1108,7 @@ meta_fetch_on_complete (GObject           *object,
       goto out;
     }
 
-  /* Now delete it, keeping the fd open as the last reference); see comment in
+  /* Now delete it, keeping the fd open as the last reference; see comment in
    * corresponding content fetch path.
    */
   ot_cleanup_unlinkat (&tmp_unlinker);

src/libostree/ostree-repo-static-delta-processing.c

@@ -56,7 +56,7 @@ typedef struct {
   GError        **async_error;
 
   OstreeObjectType output_objtype;
-  OstreeRepoContentBareCommit barecommitstate;
+  OtTmpfile        tmpf;
   guint64          content_size;
   GOutputStream   *content_out;
   GChecksum       *content_checksum;
@@ -281,6 +281,8 @@ _ostree_static_delta_part_execute (OstreeRepo      *repo,
 
   ret = TRUE;
  out:
+  ot_tmpfile_clear (&state->tmpf);
+  g_clear_object (&state->content_out);
   g_clear_pointer (&state->content_checksum, g_checksum_free);
   return ret;
 }
@@ -419,8 +421,6 @@ do_content_open_generic (OstreeRepo                 *repo,
   if (!read_varuint64 (state, &xattr_offset, error))
     goto out;
 
-  state->barecommitstate.fd = -1;
-
   modev = g_variant_get_child_value (state->mode_dict, mode_offset);
   g_variant_get (modev, "(uuu)", &uid, &gid, &mode);
   state->uid = GUINT32_FROM_BE (uid);
@@ -608,14 +608,14 @@ dispatch_open_splice_and_close (OstreeRepo                 *repo,
         {
           if (!_ostree_repo_open_content_bare (repo, state->checksum,
                                                state->content_size,
-                                               &state->barecommitstate,
+                                               &state->tmpf,
-                                               &state->content_out,
                                                &state->have_obj,
                                                cancellable, error))
             goto out;
 
           if (!state->have_obj)
             {
+              state->content_out = g_unix_output_stream_new (state->tmpf.fd, FALSE);
               if (!handle_untrusted_content_checksum (repo, state, cancellable, error))
                 goto out;
 
@@ -711,11 +711,12 @@ dispatch_open (OstreeRepo                 *repo,
   
   if (!_ostree_repo_open_content_bare (repo, state->checksum,
                                        state->content_size,
-                                       &state->barecommitstate,
+                                       &state->tmpf,
-                                       &state->content_out,
                                        &state->have_obj,
                                        cancellable, error))
     goto out;
+  if (!state->have_obj)
+    state->content_out = g_unix_output_stream_new (state->tmpf.fd, FALSE);
 
   if (!handle_untrusted_content_checksum (repo, state, cancellable, error))
     goto out;
@@ -899,11 +900,12 @@ dispatch_close (OstreeRepo                 *repo,
             }
         }
 
-      if (!_ostree_repo_commit_trusted_content_bare (repo, state->checksum, &state->barecommitstate,
+      if (!_ostree_repo_commit_trusted_content_bare (repo, state->checksum, &state->tmpf,
                                                      state->uid, state->gid, state->mode,
                                                      state->xattrs,
                                                      cancellable, error))
         goto out;
+      g_clear_object (&state->content_out);
     }
 
   if (!dispatch_unset_read_source (repo, state, cancellable, error))
@@ -911,7 +913,6 @@ dispatch_close (OstreeRepo                 *repo,
       
   g_clear_pointer (&state->xattrs, g_variant_unref);
   g_clear_pointer (&state->content_checksum, g_checksum_free);
-  g_clear_object (&state->content_out);
   
   state->checksum_index++;
   state->output_target = NULL;