shaba/ostree
1
0
Fork 0
mirror of https://github.com/ostreedev/ostree.git synced 2025-01-26 14:04:12 +03:00
Code Issues Projects Releases Wiki Activity

libostree: Variant-related leak plugs and fixes

Browse Source 
This tries to avoid leaking GVariantBuilders and GVariants in some
situations. The leaks were usually happening when some error occurred
or because of unclear variant ownership situation.

The former is mostly about making sure that g_variant_builder_clear is
called on builders that didn't finish their variant building process.

The latter is surely more work - sometimes the result of
g_variant_builder_end() should not be passed directly to a function,
but rather stored in a g_autoptr(GVariant), sunk and then passed to a
function. IMO, with an advent of g_autoptr, GVariants should be always
sunk instead of relying on some receiver function sinking it. This
would make an easy-to-follow policy of always sinking your
variants. Functions could then assume that the passed variant is
already sunk. These leaks are still happenning in commands, but they
are less harmful, since that code will not be used by some daemon as a
library routine.

Closes: #291
Approved by: cgwalters
This commit is contained in:
Krzesimir Nowak 2016-05-11 11:04:04 +02:00 committed by Colin Walters (automation)
parent aa946cc136
commit 862e6ecdcc
5 changed files with 25 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/libostree/ostree-repo-commit.c
View File

@ -2384,7 +2384,7 @@ get_modified_xattrs (OstreeRepo *self,
if (label)
{
GVariantBuilder *builder;
g_autoptr(GVariantBuilder) builder = NULL;
/* ret_xattrs may be NULL */
builder = ot_util_variant_builder_from_variant (ret_xattrs,

4
src/libostree/ostree-repo-libarchive.c
View File

@ -331,6 +331,7 @@ aic_ensure_parent_dir_with_file_info (OstreeRepoArchiveImportContext *ctx,
{
const char *name = glnx_basename (fullpath);
g_auto(GVariantBuilder) xattrs_builder;
g_autoptr(GVariant) xattrs = NULL;
/* is this the root directory itself? transform into empty string */
if (name[0] == '/' && name[1] == '\0')
@ -343,8 +344,9 @@ aic_ensure_parent_dir_with_file_info (OstreeRepoArchiveImportContext *ctx,
DEFAULT_DIRMODE, cancellable, error))
return FALSE;
xattrs = g_variant_ref_sink (g_variant_builder_end (&xattrs_builder));
return mtree_ensure_dir_with_meta (ctx->repo, parent, name, file_info,
g_variant_builder_end (&xattrs_builder),
xattrs,
FALSE /* error_if_exist */, out_dir,
cancellable, error);
}

5
src/libostree/ostree-repo-pull.c
View File

@ -1759,6 +1759,8 @@ ostree_repo_pull_one_dir (OstreeRepo *self,
GError **error)
{
GVariantBuilder builder;
g_autoptr(GVariant) options = NULL;
g_variant_builder_init (&builder, G_VARIANT_TYPE ("a{sv}"));
if (dir_to_pull)
@ -1770,7 +1772,8 @@ ostree_repo_pull_one_dir (OstreeRepo *self,
g_variant_builder_add (&builder, "{s@v}", "refs",
g_variant_new_variant (g_variant_new_strv ((const char *const*) refs_to_fetch, -1)));
return ostree_repo_pull_with_options (self, remote_name, g_variant_builder_end (&builder),
options = g_variant_ref_sink (g_variant_builder_end (&builder));
return ostree_repo_pull_with_options (self, remote_name, options,
progress, cancellable, error);
}

14
src/libostree/ostree-repo-static-delta-compilation.c
View File

@ -1253,7 +1253,7 @@ ostree_repo_static_delta_generate (OstreeRepo *self,
guint min_fallback_size;
guint max_bsdiff_size;
guint max_chunk_size;
GVariantBuilder metadata_builder;
g_auto(GVariantBuilder) metadata_builder = {0,};
DeltaOpts delta_opts = DELTAOPT_FLAG_NONE;
guint64 total_compressed_size = 0;
guint64 total_uncompressed_size = 0;
@ -1384,16 +1384,18 @@ ostree_repo_static_delta_generate (OstreeRepo *self,
g_autoptr(GVariant) delta_part_content = NULL;
g_autoptr(GVariant) delta_part = NULL;
g_autoptr(GVariant) delta_part_header = NULL;
GVariantBuilder *mode_builder = g_variant_builder_new (G_VARIANT_TYPE ("a(uuu)"));
GVariantBuilder *xattr_builder = g_variant_builder_new (G_VARIANT_TYPE ("aa(ayay)"));
g_auto(GVariantBuilder) mode_builder = {0,};
g_auto(GVariantBuilder) xattr_builder = {0,};
guint8 compression_type_char;
g_variant_builder_init (&mode_builder, G_VARIANT_TYPE ("a(uuu)"));
g_variant_builder_init (&xattr_builder, G_VARIANT_TYPE ("aa(ayay)"));
{ guint j;
for (j = 0; j < part_builder->modes->len; j++)
g_variant_builder_add_value (mode_builder, part_builder->modes->pdata[j]);
g_variant_builder_add_value (&mode_builder, part_builder->modes->pdata[j]);
for (j = 0; j < part_builder->xattrs->len; j++)
g_variant_builder_add_value (xattr_builder, part_builder->xattrs->pdata[j]);
g_variant_builder_add_value (&xattr_builder, part_builder->xattrs->pdata[j]);
}
payload_b = g_string_free_to_bytes (part_builder->payload);
@ -1403,7 +1405,7 @@ ostree_repo_static_delta_generate (OstreeRepo *self,
part_builder->operations = NULL;
/* FIXME - avoid duplicating memory here */
delta_part_content = g_variant_new ("(a(uuu)aa(ayay)@ay@ay)",
mode_builder, xattr_builder,
&mode_builder, &xattr_builder,
ot_gvariant_new_ay_bytes (payload_b),
ot_gvariant_new_ay_bytes (operations_b));
g_variant_ref_sink (delta_part_content);

16
src/libostree/ostree-repo.c
View File

@ -3510,13 +3510,16 @@ ostree_repo_delete_object (OstreeRepo *self,
if (tombstone_commits)
{
g_autoptr(GVariantBuilder) builder = NULL;
builder = g_variant_builder_new (G_VARIANT_TYPE ("a{sv}"));
g_variant_builder_add (builder, "{sv}", "commit", g_variant_new_bytestring (sha256));
g_auto(GVariantBuilder) builder = {0,};
g_autoptr(GVariant) variant = NULL;
g_variant_builder_init (&builder, G_VARIANT_TYPE ("a{sv}"));
g_variant_builder_add (&builder, "{sv}", "commit", g_variant_new_bytestring (sha256));
variant = g_variant_ref_sink (g_variant_builder_end (&builder));
if (!ostree_repo_write_metadata_trusted (self,
OSTREE_OBJECT_TYPE_TOMBSTONE_COMMIT,
sha256,
g_variant_builder_end (builder),
variant,
cancellable,
error))
goto out;
@ -4286,7 +4289,6 @@ ostree_repo_append_gpg_signature (OstreeRepo *self,
gboolean ret = FALSE;
g_autoptr(GVariant) metadata = NULL;
g_autoptr(GVariant) new_metadata = NULL;
g_autoptr(GVariantBuilder) builder = NULL;
if (!ostree_repo_read_commit_detached_metadata (self,
commit_checksum,
@ -4954,7 +4956,7 @@ ostree_repo_regenerate_summary (OstreeRepo *self,
g_autoptr(GVariant) summary = NULL;
GList *ordered_keys = NULL;
GList *iter = NULL;
GVariantDict additional_metadata_builder;
g_auto(GVariantDict) additional_metadata_builder = {0,};
if (!ostree_repo_list_refs (self, NULL, &refs, cancellable, error))
goto out;
@ -4987,7 +4989,7 @@ ostree_repo_regenerate_summary (OstreeRepo *self,
{
guint i;
g_autoptr(GPtrArray) delta_names = NULL;
GVariantDict deltas_builder;
g_auto(GVariantDict) deltas_builder = {0,};
g_autoptr(GVariant) deltas = NULL;
if (!ostree_repo_list_static_delta_names (self, &delta_names, cancellable, error))