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