Fix: usage of `bt_value_array_get_length()`
authorFrancis Deslauriers <francis.deslauriers@efficios.com>
Thu, 24 Oct 2019 14:23:46 +0000 (10:23 -0400)
committerFrancis Deslauriers <francis.deslauriers@efficios.com>
Sat, 26 Oct 2019 13:42:52 +0000 (09:42 -0400)
Background
==========
Commit 601b0d3 changed the return value of the
`bt_value_array_get_size()` function now named
`bt_value_array_get_length()` from int64_t to uint64_t.

Problems
========
1. Multiple places in the code use the wrong storage type to store the
result of this function. Not only did we use `int64_t` but also, `int`,
`unsigned int` and `size_t` at some places.

2. A few places check for negative return value for handling potential
errors of this function. Because this function cannot fail and cannot
return a negative number, this is useless.

3. The babeltrace2.c file (`set_stream_intersections()` function)
contains two instances of checks against a negative value that would log
an error about the array being empty. Those log statements are
unreachable because the type is unsigned.

Solution
========
1. This commit cleanups so that we use `uint64_t` to store these value
and that we are using `uint64_t` for indexes looping on such value.

2. Remove useless error checks.

3. Those log statements are useful, so this commit corrects the
condition to compare against 0. But, this means that there is a change
in behaviour here. For example, before this commit, when doing the
stream intersection on an empty `bt_value_array` of traces the function would
return success and not log any error; now it fails and log an error.

Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Change-Id: I611bae82b4cb7092f13373283f358d8f7d13ae9d
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2249
Tested-by: jenkins <jenkins@lttng.org>
src/cli/babeltrace2-cfg-cli-args-connect.c
src/cli/babeltrace2-cfg-cli-args.c
src/cli/babeltrace2-plugins.c
src/cli/babeltrace2.c
src/lib/lib-logging.c
src/lib/trace-ir/attributes.c
src/plugins/ctf/lttng-live/viewer-connection.c

index b9693369903f94516a85309b4251c97d2f2e5d9d..3385a1bd751ff203bed524b38bc89e2c4f930b33 100644 (file)
@@ -685,7 +685,7 @@ int bt_config_cli_args_create_connections(struct bt_config *cfg,
                char *error_buf, size_t error_buf_size)
 {
        int ret;
-       size_t i;
+       uint64_t i;
 
        if (!all_named_and_printable(cfg)) {
                snprintf(error_buf, error_buf_size,
index ae8e8eef3f2fe9d3bae984e06007b9b2e18383c7..e8465b224749b8753a3cd525f9334952dc281a49 100644 (file)
@@ -735,7 +735,7 @@ int insert_flat_params_from_array(GString *params_arg,
                const bt_value *names_array, const char *prefix)
 {
        int ret = 0;
-       int i;
+       uint64_t i;
        GString *tmpstr = NULL, *default_value = NULL;
        bool default_set = false, non_default_set = false;
 
@@ -2073,20 +2073,15 @@ struct bt_config *bt_config_run_from_args_array(const bt_value *run_args,
 {
        struct bt_config *cfg = NULL;
        const char **argv;
-       int64_t i, len;
-       const size_t argc = bt_value_array_get_length(run_args);
+       uint64_t i, len = bt_value_array_get_length(run_args);
 
-       argv = calloc(argc, sizeof(*argv));
+       BT_ASSERT(len <= SIZE_MAX);
+       argv = calloc((size_t) len, sizeof(*argv));
        if (!argv) {
                BT_CLI_LOGE_APPEND_CAUSE_OOM();
                goto end;
        }
 
-       len = bt_value_array_get_length(run_args);
-       if (len < 0) {
-               BT_CLI_LOGE_APPEND_CAUSE("Invalid executable arguments.");
-               goto end;
-       }
        for (i = 0; i < len; i++) {
                const bt_value *arg_value =
                        bt_value_array_borrow_element_by_index_const(run_args,
@@ -2099,7 +2094,7 @@ struct bt_config *bt_config_run_from_args_array(const bt_value *run_args,
                argv[i] = arg;
        }
 
-       cfg = bt_config_run_from_args(argc, argv, retcode,
+       cfg = bt_config_run_from_args((int) len, argv, retcode,
                plugin_paths, default_log_level);
 
 end:
@@ -2357,7 +2352,7 @@ int append_run_args_for_implicit_component(
                bt_value *run_args)
 {
        int ret = 0;
-       size_t i;
+       uint64_t i;
        GString *component_arg_for_run = NULL;
 
        if (!impl_args->exists) {
@@ -2400,8 +2395,7 @@ int append_run_args_for_implicit_component(
                }
        }
 
-       for (i = 0; i < bt_value_array_get_length(impl_args->extra_params);
-                       i++) {
+       for (i = 0; i < bt_value_array_get_length(impl_args->extra_params); i++) {
                const bt_value *elem;
                const char *arg;
 
@@ -4338,15 +4332,17 @@ struct bt_config *bt_config_convert_from_args(int argc, const char *argv[],
         * here.
         */
        if (print_run_args || print_run_args_0) {
+               uint64_t args_idx, args_len;
                if (stream_intersection_mode) {
                        BT_CLI_LOGE_APPEND_CAUSE("Cannot specify --stream-intersection with --run-args or --run-args-0.");
                        goto error;
                }
 
-               for (i = 0; i < bt_value_array_get_length(run_args); i++) {
+               args_len = bt_value_array_get_length(run_args);
+               for (args_idx = 0; args_idx < args_len; args_idx++) {
                        const bt_value *arg_value =
                                bt_value_array_borrow_element_by_index(run_args,
-                                                                      i);
+                                       args_idx);
                        const char *arg;
                        GString *quoted = NULL;
                        const char *arg_to_print;
@@ -4371,7 +4367,7 @@ struct bt_config *bt_config_convert_from_args(int argc, const char *argv[],
                                g_string_free(quoted, TRUE);
                        }
 
-                       if (i < bt_value_array_get_length(run_args) - 1) {
+                       if (args_idx < args_len - 1) {
                                if (print_run_args) {
                                        putchar(' ');
                                } else {
index 42cda686ac587122f64fd9199db7c1ffda372fcd..8fdfe3e726008671bea3a81f37661750b1194699 100644 (file)
@@ -130,10 +130,8 @@ int load_dynamic_plugins(const bt_value *plugin_paths)
        int nr_paths, i, ret = 0;
 
        nr_paths = bt_value_array_get_length(plugin_paths);
-       if (nr_paths < 0) {
-               BT_CLI_LOGE_APPEND_CAUSE(
-                       "Cannot load dynamic plugins: no plugin path.");
-               ret = -1;
+       if (nr_paths == 0) {
+               BT_LOGI_STR("No dynamic plugin path.");
                goto end;
        }
 
index 80082d07d78e95e25c2e14ab2e839fd92476fc52..6e867734c2067fb7b2f577cab895e4ac3d8a7e24 100644 (file)
@@ -334,7 +334,6 @@ void print_value_rec(FILE *fp, const bt_value *value, size_t indent)
        uint64_t uint_val;
        double dbl_val;
        const char *str_val;
-       int size;
        GPtrArray *map_keys = NULL;
 
        if (!value) {
@@ -378,12 +377,8 @@ void print_value_rec(FILE *fp, const bt_value *value, size_t indent)
                break;
        case BT_VALUE_TYPE_ARRAY:
        {
-               guint i;
+               uint64_t i, size;
                size = bt_value_array_get_length(value);
-               if (size < 0) {
-                       goto error;
-               }
-
                if (size == 0) {
                        print_indent(fp, indent);
                        fprintf(fp, "[ ]\n");
@@ -944,7 +939,7 @@ int cmd_print_lttng_live_sessions(struct bt_config *cfg)
        static const char * const comp_cls_name = "lttng-live";
        static const bt_component_class_type comp_cls_type =
                BT_COMPONENT_CLASS_TYPE_SOURCE;
-       int64_t array_size, i;
+       uint64_t array_size, i;
        const char *fail_reason = NULL;
        FILE *out_stream = stdout;
 
@@ -1976,8 +1971,7 @@ static
 int compute_stream_intersection(const bt_value *streams,
                struct trace_range *range)
 {
-       unsigned int i;
-       unsigned int stream_count;
+       uint64_t i, stream_count;
        int ret;
 
        BT_ASSERT(bt_value_get_type(streams) == BT_VALUE_TYPE_ARRAY);
@@ -2085,7 +2079,7 @@ int set_stream_intersections(struct cmd_run_ctx *ctx,
 {
        int ret = 0;
        uint64_t trace_idx;
-       int64_t trace_count;
+       uint64_t trace_count;
        const bt_value *query_result = NULL;
        const bt_value *trace_info = NULL;
        const bt_value *stream_infos = NULL;
@@ -2119,7 +2113,7 @@ int set_stream_intersections(struct cmd_run_ctx *ctx,
        }
 
        trace_count = bt_value_array_get_length(query_result);
-       if (trace_count < 0) {
+       if (trace_count == 0) {
                BT_CLI_LOGE_APPEND_CAUSE("`babeltrace.trace-infos` query: result is empty: "
                        "component-class-name=%s", bt_component_class_get_name(comp_cls));
                ret = -1;
@@ -2163,7 +2157,7 @@ int set_stream_intersections(struct cmd_run_ctx *ctx,
                }
 
                stream_count = bt_value_array_get_length(stream_infos);
-               if (stream_count < 0) {
+               if (stream_count == 0) {
                        ret = -1;
                        BT_CLI_LOGE_APPEND_CAUSE("`babeltrace.trace-infos` query: list of streams is empty: "
                                "component-class-name=%s",
index 710aab62f780fbe1c367eb10b635faf2c460fb6f..ee11fad0ce5b0b682a0837f2acaf8dadf3b0c306 100644 (file)
@@ -909,9 +909,8 @@ static inline void format_value(char **buf_ch, bool extended,
        }
        case BT_VALUE_TYPE_ARRAY:
        {
-               int64_t count = bt_value_array_get_length(value);
+               uint64_t count = bt_value_array_get_length(value);
 
-               BT_ASSERT(count >= 0);
                BUF_APPEND(", %selement-count=%" PRId64, PRFIELD(count));
                break;
        }
index 8f03c799a79aa1b590a2432499e12d739ce71edd..4512880a5c6ec903f26e3f5af669a677a85dc653 100644 (file)
@@ -153,19 +153,11 @@ static
 struct bt_value *bt_attributes_borrow_field_by_name(
                struct bt_value *attr_obj, const char *name)
 {
-       uint64_t i;
-       int64_t attr_size;
+       uint64_t i, attr_size;
        struct bt_value *value_obj = NULL;
        struct bt_value *attr_field_name_obj = NULL;
 
        attr_size = bt_value_array_get_length(attr_obj);
-       if (attr_size < 0) {
-               BT_LIB_LOGE_APPEND_CAUSE(
-                       "Cannot get array value's size: %![value-]+v",
-                       attr_obj);
-               goto error;
-       }
-
        for (i = 0; i < attr_size; ++i) {
                const char *field_name;
 
@@ -288,14 +280,13 @@ end:
 BT_HIDDEN
 int bt_attributes_freeze(const struct bt_value *attr_obj)
 {
-       uint64_t i;
-       int64_t count;
+       uint64_t i, count;
        int ret = 0;
 
        BT_ASSERT(attr_obj);
        BT_LOGD("Freezing attributes object: value-addr=%p", attr_obj);
+
        count = bt_value_array_get_length(attr_obj);
-       BT_ASSERT(count >= 0);
 
        /*
         * We do not freeze the array value object itself here, since
index 53358131fe9b9f0a707b0f27c4c1588ecd050627..9b9a78e255e67ee4b2894f40db938c9c24f8c8a1 100644 (file)
@@ -304,7 +304,8 @@ int list_update_session(bt_value *results,
                const struct lttng_viewer_session *session,
                bool *_found, struct live_viewer_connection *viewer_connection)
 {
-       int i, len, ret = 0;
+       int ret = 0;
+       uint64_t i, len;
        bt_value *map = NULL;
        bt_value *hostname = NULL;
        bt_value *session_name = NULL;
@@ -312,16 +313,11 @@ int list_update_session(bt_value *results,
        bool found = false;
 
        len = bt_value_array_get_length(results);
-       if (len < 0) {
-               BT_COMP_LOGE_STR("Error getting size of array.");
-               ret = -1;
-               goto end;
-       }
        for (i = 0; i < len; i++) {
                const char *hostname_str = NULL;
                const char *session_name_str = NULL;
 
-               map = bt_value_array_borrow_element_by_index(results, (size_t) i);
+               map = bt_value_array_borrow_element_by_index(results, i);
                if (!map) {
                        BT_COMP_LOGE_STR("Error borrowing map.");
                        ret = -1;
This page took 0.03139 seconds and 4 git commands to generate.