From bb51a43d8111416bdd8225487ba803c34ad5b69b Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Sun, 8 Oct 2017 15:55:35 -0400 Subject: [PATCH] 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 | 14 +- src/libostree/ostree-core.c | 169 ++++++------------ src/libostree/ostree-repo-commit.c | 13 +- .../ostree-repo-static-delta-processing.c | 7 +- src/libotutil/ot-checksum-utils.h | 8 + 5 files changed, 81 insertions(+), 130 deletions(-) diff --git a/src/libostree/ostree-core-private.h b/src/libostree/ostree-core-private.h index f964b422..d7677f38 100644 --- a/src/libostree/ostree-core-private.h +++ b/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, - GVariant *xattrs); +GBytes *_ostree_file_header_new (GFileInfo *file_info, + GVariant *xattrs); -GVariant *_ostree_zlib_file_header_new (GFileInfo *file_info, - GVariant *xattrs); - -gboolean _ostree_write_variant_with_size (GOutputStream *output, - GVariant *variant, - OtChecksum *checksum, - GCancellable *cancellable, - GError **error); +GBytes *_ostree_zlib_file_header_new (GFileInfo *file_info, + GVariant *xattrs); gboolean _ostree_make_temporary_symlink_at (int tmp_dirfd, diff --git a/src/libostree/ostree-core.c b/src/libostree/ostree-core.c index 5e8dd874..d840433a 100644 --- a/src/libostree/ostree-core.c +++ b/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), - GUINT32_TO_BE (gid), GUINT32_TO_BE (mode), 0, - symlink_target, xattrs ?: tmp_xattrs)); + g_autoptr(GVariant) ret = g_variant_new ("(uuuus@a(ayay))", GUINT32_TO_BE (uid), + GUINT32_TO_BE (gid), GUINT32_TO_BE (mode), 0, + symlink_target, xattrs ?: tmp_xattrs); + return variant_to_lenprefixed_buffer (g_variant_ref_sink (ret)); } -/* - * ostree_zlib_file_header_new: - * @file_info: a #GFileInfo - * @xattrs: (allow-none): Optional extended attribute array - * - * Returns: (transfer full): A new #GVariant containing file header for an archive repository +/* Like _ostree_file_header_new(), but used for the compressed format in archive + * repositories. This format hence lives on disk; normally the uncompressed + * stream format doesn't. Instead for "bare" repositories, the file data is + * stored directly, or for the special case of bare-user repositories, as a + * user.ostreemeta xattr. */ -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))", - GUINT64_TO_BE (size), GUINT32_TO_BE (uid), - GUINT32_TO_BE (gid), GUINT32_TO_BE (mode), 0, - symlink_target, xattrs ?: tmp_xattrs)); + g_autoptr(GVariant) ret = g_variant_new ("(tuuuus@a(ayay))", + GUINT64_TO_BE (size), GUINT32_TO_BE (uid), + GUINT32_TO_BE (gid), GUINT32_TO_BE (mode), 0, + symlink_target, xattrs ?: tmp_xattrs); + return variant_to_lenprefixed_buffer (g_variant_ref_sink (ret)); } -static gboolean -write_padding (GOutputStream *output, - 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. +/* Serialize a variant to a buffer prefixed with its length. The variant will + * have an 8-byte alignment so it can be safely used with `mmap()`. */ -gboolean -_ostree_write_variant_with_size (GOutputStream *output, - GVariant *variant, - OtChecksum *checksum, - GCancellable *cancellable, - GError **error) +static GBytes * +variant_to_lenprefixed_buffer (GVariant *variant) { + /* 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), - NULL, checksum, cancellable, error)) - return FALSE; - const gsize alignment_offset = sizeof(variant_size_u32_be); + g_string_append_len (buf, (char*)&variant_size_u32_be, sizeof (variant_size_u32_be)); + const gsize alignment_offset = sizeof (variant_size_u32_be); - /* Pad to offset of 8, write variant */ - if (!write_padding (output, 8, alignment_offset, NULL, checksum, - cancellable, error)) - return FALSE; + /* Write NULs for alignment. At the moment this is a constant 4 bytes (i.e. + * align to 8, since the length is 4 bytes). For now, I decided to keep some + * of the (now legacy) more generic logic here in case we want to revive it + * 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), - variant_size, NULL, checksum, - cancellable, error)) - return FALSE; - return TRUE; + if (bits > 0) + g_string_append_len (buf, (char*)padding_nuls, padding_len); + + g_string_append_len (buf, (char*)g_variant_get_data (variant), g_variant_get_size (variant)); + 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); - 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_autoptr(GInputStream) header_in_stream = g_memory_input_stream_new_from_bytes (file_header); 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; - - zlib_compressor = G_CONVERTER (g_zlib_compressor_new (G_ZLIB_COMPRESSOR_FORMAT_RAW, compression_level)); + g_autoptr(GConverter) 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; - guint64 header_size; - - file_header = _ostree_file_header_new (file_info, xattrs); + g_autoptr(GBytes) 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, - cancellable, error)) - return FALSE; + ot_checksum_update_bytes (&checksum, file_header); if (g_file_info_get_file_type (file_info) == G_FILE_TYPE_REGULAR) { diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index a8d2aa1b..ad9bc5af 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/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, - cancellable, error)) - return FALSE; + { gsize bytes_written; + if (!g_output_stream_write_all (temp_out, file_meta_buf, file_meta_len, &bytes_written, + cancellable, error)) + return FALSE; + } if (g_file_info_get_file_type (file_info) == G_FILE_TYPE_REGULAR) { diff --git a/src/libostree/ostree-repo-static-delta-processing.c b/src/libostree/ostree-repo-static-delta-processing.c index 092b6ea7..bf32e02e 100644 --- a/src/libostree/ostree-repo-static-delta-processing.c +++ b/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); - - if (!_ostree_write_variant_with_size (NULL, header, &state->content_checksum, - cancellable, error)) - return FALSE; + ot_checksum_update_bytes (&state->content_checksum, header); return TRUE; } diff --git a/src/libotutil/ot-checksum-utils.h b/src/libotutil/ot-checksum-utils.h index abf3c6db..2d1aa612 100644 --- a/src/libotutil/ot-checksum-utils.h +++ b/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);