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 <francis.deslauriers@efficios.com>
Change-Id: I8fa7d4591da90dae50316d6db86af5ab1b76203c
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2162
Tested-by: jenkins <jenkins@lttng.org>
Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>

diff --git a/include/babeltrace2/value-const.h b/include/babeltrace2/value-const.h
index 4ac17cb083ce0c1658c8cbd591960dff475e4665..2f1ffe779b653bfdfc241f545b1de325fd15d366 100644 (file)
--- 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 b76367144f918b7d06bcce263374bf3343e55905..e01b48fe4ff6539f6fe5e7722ef029923481fd88 100644 (file)
--- 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 5df6ce3a4a3ce813d30ca6fe0f1f682bc9750357..748738af3df8450b7abecd14d3a6418e394697f8 100644 (file)
--- 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, &params_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 91d109fc5d3dbf1d1ea6afc5f5cf0e2c47a58e98..9cef6ef2cc9d903915eae3be5e78307aa75a2e17 100644 (file)
--- 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 617bdc006edc8aacf755bd2cd95a2120c736b997..1dbb9b8d8c60b93cf977c90199522ee580ee57d0 100644 (file)
--- 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,