[ostree] core: Add valgrind framework, plug various memory leaks
- From: Colin Walters <walters src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [ostree] core: Add valgrind framework, plug various memory leaks
- Date: Fri, 4 May 2012 14:05:18 +0000 (UTC)
commit 8d926c3e36f5aecf844b545b74291a51111dfc47
Author: Colin Walters <walters verbum org>
Date: Fri May 4 10:04:32 2012 -0400
core: Add valgrind framework, plug various memory leaks
src/libostree/ostree-core.c | 18 ++--
src/libostree/ostree-repo-file.c | 7 +-
src/libostree/ostree-repo.c | 14 +--
src/ostree/ostree-pull.c | 8 +-
src/ostree/ot-builtin-commit.c | 3 +-
src/ostree/ot-builtin-diff.c | 4 +
src/ostree/ot-builtin-pack.c | 2 +-
src/ostree/ot-builtin-pull-local.c | 4 +
tests/libtest.sh | 14 ++-
tests/ostree-valgrind.supp | 191 ++++++++++++++++++++++++++++++++++++
tests/t0000-basic.sh | 12 +-
tests/t0001-archive.sh | 6 +-
tests/t0010-pull.sh | 16 ++--
13 files changed, 252 insertions(+), 47 deletions(-)
---
diff --git a/src/libostree/ostree-core.c b/src/libostree/ostree-core.c
index 72906ff..1f352a7 100644
--- a/src/libostree/ostree-core.c
+++ b/src/libostree/ostree-core.c
@@ -441,18 +441,18 @@ ostree_content_stream_parse (GInputStream *input,
if (ret_file_info)
g_file_info_set_size (ret_file_info, input_length - archive_header_size - 8);
- if (g_file_info_get_file_type (ret_file_info) != G_FILE_TYPE_REGULAR)
+ if (g_file_info_get_file_type (ret_file_info) == G_FILE_TYPE_REGULAR
+ && out_input)
{
- g_clear_object (&ret_input);
+ /* Give the input stream at its current position as return value;
+ * assuming the caller doesn't seek, this should be fine. We might
+ * want to wrap it though in a non-seekable stream.
+ **/
+ ret_input = g_object_ref (input);
}
ret = TRUE;
- /* Give the input stream at its current position as return value;
- * assuming the caller doesn't seek, this should be fine. We might
- * want to wrap it though in a non-seekable stream.
- **/
- g_object_ref (input);
- ot_transfer_out_value (out_input, &input);
+ ot_transfer_out_value (out_input, &ret_input);
ot_transfer_out_value (out_file_info, &ret_file_info);
ot_transfer_out_value (out_xattrs, &ret_xattrs);
out:
@@ -1835,6 +1835,7 @@ ostree_validate_structureof_pack_superindex (GVariant *superindex,
goto out;
}
csum_v = NULL;
+ bloom = NULL;
g_variant_get_child (superindex, 3, "a(ayay)", &content_iter);
while (g_variant_iter_loop (content_iter, "(@ay ay)",
@@ -1844,6 +1845,7 @@ ostree_validate_structureof_pack_superindex (GVariant *superindex,
goto out;
}
csum_v = NULL;
+ bloom = NULL;
ret = TRUE;
out:
diff --git a/src/libostree/ostree-repo-file.c b/src/libostree/ostree-repo-file.c
index 005f59e..dd82423 100644
--- a/src/libostree/ostree-repo-file.c
+++ b/src/libostree/ostree-repo-file.c
@@ -65,6 +65,7 @@ ostree_repo_file_finalize (GObject *object)
g_free (self->tree_contents_checksum);
g_free (self->tree_metadata_checksum);
g_free (self->commit);
+ g_clear_error (&self->commit_resolve_error);
g_free (self->name);
G_OBJECT_CLASS (ostree_repo_file_parent_class)->finalize (object);
@@ -400,6 +401,8 @@ ostree_repo_file_get_checksum (OstreeRepoFile *self)
self->cached_file_checksum = ostree_checksum_from_bytes_v (csum_bytes);
+ g_variant_unref (csum_bytes);
+
return self->cached_file_checksum;
}
@@ -721,7 +724,7 @@ bsearch_in_file_variant (GVariant *variant,
imin = 0;
while (imax >= imin)
{
- GVariant *child;
+ ot_lvariant GVariant *child = NULL;
const char *cur;
int cmp;
@@ -741,11 +744,9 @@ bsearch_in_file_variant (GVariant *variant,
}
else
{
- ot_clear_gvariant (&child);
*out_pos = imid;
return TRUE;
}
- ot_clear_gvariant (&child);
}
*out_pos = imid;
diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c
index 4fd7f6d..011fde5 100644
--- a/src/libostree/ostree-repo.c
+++ b/src/libostree/ostree-repo.c
@@ -265,13 +265,12 @@ parse_rev_file (OstreeRepo *self,
if (g_str_has_prefix (rev, "ref: "))
{
- GFile *ref;
+ ot_lobj GFile *ref = NULL;
char *ref_sha256;
gboolean subret;
ref = g_file_resolve_relative_path (priv->local_heads_dir, rev + 5);
subret = parse_rev_file (self, ref, &ref_sha256, error);
- g_clear_object (&ref);
if (!subret)
{
@@ -3025,7 +3024,7 @@ list_loose_object_dir (OstreeRepo *self,
value = g_variant_new ("(b as)",
TRUE, g_variant_new_strv (NULL, 0));
/* transfer ownership */
- g_hash_table_replace (inout_objects, g_variant_ref_sink (key),
+ g_hash_table_replace (inout_objects, key,
g_variant_ref_sink (value));
}
loop_next:
@@ -3870,7 +3869,7 @@ checkout_one_file (OstreeRepo *self,
if (link_cache)
{
- ot_lobj GFile *parent;
+ ot_lobj GFile *parent = NULL;
g_assert (possible_loose_path);
parent = g_file_get_parent (possible_loose_path);
@@ -3907,8 +3906,6 @@ ostree_repo_checkout_tree (OstreeRepo *self,
ot_lobj GFileInfo *file_info = NULL;
ot_lvariant GVariant *xattrs = NULL;
ot_lobj GFileEnumerator *dir_enum = NULL;
- ot_lobj GFile *src_child = NULL;
- ot_lobj GFile *dest_path = NULL;
if (!ostree_repo_file_get_xattrs (source, &xattrs, NULL, error))
goto out;
@@ -3931,14 +3928,13 @@ ostree_repo_checkout_tree (OstreeRepo *self,
{
const char *name;
guint32 type;
+ ot_lobj GFile *dest_path = NULL;
+ ot_lobj GFile *src_child = NULL;
name = g_file_info_get_attribute_byte_string (file_info, "standard::name");
type = g_file_info_get_attribute_uint32 (file_info, "standard::type");
- g_clear_object (&dest_path);
dest_path = g_file_get_child (destination, name);
-
- g_clear_object (&src_child);
src_child = g_file_get_child ((GFile*)source, name);
if (type == G_FILE_TYPE_DIRECTORY)
diff --git a/src/ostree/ostree-pull.c b/src/ostree/ostree-pull.c
index 71f8305..71b53f7 100644
--- a/src/ostree/ostree-pull.c
+++ b/src/ostree/ostree-pull.c
@@ -280,6 +280,7 @@ fetch_one_pack_file (OtPullData *pull_data,
goto out;
}
+ g_clear_object (&ret_cached_path);
if (!ostree_repo_get_cached_remote_pack_data (pull_data->repo, pull_data->remote_name,
pack_checksum, is_meta, &ret_cached_path,
cancellable, error))
@@ -1038,6 +1039,7 @@ ostree_builtin_pull (int argc, char **argv, GFile *repo_path, GError **error)
gpointer key, value;
int i;
GCancellable *cancellable = NULL;
+ ot_lfree char *tmpstr = NULL;
ot_lobj OstreeRepo *repo = NULL;
ot_lfree char *path = NULL;
ot_lfree char *baseurl = NULL;
@@ -1081,8 +1083,8 @@ ostree_builtin_pull (int argc, char **argv, GFile *repo_path, GError **error)
config = ostree_repo_get_config (repo);
- key = g_strdup_printf ("remote \"%s\"", pull_data->remote_name);
- baseurl = g_key_file_get_string (config, key, "url", error);
+ tmpstr = g_strdup_printf ("remote \"%s\"", pull_data->remote_name);
+ baseurl = g_key_file_get_string (config, tmpstr, "url", error);
if (!baseurl)
goto out;
pull_data->base_uri = soup_uri_new (baseurl);
@@ -1248,8 +1250,10 @@ ostree_builtin_pull (int argc, char **argv, GFile *repo_path, GError **error)
if (context)
g_option_context_free (context);
g_clear_object (&pull_data->session);
+ g_free (pull_data->remote_name);
if (pull_data->base_uri)
soup_uri_free (pull_data->base_uri);
+ ot_clear_hashtable (&pull_data->file_checksums_to_fetch);
ot_clear_ptrarray (&pull_data->cached_meta_pack_indexes);
ot_clear_ptrarray (&pull_data->cached_data_pack_indexes);
if (summary_uri)
diff --git a/src/ostree/ot-builtin-commit.c b/src/ostree/ot-builtin-commit.c
index 689c4f6..add7aa2 100644
--- a/src/ostree/ot-builtin-commit.c
+++ b/src/ostree/ot-builtin-commit.c
@@ -188,7 +188,6 @@ ostree_builtin_commit (int argc, char **argv, GFile *repo_path, GError **error)
NULL, error);
if (!metadata)
goto out;
- g_variant_ref_sink (metadata);
}
else if (metadata_bin_path)
{
@@ -210,7 +209,7 @@ ostree_builtin_commit (int argc, char **argv, GFile *repo_path, GError **error)
{
const char *s;
const char *eq;
- char *key;
+ ot_lfree char *key = NULL;
s = *iter;
diff --git a/src/ostree/ot-builtin-diff.c b/src/ostree/ot-builtin-diff.c
index 91f7933..335096a 100644
--- a/src/ostree/ot-builtin-diff.c
+++ b/src/ostree/ot-builtin-diff.c
@@ -362,6 +362,7 @@ diff_dirs (GFile *a,
if (!dir_enum)
goto out;
+ g_clear_object (&child_b_info);
while ((child_b_info = g_file_enumerator_next_file (dir_enum, cancellable, &temp_error)) != NULL)
{
const char *name;
@@ -397,6 +398,7 @@ diff_dirs (GFile *a,
goto out;
}
}
+ g_clear_object (&child_b_info);
}
if (temp_error != NULL)
{
@@ -498,5 +500,7 @@ ostree_builtin_diff (int argc, char **argv, GFile *repo_path, GError **error)
ret = TRUE;
out:
+ if (context)
+ g_option_context_free (context);
return ret;
}
diff --git a/src/ostree/ot-builtin-pack.c b/src/ostree/ot-builtin-pack.c
index cacb876..2a62dbf 100644
--- a/src/ostree/ot-builtin-pack.c
+++ b/src/ostree/ot-builtin-pack.c
@@ -737,7 +737,7 @@ do_stats_gather_loose (OtRepackData *data,
OstreeObjectType objtype;
gboolean is_loose;
gboolean is_packed;
- GVariant *pack_array;
+ ot_lvariant GVariant *pack_array = NULL;
ostree_object_name_deserialize (serialized_key, &checksum, &objtype);
diff --git a/src/ostree/ot-builtin-pull-local.c b/src/ostree/ot-builtin-pull-local.c
index eb3f0f3..7bdd72d 100644
--- a/src/ostree/ot-builtin-pull-local.c
+++ b/src/ostree/ot-builtin-pull-local.c
@@ -206,6 +206,10 @@ ostree_builtin_pull_local (int argc, char **argv, GFile *repo_path, GError **err
ret = TRUE;
out:
+ if (data.src_repo)
+ g_object_unref (data.src_repo);
+ if (data.dest_repo)
+ g_object_unref (data.dest_repo);
if (context)
g_option_context_free (context);
return ret;
diff --git a/tests/libtest.sh b/tests/libtest.sh
index b6e57a1..691e7c3 100644
--- a/tests/libtest.sh
+++ b/tests/libtest.sh
@@ -32,6 +32,10 @@ if test -n "${OT_TESTS_DEBUG}"; then
set -x
fi
+if test -n "$OT_TESTS_VALGRIND"; then
+ CMD_PREFIX="env G_SLICE=always-malloc valgrind -q --leak-check=full --num-callers=30 --suppressions=${SRCDIR}/ostree-valgrind.supp"
+fi
+
die () {
if test -z "$OT_TESTS_SAVE_TEMPS"; then
test -f "$test_tmpdir/.test$$" && rm -rf "$test_tmpdir"
@@ -70,7 +74,7 @@ setup_test_repository () {
mkdir repo
cd repo
ot_repo="--repo=`pwd`"
- export OSTREE="ostree ${ot_repo}"
+ export OSTREE="${CMD_PREFIX} ostree ${ot_repo}"
if test "$mode" = "archive"; then
$OSTREE init --archive
else
@@ -110,20 +114,20 @@ setup_fake_remote_repo1() {
mkdir ostree-srv
cd ostree-srv
mkdir gnomerepo
- ostree --repo=gnomerepo init --archive
+ ${CMD_PREFIX} ostree --repo=gnomerepo init --archive
mkdir gnomerepo-files
cd gnomerepo-files
echo first > firstfile
mkdir baz
echo moo > baz/cow
echo alien > baz/saucer
- ostree --repo=${test_tmpdir}/ostree-srv/gnomerepo commit -b main -s "A remote commit" -m "Some Commit body"
+ ${CMD_PREFIX} ostree --repo=${test_tmpdir}/ostree-srv/gnomerepo commit -b main -s "A remote commit" -m "Some Commit body"
mkdir baz/deeper
- ostree --repo=${test_tmpdir}/ostree-srv/gnomerepo commit -b main -s "Add deeper"
+ ${CMD_PREFIX} ostree --repo=${test_tmpdir}/ostree-srv/gnomerepo commit -b main -s "Add deeper"
echo hi > baz/deeper/ohyeah
mkdir baz/another/
echo x > baz/another/y
- ostree --repo=${test_tmpdir}/ostree-srv/gnomerepo commit -b main -s "The rest"
+ ${CMD_PREFIX} ostree --repo=${test_tmpdir}/ostree-srv/gnomerepo commit -b main -s "The rest"
cd ..
rm -rf gnomerepo-files
diff --git a/tests/ostree-valgrind.supp b/tests/ostree-valgrind.supp
new file mode 100644
index 0000000..ce22aa7
--- /dev/null
+++ b/tests/ostree-valgrind.supp
@@ -0,0 +1,191 @@
+{
+ g_type_init_with_debug_flags calloc
+ Memcheck:Leak
+ fun:calloc
+ ...
+ fun:g_type_init_with_debug_flags
+ ...
+}
+
+{
+ g_type_add_interface_static malloc
+ Memcheck:Leak
+ fun:malloc
+ ...
+ fun:g_type_add_interface_static
+ ...
+}
+
+{
+ g_type_add_interface_dynamic malloc
+ Memcheck:Leak
+ fun:malloc
+ ...
+ fun:g_type_add_interface_dynamic
+ ...
+}
+
+{
+ g_type_class_ref malloc
+ Memcheck:Leak
+ fun:malloc
+ ...
+ fun:g_type_class_ref
+ ...
+}
+
+{
+ g_type_register_dynamic malloc
+ Memcheck:Leak
+ fun:malloc
+ ...
+ fun:g_type_register_dynamic
+ ...
+}
+
+{
+ g_type_init_with_debug_flags malloc
+ Memcheck:Leak
+ fun:malloc
+ ...
+ fun:g_type_init_with_debug_flags
+ ...
+}
+
+{
+ g_type_init_with_debug_flags realloc
+ Memcheck:Leak
+ fun:realloc
+ ...
+ fun:g_type_init_with_debug_flags
+ ...
+}
+
+{
+ g_test_add_vtable malloc
+ Memcheck:Leak
+ fun:malloc
+ ...
+ fun:g_test_add_vtable
+ ...
+}
+
+{
+ g_test_init
+ Memcheck:Leak
+ fun:malloc
+ ...
+ fun:g_test_init
+ ...
+}
+
+{
+ g_type_register_static malloc
+ Memcheck:Leak
+ fun:malloc
+ ...
+ fun:g_type_register_static
+ ...
+}
+
+{
+ g_type_register_static realloc
+ Memcheck:Leak
+ fun:realloc
+ ...
+ fun:g_type_register_static
+ ...
+}
+
+{
+ g_type_register_fundamental never freed
+ Memcheck:Leak
+ fun:malloc
+ ...
+ fun:g_type_register_fundamental
+ ...
+}
+
+{
+ g_type_class_ref never finalized
+ Memcheck:Leak
+ fun:calloc
+ fun:g_malloc0
+ fun:g_type_class_ref
+ ...
+}
+
+{
+ DBusGValue qdata
+ Memcheck:Leak
+ fun:realloc
+ fun:g_realloc
+ fun:g_type_set_qdata
+ fun:_dbus_g_value_types_init
+ ...
+}
+
+{
+ gettext conditional jump
+ Memcheck:Cond
+ fun:__GI___strcasecmp_l
+ fun:__gconv_open
+ fun:_nl_find_msg
+ fun:__dcigettext
+ ...
+}
+
+{
+ gettext uninitialized value
+ Memcheck:Value8
+ fun:__GI___strcasecmp_l
+ fun:__gconv_open
+ fun:_nl_find_msg
+ fun:__dcigettext
+ ...
+}
+
+{
+ font config invalid reads
+ Memcheck:Addr4
+ ...
+ fun:FcConfigParseAndLoad
+ ...
+}
+
+{
+ dynamic loader conditional jump
+ Memcheck:Cond
+ fun:index
+ fun:expand_dynamic_string_token
+ fun:_dl_map_object
+ fun:map_doit
+ fun:_dl_catch_error
+ fun:do_preload
+ fun:dl_main
+ ...
+}
+
+{
+ g_vfs_get_local
+ Memcheck:Leak
+ ...
+ fun:g_vfs_get_local
+ ...
+}
+
+{
+ _g_io_modules_ensure_loaded
+ Memcheck:Leak
+ ...
+ fun:_g_io_modules_ensure_loaded
+ ...
+}
+
+{
+ _g_io_module_get_default
+ Memcheck:Leak
+ ...
+ fun:_g_io_module_get_default
+ ...
+}
diff --git a/tests/t0000-basic.sh b/tests/t0000-basic.sh
index d59d231..379a8e9 100755
--- a/tests/t0000-basic.sh
+++ b/tests/t0000-basic.sh
@@ -131,12 +131,12 @@ echo "ok metadata content"
cd ${test_tmpdir}
mkdir repo2
-ostree --repo=repo2 init
-ostree --repo=repo2 pull-local repo
+${CMD_PREFIX} ostree --repo=repo2 init
+${CMD_PREFIX} ostree --repo=repo2 pull-local repo
echo "ok pull-local"
cd ${test_tmpdir}
-ostree --repo=repo2 checkout test2 test2-checkout-from-local-clone
+${CMD_PREFIX} ostree --repo=repo2 checkout test2 test2-checkout-from-local-clone
cd test2-checkout-from-local-clone
assert_file_has_content yet/another/tree/green 'leaf'
echo "ok local clone checkout"
@@ -223,9 +223,9 @@ echo "ok checkout link cache"
cd ${test_tmpdir}
rm -rf shadow-repo
mkdir shadow-repo
-ostree --repo=shadow-repo init
-ostree --repo=shadow-repo config set core.parent $(pwd)/repo
+${CMD_PREFIX} ostree --repo=shadow-repo init
+${CMD_PREFIX} ostree --repo=shadow-repo config set core.parent $(pwd)/repo
rm -rf test2-checkout
parent_rev_test2=$(ostree --repo=repo rev-parse test2)
-ostree --repo=shadow-repo checkout "${parent_rev_test2}" test2-checkout
+${CMD_PREFIX} ostree --repo=shadow-repo checkout "${parent_rev_test2}" test2-checkout
echo "ok checkout from shadow repo"
diff --git a/tests/t0001-archive.sh b/tests/t0001-archive.sh
index 2b2c1d2..496b6c7 100755
--- a/tests/t0001-archive.sh
+++ b/tests/t0001-archive.sh
@@ -38,12 +38,12 @@ echo "ok content"
cd ${test_tmpdir}
mkdir repo2
-ostree --repo=repo2 init
-ostree --repo=repo2 pull-local repo
+${CMD_PREFIX} ostree --repo=repo2 init
+${CMD_PREFIX} ostree --repo=repo2 pull-local repo
echo "ok local clone"
cd ${test_tmpdir}
-ostree --repo=repo2 checkout test2 test2-checkout-from-local-clone
+${CMD_PREFIX} ostree --repo=repo2 checkout test2 test2-checkout-from-local-clone
cd test2-checkout-from-local-clone
assert_file_has_content baz/cow moo
echo "ok local clone checkout"
diff --git a/tests/t0010-pull.sh b/tests/t0010-pull.sh
index 5f58d11..57416c6 100755
--- a/tests/t0010-pull.sh
+++ b/tests/t0010-pull.sh
@@ -26,10 +26,10 @@ echo '1..4'
setup_fake_remote_repo1
cd ${test_tmpdir}
mkdir repo
-ostree --repo=repo init
-ostree --repo=repo remote add origin $(cat httpd-address)/ostree/gnomerepo
-ostree-pull --repo=repo origin main
-ostree --repo=repo fsck
+${CMD_PREFIX} ostree --repo=repo init
+${CMD_PREFIX} ostree --repo=repo remote add origin $(cat httpd-address)/ostree/gnomerepo
+${CMD_PREFIX} ostree-pull --repo=repo origin main
+${CMD_PREFIX} ostree --repo=repo fsck
echo "ok pull"
cd ${test_tmpdir}
@@ -43,10 +43,10 @@ cd ${test_tmpdir}
ostree --repo=$(pwd)/ostree-srv/gnomerepo pack
rm -rf repo
mkdir repo
-ostree --repo=repo init
-ostree --repo=repo remote add origin $(cat httpd-address)/ostree/gnomerepo
-ostree-pull --repo=repo origin main
-ostree --repo=repo fsck
+${CMD_PREFIX} ostree --repo=repo init
+${CMD_PREFIX} ostree --repo=repo remote add origin $(cat httpd-address)/ostree/gnomerepo
+${CMD_PREFIX} ostree-pull --repo=repo origin main
+${CMD_PREFIX} ostree --repo=repo fsck
echo "ok pull packed"
cd ${test_tmpdir}
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
