From: Francis Deslauriers Date: Wed, 9 Oct 2019 19:36:22 +0000 (-0400) Subject: values: make `bt_value_map_extend()` extend the provided base map X-Git-Tag: v2.0.0-rc1~43 X-Git-Url: http://git.efficios.com/?p=babeltrace.git;a=commitdiff_plain;h=ca914e549e4ff113292102d27edd6af662b860d3 values: make `bt_value_map_extend()` extend the provided base map The current behavior of this function is to create a new extended map leaving the two provided maps untouched. I can see cases where that would be useful, but I believe the user is going to expect the `_extend()` function to extend an existing map. If a user want to get a new map, she could still copy the base map using `bt_value_copy()` before extending it. So this commit changes the behavior of `bt_value_map_extend()` so that it takes two arguments: the base map, and the extension map. The entries of the extension map are added to the base map, overwriting any existing keys. Also, remove the `_MEMORY_ERROR` status from the `bt_value_map_foreach_entry_status` and `bt_value_map_foreach_entry_const_status` enums since those two are not used at the moment. Signed-off-by: Francis Deslauriers Change-Id: I8fa7d4591da90dae50316d6db86af5ab1b76203c Reviewed-on: https://review.lttng.org/c/babeltrace/+/2162 Tested-by: jenkins Reviewed-by: Philippe Proulx --- diff --git a/include/babeltrace2/value-const.h b/include/babeltrace2/value-const.h index 4ac17cb0..2f1ffe77 100644 --- a/include/babeltrace2/value-const.h +++ b/include/babeltrace2/value-const.h @@ -159,7 +159,6 @@ typedef bt_bool (* bt_value_map_foreach_entry_const_func)(const char *key, const bt_value *object, void *data); typedef enum bt_value_map_foreach_entry_const_status { - BT_VALUE_MAP_FOREACH_ENTRY_CONST_STATUS_MEMORY_ERROR = __BT_FUNC_STATUS_MEMORY_ERROR, BT_VALUE_MAP_FOREACH_ENTRY_CONST_STATUS_OK = __BT_FUNC_STATUS_OK, BT_VALUE_MAP_FOREACH_ENTRY_CONST_STATUS_INTERRUPTED = __BT_FUNC_STATUS_INTERRUPTED, } bt_value_map_foreach_entry_const_status; @@ -171,16 +170,6 @@ extern bt_value_map_foreach_entry_const_status bt_value_map_foreach_entry_const( extern bt_bool bt_value_map_has_entry(const bt_value *map_obj, const char *key); -typedef enum bt_value_map_extend_status { - BT_VALUE_MAP_EXTEND_STATUS_MEMORY_ERROR = __BT_FUNC_STATUS_MEMORY_ERROR, - BT_VALUE_MAP_EXTEND_STATUS_OK = __BT_FUNC_STATUS_OK, -} bt_value_map_extend_status; - -extern bt_value_map_extend_status bt_value_map_extend( - const bt_value *base_map_obj, - const bt_value *extension_map_obj, - bt_value **extended_map_obj); - extern void bt_value_get_ref(const bt_value *value); extern void bt_value_put_ref(const bt_value *value); diff --git a/include/babeltrace2/value.h b/include/babeltrace2/value.h index b7636714..e01b48fe 100644 --- a/include/babeltrace2/value.h +++ b/include/babeltrace2/value.h @@ -130,7 +130,6 @@ typedef bt_bool (* bt_value_map_foreach_entry_func)(const char *key, bt_value *object, void *data); typedef enum bt_value_map_foreach_entry_status { - BT_VALUE_MAP_FOREACH_ENTRY_STATUS_MEMORY_ERROR = __BT_FUNC_STATUS_MEMORY_ERROR, BT_VALUE_MAP_FOREACH_ENTRY_STATUS_OK = __BT_FUNC_STATUS_OK, BT_VALUE_MAP_FOREACH_ENTRY_STATUS_INTERRUPTED = __BT_FUNC_STATUS_INTERRUPTED, } bt_value_map_foreach_entry_status; @@ -173,6 +172,15 @@ extern bt_value_map_insert_entry_status bt_value_map_insert_empty_map_entry(bt_value *map_obj, const char *key, bt_value **entry_obj); +typedef enum bt_value_map_extend_status { + BT_VALUE_MAP_EXTEND_STATUS_MEMORY_ERROR = __BT_FUNC_STATUS_MEMORY_ERROR, + BT_VALUE_MAP_EXTEND_STATUS_OK = __BT_FUNC_STATUS_OK, +} bt_value_map_extend_status; + +extern bt_value_map_extend_status bt_value_map_extend( + bt_value *base_map_obj, + const bt_value *extension_map_obj); + #ifdef __cplusplus } #endif diff --git a/src/cli/babeltrace2-cfg-cli-args.c b/src/cli/babeltrace2-cfg-cli-args.c index 5df6ce3a..748738af 100644 --- a/src/cli/babeltrace2-cfg-cli-args.c +++ b/src/cli/babeltrace2-cfg-cli-args.c @@ -1919,7 +1919,6 @@ struct bt_config *bt_config_run_from_args(int argc, const char *argv[], case OPT_PARAMS: { bt_value *params; - bt_value *params_to_set; if (!cur_cfg_comp) { BT_CLI_LOGE_APPEND_CAUSE("Cannot add parameters to unavailable component:\n %s", @@ -1934,8 +1933,8 @@ struct bt_config *bt_config_run_from_args(int argc, const char *argv[], goto error; } - extend_status = bt_value_map_extend( - cur_cfg_comp->params, params, ¶ms_to_set); + extend_status = bt_value_map_extend(cur_cfg_comp->params, + params); BT_VALUE_PUT_REF_AND_RESET(params); if (extend_status != BT_VALUE_MAP_EXTEND_STATUS_OK) { BT_CLI_LOGE_APPEND_CAUSE("Cannot extend current component parameters with --params option's argument:\n %s", @@ -1943,7 +1942,6 @@ struct bt_config *bt_config_run_from_args(int argc, const char *argv[], goto error; } - BT_OBJECT_MOVE_REF(cur_cfg_comp->params, params_to_set); break; } case OPT_LOG_LEVEL: diff --git a/src/lib/value.c b/src/lib/value.c index 91d109fc..9cef6ef2 100644 --- a/src/lib/value.c +++ b/src/lib/value.c @@ -1250,7 +1250,7 @@ enum bt_value_map_foreach_entry_const_status bt_value_map_foreach_entry_const( } struct extend_map_element_data { - struct bt_value *extended_obj; + struct bt_value *base_obj; int status; }; @@ -1273,15 +1273,15 @@ bt_bool extend_map_element(const char *key, BT_ASSERT(extension_obj_elem_copy); - /* Replace in extended object */ + /* Replace in base map value. */ extend_data->status = bt_value_map_insert_entry( - extend_data->extended_obj, key, + extend_data->base_obj, key, (void *) extension_obj_elem_copy); if (extend_data->status) { BT_LIB_LOGE_APPEND_CAUSE( - "Cannot replace value in extended value: key=\"%s\", " - "%![extended-value-]+v, %![element-value-]+v", - key, extend_data->extended_obj, + "Cannot replace value in base map value: key=\"%s\", " + "%![base-map-value-]+v, %![element-value-]+v", + key, extend_data->base_obj, extension_obj_elem_copy); goto error; } @@ -1298,66 +1298,35 @@ end: } enum bt_value_map_extend_status bt_value_map_extend( - const struct bt_value *base_map_obj, - const struct bt_value *extension_obj, - struct bt_value **extended_map_obj) + struct bt_value *base_map_obj, + const struct bt_value *extension_obj) { struct extend_map_element_data extend_data = { - .extended_obj = NULL, + .base_obj = NULL, .status = BT_FUNC_STATUS_OK, }; BT_ASSERT_PRE_NON_NULL(base_map_obj, "Base value object"); + BT_ASSERT_PRE_DEV_VALUE_HOT(base_map_obj, "Base value object"); BT_ASSERT_PRE_NON_NULL(extension_obj, "Extension value object"); - BT_ASSERT_PRE_NON_NULL(extended_map_obj, - "Extended value object (output)"); BT_ASSERT_PRE_VALUE_IS_TYPE(base_map_obj, BT_VALUE_TYPE_MAP); BT_ASSERT_PRE_VALUE_IS_TYPE(extension_obj, BT_VALUE_TYPE_MAP); BT_LOGD("Extending map value: base-value-addr=%p, extension-value-addr=%p", base_map_obj, extension_obj); - *extended_map_obj = NULL; - - /* Create copy of base map object to start with */ - extend_data.status = bt_value_copy(base_map_obj, extended_map_obj); - if (extend_data.status) { - BT_LIB_LOGE_APPEND_CAUSE( - "Cannot copy base value: %![base-value-]+v", - base_map_obj); - goto error; - } - - BT_ASSERT(extended_map_obj); /* * For each key in the extension map object, replace this key - * in the copied map object. + * in the base map object. */ - extend_data.extended_obj = *extended_map_obj; + extend_data.base_obj = base_map_obj; if (bt_value_map_foreach_entry_const(extension_obj, extend_map_element, &extend_data)) { BT_LIB_LOGE_APPEND_CAUSE( "Cannot iterate on the extension object's elements: " "%![extension-value-]+v", extension_obj); - goto error; - } - - if (extend_data.status) { - BT_LIB_LOGE_APPEND_CAUSE( - "Failed to successfully iterate on the extension object's elements: " - "%![extension-value-]+v", extension_obj); - goto error; } - BT_LOGD("Extended map value: extended-value-addr=%p", - *extended_map_obj); - goto end; - -error: - BT_OBJECT_PUT_REF_AND_RESET(*extended_map_obj); - *extended_map_obj = NULL; - -end: return extend_data.status; } diff --git a/tests/lib/test_bt_values.c b/tests/lib/test_bt_values.c index 617bdc00..1dbb9b8d 100644 --- a/tests/lib/test_bt_values.c +++ b/tests/lib/test_bt_values.c @@ -1094,6 +1094,7 @@ void test_extend(void) bt_value *extended_map = NULL; bt_value *array = bt_value_array_create(); bt_value_map_insert_entry_status insert_status; + bt_value_copy_status copy_status; bt_value_map_extend_status extend_status; BT_ASSERT(base_map); @@ -1120,7 +1121,9 @@ void test_extend(void) insert_status = bt_value_map_insert_real_entry(extension_map, "project", -404); BT_ASSERT(insert_status == BT_VALUE_MAP_INSERT_ENTRY_STATUS_OK); - extend_status = bt_value_map_extend(base_map, extension_map, &extended_map); + copy_status = bt_value_copy(base_map, &extended_map); + BT_ASSERT(copy_status == BT_VALUE_COPY_STATUS_OK); + extend_status = bt_value_map_extend(extended_map, extension_map); ok(extend_status == BT_VALUE_MAP_EXTEND_STATUS_OK && extended_map, "bt_value_map_extend() succeeds"); ok(bt_value_map_get_size(extended_map) == 5,