lib/repo: Split archive/bare file parsing
authorColin Walters <walters@verbum.org>
Thu, 22 Jun 2017 17:03:14 +0000 (13:03 -0400)
committerAtomic Bot <atomic-devel@projectatomic.io>
Fri, 23 Jun 2017 14:11:36 +0000 (14:11 +0000)
Prep for future cleanup patches (in particular I want an internal-only
version at first that uses a fd+`struct stat`) to avoid allocations.

The new version avoids lots of deep nesting of conditionals as well
by hoisting the "not found" handling to an early return.

There's a bit of code duplication between the two cases but it's
quite worth the result.

Closes: #951
Approved by: jlebon

src/libostree/ostree-repo.c

index d1253da5521dffa0b4176340060fc6b9b438c08e..ec52dc0917174f6e7dcaff2cf00e05c4f353e532 100644 (file)
@@ -2614,225 +2614,196 @@ _ostree_repo_read_bare_fd (OstreeRepo           *self,
   return TRUE;
 }
 
-/**
- * ostree_repo_load_file:
- * @self: Repo
- * @checksum: ASCII SHA256 checksum
- * @out_input: (out) (optional) (nullable): File content
- * @out_file_info: (out) (optional) (nullable): File information
- * @out_xattrs: (out) (optional) (nullable): Extended attributes
- * @cancellable: Cancellable
- * @error: Error
- *
- * Load content object, decomposing it into three parts: the actual
- * content (for regular files), the metadata, and extended attributes.
- */
-gboolean
-ostree_repo_load_file (OstreeRepo         *self,
-                       const char         *checksum,
-                       GInputStream      **out_input,
-                       GFileInfo         **out_file_info,
-                       GVariant          **out_xattrs,
-                       GCancellable       *cancellable,
-                       GError            **error)
+static gboolean
+repo_load_file_archive (OstreeRepo *self,
+                        const char         *checksum,
+                        GInputStream      **out_input,
+                        GFileInfo         **out_file_info,
+                        GVariant          **out_xattrs,
+                        GCancellable       *cancellable,
+                        GError            **error)
+{
+  struct stat stbuf;
+  char loose_path_buf[_OSTREE_LOOSE_PATH_MAX];
+  _ostree_loose_path (loose_path_buf, checksum, OSTREE_OBJECT_TYPE_FILE, self->mode);
+
+  glnx_fd_close int fd = -1;
+  if (!ot_openat_ignore_enoent (self->objects_dir_fd, loose_path_buf, &fd,
+                                error))
+    return FALSE;
+
+  if (fd < 0 && self->commit_stagedir_fd != -1)
+    {
+      if (!ot_openat_ignore_enoent (self->commit_stagedir_fd, loose_path_buf, &fd,
+                                    error))
+        return FALSE;
+    }
+
+  if (fd != -1)
+    {
+      if (!glnx_fstat (fd, &stbuf, error))
+        return FALSE;
+
+      g_autoptr(GInputStream) tmp_stream = g_unix_input_stream_new (glnx_steal_fd (&fd), TRUE);
+      /* Note return here */
+      return ostree_content_stream_parse (TRUE, tmp_stream, stbuf.st_size, TRUE,
+                                          out_input, out_file_info, out_xattrs,
+                                          cancellable, error);
+    }
+  else if (self->parent_repo)
+    {
+      return ostree_repo_load_file (self->parent_repo, checksum,
+                                    out_input, out_file_info, out_xattrs,
+                                    cancellable, error);
+    }
+  else
+    {
+      g_set_error (error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND,
+                   "Couldn't find file object '%s'", checksum);
+      return FALSE;
+    }
+}
+
+static gboolean
+repo_load_file_bare (OstreeRepo *self,
+                     const char         *checksum,
+                     GInputStream      **out_input,
+                     GFileInfo         **out_file_info,
+                     GVariant          **out_xattrs,
+                     GCancellable       *cancellable,
+                     GError            **error)
 {
-  gboolean found = FALSE;
   g_autoptr(GInputStream) ret_input = NULL;
   g_autoptr(GFileInfo) ret_file_info = NULL;
   g_autoptr(GVariant) ret_xattrs = NULL;
+  char loose_path_buf[_OSTREE_LOOSE_PATH_MAX];
+  _ostree_loose_path (loose_path_buf, checksum, OSTREE_OBJECT_TYPE_FILE, self->mode);
 
-  OstreeRepoMode repo_mode = ostree_repo_get_mode (self);
+  int objdir_fd = -1; /* referenced */
+  if (!stat_bare_content_object (self, loose_path_buf,
+                                 &objdir_fd,
+                                 &ret_file_info,
+                                 cancellable, error))
+    return FALSE;
 
-  char loose_path_buf[_OSTREE_LOOSE_PATH_MAX];
-  _ostree_loose_path (loose_path_buf, checksum, OSTREE_OBJECT_TYPE_FILE, repo_mode);
+  if (!ret_file_info)
+    {
+      if (self->parent_repo)
+        {
+          return ostree_repo_load_file (self->parent_repo, checksum,
+                                        out_input, out_file_info, out_xattrs,
+                                        cancellable, error);
+        }
+      else
+        {
+          g_set_error (error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND,
+                       "Couldn't find file object '%s'", checksum);
+          return FALSE;
+        }
+    }
 
-  if (repo_mode == OSTREE_REPO_MODE_ARCHIVE_Z2)
+  if (self->mode == OSTREE_REPO_MODE_BARE_USER)
     {
-      int fd = -1;
-      struct stat stbuf;
-      g_autoptr(GInputStream) tmp_stream = NULL;
+      /* In bare-user, symlinks are stored as regular files, so we just
+       * always do an open, then query the user.ostreemeta xattr for
+       * more information.
+       */
+      glnx_fd_close int fd = openat (objdir_fd, loose_path_buf, O_RDONLY | O_CLOEXEC);
+      if (fd < 0)
+        return glnx_throw_errno (error);
 
-      if (!ot_openat_ignore_enoent (self->objects_dir_fd, loose_path_buf, &fd,
-                                    error))
+      g_autoptr(GBytes) bytes = glnx_fgetxattr_bytes (fd, "user.ostreemeta", error);
+      if (bytes == NULL)
         return FALSE;
 
-      if (fd < 0 && self->commit_stagedir_fd != -1)
+      g_autoptr(GVariant) metadata = g_variant_ref_sink (g_variant_new_from_bytes (OSTREE_FILEMETA_GVARIANT_FORMAT,
+                                                                                   bytes, FALSE));
+      ret_xattrs = set_info_from_filemeta (ret_file_info, metadata);
+
+      guint32 mode = g_file_info_get_attribute_uint32 (ret_file_info, "unix::mode");
+      if (S_ISREG (mode) && out_input)
+        {
+          g_assert (fd != -1);
+          ret_input = g_unix_input_stream_new (glnx_steal_fd (&fd), TRUE);
+        }
+      else if (S_ISLNK (mode))
         {
-          if (!ot_openat_ignore_enoent (self->commit_stagedir_fd, loose_path_buf, &fd,
-                                        error))
+          g_file_info_set_file_type (ret_file_info, G_FILE_TYPE_SYMBOLIC_LINK);
+          g_file_info_set_size (ret_file_info, 0);
+
+          char targetbuf[PATH_MAX+1];
+          gsize target_size;
+          g_autoptr(GInputStream) target_input = g_unix_input_stream_new (fd, FALSE);
+          if (!g_input_stream_read_all (target_input, targetbuf, sizeof (targetbuf),
+                                        &target_size, cancellable, error))
             return FALSE;
+
+          g_file_info_set_symlink_target (ret_file_info, targetbuf);
         }
+    }
+  else if (self->mode == OSTREE_REPO_MODE_BARE_USER_ONLY)
+    {
 
-      if (fd != -1)
-        {
-          tmp_stream = g_unix_input_stream_new (fd, TRUE);
-          fd = -1; /* Transfer ownership */
+      /* Canonical info is: uid/gid is 0 and no xattrs, which
+         might be wrong and thus not validate correctly, but
+         at least we report something consistent. */
+      g_file_info_set_attribute_uint32 (ret_file_info, "unix::uid", 0);
+      g_file_info_set_attribute_uint32 (ret_file_info, "unix::gid", 0);
 
-          if (!glnx_stream_fstat ((GFileDescriptorBased*) tmp_stream, &stbuf,
-                                  error))
-            return FALSE;
+      if (g_file_info_get_file_type (ret_file_info) == G_FILE_TYPE_REGULAR &&
+          out_input)
+        {
+          glnx_fd_close int fd = openat (objdir_fd, loose_path_buf, O_RDONLY | O_CLOEXEC);
+          if (fd < 0)
+            return glnx_throw_errno (error);
 
-          if (!ostree_content_stream_parse (TRUE, tmp_stream, stbuf.st_size, TRUE,
-                                            out_input ? &ret_input : NULL,
-                                            &ret_file_info, &ret_xattrs,
-                                            cancellable, error))
-            return FALSE;
+          ret_input = g_unix_input_stream_new (fd, TRUE);
+          fd = -1; /* Transfer ownership */
+        }
 
-          found = TRUE;
+      if (out_xattrs)
+        {
+          GVariantBuilder builder;
+          g_variant_builder_init (&builder, G_VARIANT_TYPE ("a(ayay)"));
+          ret_xattrs = g_variant_ref_sink (g_variant_builder_end (&builder));
         }
     }
   else
     {
-      int objdir_fd = -1; /* referenced */
-      if (!stat_bare_content_object (self, loose_path_buf,
-                                     &objdir_fd,
-                                     &ret_file_info,
-                                     cancellable, error))
-        return FALSE;
+      g_assert (self->mode == OSTREE_REPO_MODE_BARE);
 
-      if (ret_file_info)
+      if (g_file_info_get_file_type (ret_file_info) == G_FILE_TYPE_REGULAR
+          && (out_input || out_xattrs))
         {
-          found = TRUE;
+          glnx_fd_close int fd = openat (objdir_fd, loose_path_buf, O_RDONLY | O_CLOEXEC);
+          if (fd < 0)
+            return glnx_throw_errno (error);
 
-          if (repo_mode == OSTREE_REPO_MODE_BARE_USER)
+          if (out_xattrs)
             {
-              guint32 mode;
-              g_autoptr(GVariant) metadata = NULL;
-              g_autoptr(GBytes) bytes = NULL;
-              glnx_fd_close int fd = -1;
-
-              /* In bare-user, symlinks are stored as regular files, so we just
-               * always do an open, then query the user.ostreemeta xattr for
-               * more information.
-               */
-              fd = openat (objdir_fd, loose_path_buf, O_RDONLY | O_CLOEXEC);
-              if (fd < 0)
-                return glnx_throw_errno (error);
-
-              bytes = glnx_fgetxattr_bytes (fd, "user.ostreemeta", error);
-              if (bytes == NULL)
+              if (self->disable_xattrs)
+                ret_xattrs = g_variant_ref_sink (g_variant_new_array (G_VARIANT_TYPE ("(ayay)"), NULL, 0));
+              else if (!glnx_fd_get_all_xattrs (fd, &ret_xattrs,
+                                                cancellable, error))
                 return FALSE;
-
-              metadata = g_variant_new_from_bytes (OSTREE_FILEMETA_GVARIANT_FORMAT,
-                                                   bytes, FALSE);
-              g_variant_ref_sink (metadata);
-
-              ret_xattrs = set_info_from_filemeta (ret_file_info, metadata);
-
-              mode = g_file_info_get_attribute_uint32 (ret_file_info, "unix::mode");
-
-              if (S_ISREG (mode) && out_input)
-                {
-                  g_assert (fd != -1);
-                  ret_input = g_unix_input_stream_new (fd, TRUE);
-                  fd = -1; /* Transfer ownership */
-                }
-              else if (S_ISLNK (mode))
-                {
-                  g_autoptr(GInputStream) target_input = NULL;
-                  char targetbuf[PATH_MAX+1];
-                  gsize target_size;
-
-                  g_file_info_set_file_type (ret_file_info, G_FILE_TYPE_SYMBOLIC_LINK);
-                  g_file_info_set_size (ret_file_info, 0);
-
-                  target_input = g_unix_input_stream_new (fd, TRUE);
-                  fd = -1; /* Transfer ownership */
-
-                  if (!g_input_stream_read_all (target_input, targetbuf, sizeof (targetbuf),
-                                                &target_size, cancellable, error))
-                    return FALSE;
-
-                  g_file_info_set_symlink_target (ret_file_info, targetbuf);
-                }
-            }
-          else if (repo_mode == OSTREE_REPO_MODE_BARE_USER_ONLY)
-            {
-              glnx_fd_close int fd = -1;
-
-              /* Canonical info is: uid/gid is 0 and no xattrs, which
-                 might be wrong and thus not validate correctly, but
-                 at least we report something consistent. */
-              g_file_info_set_attribute_uint32 (ret_file_info, "unix::uid", 0);
-              g_file_info_set_attribute_uint32 (ret_file_info, "unix::gid", 0);
-
-              if (g_file_info_get_file_type (ret_file_info) == G_FILE_TYPE_REGULAR &&
-                  out_input)
-                {
-                  fd = openat (objdir_fd, loose_path_buf, O_RDONLY | O_CLOEXEC);
-                  if (fd < 0)
-                    return glnx_throw_errno (error);
-
-                  ret_input = g_unix_input_stream_new (fd, TRUE);
-                  fd = -1; /* Transfer ownership */
-                }
-
-              if (out_xattrs)
-                {
-                  GVariantBuilder builder;
-                  g_variant_builder_init (&builder, G_VARIANT_TYPE ("a(ayay)"));
-                  ret_xattrs = g_variant_ref_sink (g_variant_builder_end (&builder));
-                }
             }
-          else
+
+          if (out_input)
             {
-              g_assert (repo_mode == OSTREE_REPO_MODE_BARE);
-
-              if (g_file_info_get_file_type (ret_file_info) == G_FILE_TYPE_REGULAR
-                  && (out_input || out_xattrs))
-                {
-                  glnx_fd_close int fd = -1;
-
-                  fd = openat (objdir_fd, loose_path_buf, O_RDONLY | O_CLOEXEC);
-                  if (fd < 0)
-                    return glnx_throw_errno (error);
-
-                  if (out_xattrs)
-                    {
-                      if (self->disable_xattrs)
-                        ret_xattrs = g_variant_ref_sink (g_variant_new_array (G_VARIANT_TYPE ("(ayay)"), NULL, 0));
-                      else if (!glnx_fd_get_all_xattrs (fd, &ret_xattrs,
-                                                        cancellable, error))
-                        return FALSE;
-                    }
-
-                  if (out_input)
-                    {
-                      ret_input = g_unix_input_stream_new (fd, TRUE);
-                      fd = -1; /* Transfer ownership */
-                    }
-                }
-              else if (g_file_info_get_file_type (ret_file_info) == G_FILE_TYPE_SYMBOLIC_LINK
-                       && out_xattrs)
-                {
-                  if (self->disable_xattrs)
-                    ret_xattrs = g_variant_ref_sink (g_variant_new_array (G_VARIANT_TYPE ("(ayay)"), NULL, 0));
-                  else if (!glnx_dfd_name_get_all_xattrs (objdir_fd, loose_path_buf,
-                                                          &ret_xattrs,
-                                                          cancellable, error))
-                    return FALSE;
-                }
+              ret_input = g_unix_input_stream_new (fd, TRUE);
+              fd = -1; /* Transfer ownership */
             }
         }
-    }
-
-  if (!found)
-    {
-      if (self->parent_repo)
+      else if (g_file_info_get_file_type (ret_file_info) == G_FILE_TYPE_SYMBOLIC_LINK
+               && out_xattrs)
         {
-          if (!ostree_repo_load_file (self->parent_repo, checksum,
-                                      out_input ? &ret_input : NULL,
-                                      out_file_info ? &ret_file_info : NULL,
-                                      out_xattrs ? &ret_xattrs : NULL,
-                                      cancellable, error))
+          if (self->disable_xattrs)
+            ret_xattrs = g_variant_ref_sink (g_variant_new_array (G_VARIANT_TYPE ("(ayay)"), NULL, 0));
+          else if (!glnx_dfd_name_get_all_xattrs (objdir_fd, loose_path_buf,
+                                                  &ret_xattrs,
+                                                  cancellable, error))
             return FALSE;
         }
-      else
-        {
-          g_set_error (error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND,
-                       "Couldn't find file object '%s'", checksum);
-          return FALSE;
-        }
     }
 
   ot_transfer_out_value (out_input, &ret_input);
@@ -2841,6 +2812,36 @@ ostree_repo_load_file (OstreeRepo         *self,
   return TRUE;
 }
 
+/**
+ * ostree_repo_load_file:
+ * @self: Repo
+ * @checksum: ASCII SHA256 checksum
+ * @out_input: (out) (optional) (nullable): File content
+ * @out_file_info: (out) (optional) (nullable): File information
+ * @out_xattrs: (out) (optional) (nullable): Extended attributes
+ * @cancellable: Cancellable
+ * @error: Error
+ *
+ * Load content object, decomposing it into three parts: the actual
+ * content (for regular files), the metadata, and extended attributes.
+ */
+gboolean
+ostree_repo_load_file (OstreeRepo         *self,
+                       const char         *checksum,
+                       GInputStream      **out_input,
+                       GFileInfo         **out_file_info,
+                       GVariant          **out_xattrs,
+                       GCancellable       *cancellable,
+                       GError            **error)
+{
+  if (self->mode == OSTREE_REPO_MODE_ARCHIVE_Z2)
+    return repo_load_file_archive (self, checksum, out_input, out_file_info, out_xattrs,
+                                   cancellable, error);
+  else
+    return repo_load_file_bare (self, checksum, out_input, out_file_info, out_xattrs,
+                                cancellable, error);
+}
+
 /**
  * ostree_repo_load_object_stream:
  * @self: Repo