lib/core: Use GBytes for file headers

This simplifies a lot of code; the header function was structured
to write to an input stream, but many callers only wanted the checksum,
so it's simpler (and error-free) to simply allocate a whole buffer
and checksum that.

For the callers that want to write it, it's also still simpler to allocate the
buffer and write the whole thing rather than having this function do the
writing.

A lot of the complexity here again is a legacy of the packfile code, which is
dead.

This is prep for faster regfile commits where we can avoid `G{In,Out}putStream`.

Closes: #1257
Approved by: jlebon

src/libostree/ostree-core-private.h

@@ -68,17 +68,11 @@ G_BEGIN_DECLS
 #define _OSTREE_ZLIB_FILE_HEADER_GVARIANT_FORMAT G_VARIANT_TYPE ("(tuuuusa(ayay))")
 
 
-GVariant *_ostree_file_header_new (GFileInfo         *file_info,
+GBytes *_ostree_file_header_new (GFileInfo         *file_info,
-                                   GVariant          *xattrs);
+                                 GVariant          *xattrs);
 
-GVariant *_ostree_zlib_file_header_new (GFileInfo         *file_info,
+GBytes *_ostree_zlib_file_header_new (GFileInfo         *file_info,
-                                        GVariant          *xattrs);
+                                      GVariant          *xattrs);
-
-gboolean _ostree_write_variant_with_size (GOutputStream      *output,
-                                          GVariant           *variant,
-                                          OtChecksum         *checksum,
-                                          GCancellable       *cancellable,
-                                          GError            **error);
 
 gboolean
 _ostree_make_temporary_symlink_at (int             tmp_dirfd,

src/libostree/ostree-core.c

@@ -39,6 +39,8 @@ G_STATIC_ASSERT(OSTREE_REPO_MODE_ARCHIVE == OSTREE_REPO_MODE_ARCHIVE_Z2);
 G_STATIC_ASSERT(OSTREE_REPO_MODE_BARE_USER == 2);
 G_STATIC_ASSERT(OSTREE_REPO_MODE_BARE_USER_ONLY == 3);
 
+static GBytes *variant_to_lenprefixed_buffer (GVariant *variant);
+
 #define ALIGN_VALUE(this, boundary) \
   (( ((unsigned long)(this)) + (((unsigned long)(boundary)) -1)) & (~(((unsigned long)(boundary))-1)))
 
@@ -283,7 +285,12 @@ ostree_validate_collection_id (const char *collection_id, GError **error)
   return TRUE;
 }
 
-GVariant *
+/* The file header is part of the "object stream" format
+ * that's not compressed.  It's comprised of uid,gid,mode,
+ * and possibly symlink targets from @file_info, as well
+ * as @xattrs (which if NULL, is taken to be the empty set).
+ */
+GBytes *
 _ostree_file_header_new (GFileInfo         *file_info,
                          GVariant          *xattrs)
 {
@@ -302,19 +309,19 @@ _ostree_file_header_new (GFileInfo         *file_info,
   if (xattrs == NULL)
     tmp_xattrs = g_variant_ref_sink (g_variant_new_array (G_VARIANT_TYPE ("(ayay)"), NULL, 0));
 
-  return g_variant_ref_sink (g_variant_new ("(uuuus@a(ayay))", GUINT32_TO_BE (uid),
+  g_autoptr(GVariant) ret = g_variant_new ("(uuuus@a(ayay))", GUINT32_TO_BE (uid),
-                                            GUINT32_TO_BE (gid), GUINT32_TO_BE (mode), 0,
+                                           GUINT32_TO_BE (gid), GUINT32_TO_BE (mode), 0,
-                                            symlink_target, xattrs ?: tmp_xattrs));
+                                           symlink_target, xattrs ?: tmp_xattrs);
+  return variant_to_lenprefixed_buffer (g_variant_ref_sink (ret));
 }
 
-/*
+/* Like _ostree_file_header_new(), but used for the compressed format in archive
- * ostree_zlib_file_header_new:
+ * repositories. This format hence lives on disk; normally the uncompressed
- * @file_info: a #GFileInfo
+ * stream format doesn't. Instead for "bare" repositories, the file data is
- * @xattrs: (allow-none): Optional extended attribute array
+ * stored directly, or for the special case of bare-user repositories, as a
- *
+ * user.ostreemeta xattr.
- * Returns: (transfer full): A new #GVariant containing file header for an archive repository
  */
-GVariant *
+GBytes *
 _ostree_zlib_file_header_new (GFileInfo         *file_info,
                               GVariant          *xattrs)
 {
@@ -333,83 +340,48 @@ _ostree_zlib_file_header_new (GFileInfo         *file_info,
   if (xattrs == NULL)
     tmp_xattrs = g_variant_ref_sink (g_variant_new_array (G_VARIANT_TYPE ("(ayay)"), NULL, 0));
 
-  return g_variant_ref_sink (g_variant_new ("(tuuuus@a(ayay))",
+  g_autoptr(GVariant) ret = g_variant_new ("(tuuuus@a(ayay))",
-                                            GUINT64_TO_BE (size), GUINT32_TO_BE (uid),
+                                           GUINT64_TO_BE (size), GUINT32_TO_BE (uid),
-                                            GUINT32_TO_BE (gid), GUINT32_TO_BE (mode), 0,
+                                           GUINT32_TO_BE (gid), GUINT32_TO_BE (mode), 0,
-                                            symlink_target, xattrs ?: tmp_xattrs));
+                                           symlink_target, xattrs ?: tmp_xattrs);
+  return variant_to_lenprefixed_buffer (g_variant_ref_sink (ret));
 }
 
-static gboolean
+/* Serialize a variant to a buffer prefixed with its length. The variant will
-write_padding (GOutputStream    *output,
+ * have an 8-byte alignment so it can be safely used with `mmap()`.
-               guint             alignment,
-               gsize             offset,
-               gsize            *out_bytes_written,
-               OtChecksum       *checksum,
-               GCancellable     *cancellable,
-               GError          **error)
-{
-  guint bits;
-  guint padding_len;
-  guchar padding_nuls[8] = {0, 0, 0, 0, 0, 0, 0, 0};
-
-  if (alignment == 8)
-    bits = ((offset) & 7);
-  else
-    bits = ((offset) & 3);
-
-  if (bits > 0)
-    {
-      padding_len = alignment - bits;
-      if (!ot_gio_write_update_checksum (output, (guchar*)padding_nuls, padding_len,
-                                         out_bytes_written, checksum,
-                                         cancellable, error))
-        return FALSE;
-    }
-
-  return TRUE;
-}
-
-/*
- * _ostree_write_variant_with_size:
- * @output: Stream
- * @variant: A variant
- * @out_bytes_written: (out): Number of bytes written
- * @checksum: (allow-none): If provided, update with written data
- * @cancellable: Cancellable
- * @error: Error
- *
- * Use this function for serializing a chain of 1 or more variants
- * into a stream; the @alignment_offset parameter is used to ensure
- * that each variant begins on an 8-byte alignment so it can be safely
- * accessed.
  */
-gboolean
+static GBytes *
-_ostree_write_variant_with_size (GOutputStream      *output,
+variant_to_lenprefixed_buffer (GVariant *variant)
-                                 GVariant           *variant,
-                                 OtChecksum         *checksum,
-                                 GCancellable       *cancellable,
-                                 GError            **error)
 {
+  /* This string is really a binary memory buffer */
+  g_autoptr(GString) buf = g_string_new (NULL);
   /* Write variant size */
   const guint64 variant_size = g_variant_get_size (variant);
   g_assert (variant_size < G_MAXUINT32);
   const guint32 variant_size_u32_be = GUINT32_TO_BE((guint32) variant_size);
 
-  if (!ot_gio_write_update_checksum (output, &variant_size_u32_be, sizeof (variant_size_u32_be),
+  g_string_append_len (buf, (char*)&variant_size_u32_be, sizeof (variant_size_u32_be));
-                                     NULL, checksum, cancellable, error))
+  const gsize alignment_offset = sizeof (variant_size_u32_be);
-    return FALSE;
-  const gsize alignment_offset = sizeof(variant_size_u32_be);
 
-  /* Pad to offset of 8, write variant */
+  /* Write NULs for alignment. At the moment this is a constant 4 bytes (i.e.
-  if (!write_padding (output, 8, alignment_offset, NULL, checksum,
+   * align to 8, since the length is 4 bytes). For now, I decided to keep some
-                      cancellable, error))
+   * of the (now legacy) more generic logic here in case we want to revive it
-    return FALSE;
+   * later.
+   */
+  const guint alignment = 8;
+  const guchar padding_nuls[8] = {0, 0, 0, 0, 0, 0, 0, 0};
+  guint bits;
+  if (alignment == 8)
+    bits = alignment_offset & 7; /* mod 8 */
+  else
+    bits = alignment_offset & 3; /* mod 4 */
+  const guint padding_len = alignment - bits;
 
-  if (!ot_gio_write_update_checksum (output, g_variant_get_data (variant),
+  if (bits > 0)
-                                     variant_size, NULL, checksum,
+    g_string_append_len (buf, (char*)padding_nuls, padding_len);
-                                     cancellable, error))
+
-    return FALSE;
+  g_string_append_len (buf, (char*)g_variant_get_data (variant), g_variant_get_size (variant));
-  return TRUE;
+  return g_string_free_to_bytes (g_steal_pointer (&buf));
 }
 
 /*
@@ -417,35 +389,23 @@ _ostree_write_variant_with_size (GOutputStream      *output,
  * @file_header: A file header
  * @input: File raw content stream
  * @out_input: (out): Serialized object stream
- * @out_header_size: (out): Length of the header
  * @cancellable: Cancellable
  * @error: Error
  *
  * Combines @file_header and @input into a single stream.
  */
 static gboolean
-header_and_input_to_stream (GVariant           *file_header,
+header_and_input_to_stream (GBytes             *file_header,
                             GInputStream       *input,
                             GInputStream      **out_input,
-                            guint64            *out_header_size,
                             GCancellable       *cancellable,
                             GError            **error)
 {
-  /* Get a memory buffer for the header */
-  g_autoptr(GOutputStream) header_out_stream = g_memory_output_stream_new (NULL, 0, g_realloc, g_free);
-  if (!_ostree_write_variant_with_size (header_out_stream, file_header, NULL,
-                                        cancellable, error))
-    return FALSE;
-  if (!g_output_stream_close (header_out_stream, cancellable, error))
-    return FALSE;
-
   /* Our result stream chain */
   g_autoptr(GPtrArray) streams = g_ptr_array_new_with_free_func ((GDestroyNotify)g_object_unref);
 
   /* Append the header to the chain */
-  const gsize header_size = g_memory_output_stream_get_data_size ((GMemoryOutputStream*) header_out_stream);
+  g_autoptr(GInputStream) header_in_stream = g_memory_input_stream_new_from_bytes (file_header);
-  gpointer header_data = g_memory_output_stream_steal_data ((GMemoryOutputStream*) header_out_stream);
-  g_autoptr(GInputStream) header_in_stream = g_memory_input_stream_new_from_data (header_data, header_size, g_free);
 
   g_ptr_array_add (streams, g_object_ref (header_in_stream));
 
@@ -456,12 +416,11 @@ header_and_input_to_stream (GVariant           *file_header,
   /* Return the result stream */
   g_autoptr(GInputStream) ret_input = (GInputStream*)ostree_chain_input_stream_new (streams);
   ot_transfer_out_value (out_input, &ret_input);
-  if (out_header_size)
-    *out_header_size = header_size;
 
   return TRUE;
 }
 
+/* Convert file metadata + file content into an archive-format stream. */
 gboolean
 _ostree_raw_file_to_archive_stream (GInputStream       *input,
                                     GFileInfo          *file_info,
@@ -471,21 +430,17 @@ _ostree_raw_file_to_archive_stream (GInputStream       *input,
                                     GCancellable       *cancellable,
                                     GError            **error)
 {
-  g_autoptr(GVariant) file_header = NULL;
   g_autoptr(GInputStream) zlib_input = NULL;
-
-  file_header = _ostree_zlib_file_header_new (file_info, xattrs);
   if (input != NULL)
     {
-      g_autoptr(GConverter) zlib_compressor = NULL;
+      g_autoptr(GConverter) zlib_compressor =
-
+        G_CONVERTER (g_zlib_compressor_new (G_ZLIB_COMPRESSOR_FORMAT_RAW, compression_level));
-      zlib_compressor = G_CONVERTER (g_zlib_compressor_new (G_ZLIB_COMPRESSOR_FORMAT_RAW, compression_level));
       zlib_input = g_converter_input_stream_new (input, zlib_compressor);
     }
+  g_autoptr(GBytes) file_header = _ostree_zlib_file_header_new (file_info, xattrs);
   return header_and_input_to_stream (file_header,
                                      zlib_input,
                                      out_input,
-                                     NULL,
                                      cancellable,
                                      error);
 }
@@ -578,19 +533,15 @@ ostree_raw_file_to_content_stream (GInputStream       *input,
                                    GCancellable       *cancellable,
                                    GError            **error)
 {
-  g_autoptr(GVariant) file_header = NULL;
+  g_autoptr(GBytes) file_header = _ostree_file_header_new (file_info, xattrs);
-  guint64 header_size;
-
-  file_header = _ostree_file_header_new (file_info, xattrs);
   if (!header_and_input_to_stream (file_header,
                                    input,
                                    out_input,
-                                   &header_size,
                                    cancellable,
                                    error))
     return FALSE;
   if (out_length)
-    *out_length = header_size + g_file_info_get_size (file_info);
+    *out_length = g_bytes_get_size (file_header) + g_file_info_get_size (file_info);
   return TRUE;
 }
 
@@ -813,11 +764,9 @@ ostree_checksum_file_from_input (GFileInfo        *file_info,
     }
   else
     {
-      g_autoptr(GVariant) file_header = _ostree_file_header_new (file_info, xattrs);
+      g_autoptr(GBytes) file_header = _ostree_file_header_new (file_info, xattrs);
 
-      if (!_ostree_write_variant_with_size (NULL, file_header, &checksum,
+      ot_checksum_update_bytes (&checksum, file_header);
-                                            cancellable, error))
-        return FALSE;
 
       if (g_file_info_get_file_type (file_info) == G_FILE_TYPE_REGULAR)
         {

src/libostree/ostree-repo-commit.c

@@ -631,7 +631,6 @@ write_content_object (OstreeRepo         *self,
     }
   else
     {
-      g_autoptr(GVariant) file_meta = NULL;
       g_autoptr(GConverter) zlib_compressor = NULL;
       g_autoptr(GOutputStream) compressed_out_stream = NULL;
       g_autoptr(GOutputStream) temp_out = NULL;
@@ -646,11 +645,15 @@ write_content_object (OstreeRepo         *self,
         return FALSE;
       temp_out = g_unix_output_stream_new (tmpf.fd, FALSE);
 
-      file_meta = _ostree_zlib_file_header_new (file_info, xattrs);
+      g_autoptr(GBytes) file_meta_header = _ostree_zlib_file_header_new (file_info, xattrs);
+      gsize file_meta_len;
+      const guint8* file_meta_buf = g_bytes_get_data (file_meta_header, &file_meta_len);
 
-      if (!_ostree_write_variant_with_size (temp_out, file_meta, NULL,
+      { gsize bytes_written;
-                                            cancellable, error))
+        if (!g_output_stream_write_all (temp_out, file_meta_buf, file_meta_len, &bytes_written,
-        return FALSE;
+                                        cancellable, error))
+          return FALSE;
+      }
 
       if (g_file_info_get_file_type (file_info) == G_FILE_TYPE_REGULAR)
         {

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

@@ -501,13 +501,10 @@ handle_untrusted_content_checksum (OstreeRepo                 *repo,
                                    GError                    **error)
 {
   g_autoptr(GFileInfo) finfo = _ostree_mode_uidgid_to_gfileinfo (state->mode, state->uid, state->gid);
-  g_autoptr(GVariant) header = _ostree_file_header_new (finfo, state->xattrs);
+  g_autoptr(GBytes) header = _ostree_file_header_new (finfo, state->xattrs);
 
   ot_checksum_init (&state->content_checksum);
-
+  ot_checksum_update_bytes (&state->content_checksum, header);
-  if (!_ostree_write_variant_with_size (NULL, header, &state->content_checksum,
-                                        cancellable, error))
-    return FALSE;
 
   return TRUE;
 }

src/libotutil/ot-checksum-utils.h

@@ -51,6 +51,14 @@ void ot_checksum_init (OtChecksum *checksum);
 void ot_checksum_update (OtChecksum *checksum,
                          const guint8   *buf,
                          size_t          len);
+static inline void
+ot_checksum_update_bytes (OtChecksum *checksum,
+                          GBytes     *buf)
+{
+  gsize len;
+  const guint8 *bufdata = g_bytes_get_data (buf, &len);
+  ot_checksum_update (checksum, bufdata, len);
+}
 void ot_checksum_get_digest (OtChecksum *checksum,
                              guint8      *buf,
                              size_t       buflen);