core: Add valgrind framework, plug various memory leaks

2025-02-02 13:47:38 +03:00 · 2012-05-04 10:04:32 -04:00 · 2012-05-04 10:04:32 -04:00 · 8d926c3e36
commit 8d926c3e36
parent 7ce587da30
13 changed files with 252 additions and 47 deletions
--- 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:
--- 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;
--- 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)
--- 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)
--- 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;

--- 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;
 }
--- 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);

--- 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;
--- 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
    
--- a/tests/ostree-valgrind.supp
+++ 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
+   ...
+}
--- 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"
--- 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"
--- 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}