From: Francis Deslauriers Date: Thu, 24 Oct 2019 14:23:46 +0000 (-0400) Subject: Fix: usage of `bt_value_array_get_length()` X-Git-Tag: v2.0.0-rc2~47 X-Git-Url: http://git.efficios.com/?p=babeltrace.git;a=commitdiff_plain;h=b602018bfbd512fd2f8f41778f6560e9fb607bcb Fix: usage of `bt_value_array_get_length()` 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 Change-Id: I611bae82b4cb7092f13373283f358d8f7d13ae9d Reviewed-on: https://review.lttng.org/c/babeltrace/+/2249 Tested-by: jenkins --- diff --git a/src/cli/babeltrace2-cfg-cli-args-connect.c b/src/cli/babeltrace2-cfg-cli-args-connect.c index b9693369..3385a1bd 100644 --- a/src/cli/babeltrace2-cfg-cli-args-connect.c +++ b/src/cli/babeltrace2-cfg-cli-args-connect.c @@ -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, diff --git a/src/cli/babeltrace2-cfg-cli-args.c b/src/cli/babeltrace2-cfg-cli-args.c index ae8e8eef..e8465b22 100644 --- a/src/cli/babeltrace2-cfg-cli-args.c +++ b/src/cli/babeltrace2-cfg-cli-args.c @@ -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 { diff --git a/src/cli/babeltrace2-plugins.c b/src/cli/babeltrace2-plugins.c index 42cda686..8fdfe3e7 100644 --- a/src/cli/babeltrace2-plugins.c +++ b/src/cli/babeltrace2-plugins.c @@ -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; } diff --git a/src/cli/babeltrace2.c b/src/cli/babeltrace2.c index 80082d07..6e867734 100644 --- a/src/cli/babeltrace2.c +++ b/src/cli/babeltrace2.c @@ -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", diff --git a/src/lib/lib-logging.c b/src/lib/lib-logging.c index 710aab62..ee11fad0 100644 --- a/src/lib/lib-logging.c +++ b/src/lib/lib-logging.c @@ -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; } diff --git a/src/lib/trace-ir/attributes.c b/src/lib/trace-ir/attributes.c index 8f03c799..4512880a 100644 --- a/src/lib/trace-ir/attributes.c +++ b/src/lib/trace-ir/attributes.c @@ -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 diff --git a/src/plugins/ctf/lttng-live/viewer-connection.c b/src/plugins/ctf/lttng-live/viewer-connection.c index 53358131..9b9a78e2 100644 --- a/src/plugins/ctf/lttng-live/viewer-connection.c +++ b/src/plugins/ctf/lttng-live/viewer-connection.c @@ -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;