Fix: src.ctf.fs: free ds_file_info when add_ds_file_to_ds_file_group fails

When add_ds_file_to_ds_file_group fails, the created ds_file_info is not
freed, resulting in this leak:

==16942== 168 (16 direct, 152 indirect) bytes in 1 blocks are definitely lost in loss record 11 of 29
==16942==    at 0x4C31B25: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==16942==    by 0x534BB10: g_malloc0 (gmem.c:129)
==16942==    by 0x7795E39: ctf_fs_ds_file_info_create (fs.c:558)
==16942==    by 0x7795E39: add_ds_file_to_ds_file_group (fs.c:781)
==16942==    by 0x7795E39: create_ds_file_groups (fs.c:954)
==16942==    by 0x7795E39: ctf_fs_trace_create (fs.c:1100)
==16942==    by 0x7795E39: ctf_fs_component_create_ctf_fs_trace_one_path (fs.c:1183)
==16942==    by 0x7795E39: ctf_fs_component_create_ctf_fs_trace (fs.c:2059)
==16942==    by 0x7797044: ctf_fs_create (fs.c:2352)
==16942==    by 0x7797044: ctf_fs_init (fs.c:2386)
==16942==    by 0x4E7172F: add_component_with_init_method_data (graph.c:1330)
==16942==    by 0x4E76213: bt_graph_add_source_component_with_initialize_method_data (graph.c:1405)
==16942==    by 0x4E762D0: bt_graph_add_source_component (graph.c:1417)
==16942==    by 0x1149A9: cmd_run_ctx_create_components_from_config_components (babeltrace2.c:2313)
==16942==    by 0x110353: cmd_run_ctx_create_components (babeltrace2.c:2405)
==16942==    by 0x110353: cmd_run (babeltrace2.c:2519)
==16942==    by 0x110353: main (babeltrace2.c:2814)

This can happen if the index fails to build, as in:

  $ ./src/cli/babeltrace2 /home/smarchi/lttng-traces/auto-20191021-162849/ust/uid/1000/64-bit

If add_ds_file_to_ds_file_group is successful, it transfers its
ownership of the ds_file_info object to the ds_file_group, in which case
it should not free it.  But if it fails, the ds_file_info object should
be freed.

This patch introduces the BT_MOVE_REF macro, analogous to C++'s
std::move, for cases like these.  It passes the value of a pointer
variable to a function, while setting that variable to NULL.  This is
meant to be used when the caller transfers its owning reference to the
callee.

Change-Id: I376403caf95e1eee9355ccd587620cf15c75b686
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2234
Reviewed-by: Francis Deslauriers <francis.deslauriers@efficios.com>

diff --git a/src/common/macros.h b/src/common/macros.h
index 23d4b310d573ab49c0f001d900fef04cf678b307..036b3e012b7fc47b360f63007501d5eea20ffe7d 100644 (file)
--- a/src/common/macros.h
+++ b/src/common/macros.h
@@ -38,4 +38,27 @@
 #define BT_HIDDEN __attribute__((visibility("hidden")))
 #endif
 
+/*
+ * Yield `ref`'s value while setting `ref` to NULL.
+ *
+ * This can be used to give a strong reference to a callee:
+ *
+ *   add_foo_to_list(list, BT_MOVE_REF(foo));
+ *
+ * or in a simple assignment:
+ *
+ *   my_struct->foo = BT_MOVE_REF(foo);
+ *
+ * When moving a reference in a function call, the reference is given to the
+ * callee even if that function call fails, so make sure the called function
+ * is written accordingly.
+ */
+
+#define BT_MOVE_REF(ref)               \
+       ({                              \
+               __auto_type _ref = ref; \
+               ref = NULL;             \
+               _ref;                   \
+       })
+
 #endif

diff --git a/src/plugins/ctf/fs-src/fs.c b/src/plugins/ctf/fs-src/fs.c
index e92de4bca52a0b4db2593f25862668a6388e0c7a..28e2b8119cf79b0e88f19d7976426b1a37ff8b2b 100644 (file)
--- a/src/plugins/ctf/fs-src/fs.c
+++ b/src/plugins/ctf/fs-src/fs.c
@@ -818,7 +818,8 @@ int add_ds_file_to_ds_file_group(struct ctf_fs_trace *ctf_fs_trace,
                        goto error;
                }
 
-               ds_file_group_insert_ds_file_info_sorted(ds_file_group, ds_file_info);
+               ds_file_group_insert_ds_file_info_sorted(ds_file_group,
+                       BT_MOVE_REF(ds_file_info));
 
                add_group = true;
                goto end;
@@ -855,7 +856,8 @@ int add_ds_file_to_ds_file_group(struct ctf_fs_trace *ctf_fs_trace,
                merge_ctf_fs_ds_indexes(ds_file_group->index, index);
        }
 
-       ds_file_group_insert_ds_file_info_sorted(ds_file_group, ds_file_info);
+       ds_file_group_insert_ds_file_info_sorted(ds_file_group,
+               BT_MOVE_REF(ds_file_info));
 
        goto end;
 
@@ -870,6 +872,7 @@ end:
        }
 
        ctf_fs_ds_file_destroy(ds_file);
+       ctf_fs_ds_file_info_destroy(ds_file_info);
 
        if (msg_iter) {
                bt_msg_iter_destroy(msg_iter);