From e358ddd51a5be6017f524523ac10d7c17fb78f65 Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=A9mie=20Galarneau?= Date: Thu, 29 Apr 2021 19:01:44 -0400 Subject: [PATCH] Fix: lttng: add-trigger: invalid access past end of exclusions buffer MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit The following command causes the `lttng` client to SEGFAULT on 32-bit machines: lttng add-trigger \ --condition event-rule-matches --domain=user \ '--name=jean-*' \ --exclude-name jean-chretien -x jean-charest \ --action notify Running with valgrind results in the following report: Invalid read of size 8 at 0x12EEA4: validate_exclusion_list (enable_events.c:354) by 0x149018: parse_event_rule (add_trigger.c:964) by 0x148356: handle_condition_event (add_trigger.c:1222) by 0x147EC3: parse_condition (add_trigger.c:1300) by 0x147702: cmd_add_trigger (add_trigger.c:2107) by 0x154067: handle_command (lttng.c:237) by 0x1534D1: parse_args (lttng.c:426) by 0x152C54: main (lttng.c:475) validate_exclusion_list expects a NULL terminated array of exclusions while handle_condition_event prepares an array of exclusions using the dynamic pointer array facilities (which doesn't need to null-terminate arrays). The code that deals with exclusions "lists" mixes various conventions (null-terminated vs explicit size) in different places leading to this kind of errors. All the code that references exclusion lists, along with the string utils, are adapted to make use of the common dynamic pointer array facilities. Signed-off-by: Jérémie Galarneau Change-Id: Icbb7f0e8601c7ecc887dc9ae64d0ec6390e6aba3 --- .gitignore | 2 +- src/bin/lttng/commands/add_trigger.c | 33 ++-- src/bin/lttng/commands/enable_events.c | 142 +++++++++--------- src/bin/lttng/uprobe.c | 32 ++-- src/bin/lttng/utils.h | 6 +- src/common/Makefile.am | 6 + src/common/{filter => }/filter-grammar-test.c | 9 +- src/common/filter/Makefile.am | 6 - src/common/string-utils/string-utils.c | 42 +++--- src/common/string-utils/string-utils.h | 4 +- tests/unit/test_string_utils.c | 29 ++-- 11 files changed, 170 insertions(+), 141 deletions(-) rename src/common/{filter => }/filter-grammar-test.c (95%) diff --git a/.gitignore b/.gitignore index cbd57ea72..26d3511f0 100644 --- a/.gitignore +++ b/.gitignore @@ -61,7 +61,7 @@ compile_commands.json /src/bin/lttng-crash/lttng-crash /src/bin/lttng-relayd/lttng-relayd /src/lib/lttng-ctl/lttng-ctl.pc -/src/common/filter/filter-grammar-test +/src/common/filter-grammar-test /src/common/filter/filter-lexer.c /src/common/filter/filter-parser.c /src/common/filter/filter-parser.h diff --git a/src/bin/lttng/commands/add_trigger.c b/src/bin/lttng/commands/add_trigger.c index ee5c353e4..9c6b2cceb 100644 --- a/src/bin/lttng/commands/add_trigger.c +++ b/src/bin/lttng/commands/add_trigger.c @@ -209,11 +209,6 @@ error: return ret; } -/* This is defined in enable_events.c. */ -LTTNG_HIDDEN -int validate_exclusion_list( - const char *event_name, const char *const *exclusions); - /* * Parse `str` as a log level in domain `domain_type`. * @@ -961,11 +956,7 @@ struct parse_event_rule_res parse_event_rule(int *argc, const char ***argv) goto error; } - if (validate_exclusion_list(name, - (const char **) exclude_names.array.buffer - .data - - ) != 0) { + if (validate_exclusion_list(name, &exclude_names) != 0) { /* * Assume validate_exclusion_list already prints an * error message. @@ -1312,8 +1303,9 @@ end: static struct lttng_rate_policy *parse_rate_policy(const char *policy_str) { - int num_token; - char **tokens = NULL; + int ret; + size_t num_token = 0; + struct lttng_dynamic_pointer_array tokens; struct lttng_rate_policy *policy = NULL; enum lttng_rate_policy_type policy_type; unsigned long long value; @@ -1321,12 +1313,13 @@ static struct lttng_rate_policy *parse_rate_policy(const char *policy_str) char *policy_value_str; assert(policy_str); + lttng_dynamic_pointer_array_init(&tokens, NULL); - /* - * rate policy fields are separated by ':'. - */ - tokens = strutils_split(policy_str, ':', 1); - num_token = strutils_array_of_strings_len(tokens); + /* Rate policy fields are separated by ':'. */ + ret = strutils_split(policy_str, ':', 1, &tokens); + if (ret == 0) { + num_token = lttng_dynamic_pointer_array_get_count(&tokens); + } /* * Early sanity check that the number of parameter is exactly 2. @@ -1337,8 +1330,8 @@ static struct lttng_rate_policy *parse_rate_policy(const char *policy_str) goto end; } - policy_type_str = tokens[0]; - policy_value_str = tokens[1]; + policy_type_str = lttng_dynamic_pointer_array_get_pointer(&tokens, 0); + policy_value_str = lttng_dynamic_pointer_array_get_pointer(&tokens, 1); /* Parse the type. */ if (strcmp(policy_type_str, "once-after") == 0) { @@ -1378,7 +1371,7 @@ static struct lttng_rate_policy *parse_rate_policy(const char *policy_str) } end: - strutils_free_null_terminated_array_of_strings(tokens); + lttng_dynamic_pointer_array_reset(&tokens); return policy; } diff --git a/src/bin/lttng/commands/enable_events.c b/src/bin/lttng/commands/enable_events.c index 056977412..0704b1475 100644 --- a/src/bin/lttng/commands/enable_events.c +++ b/src/bin/lttng/commands/enable_events.c @@ -193,10 +193,11 @@ const char *print_raw_channel_name(const char *name) * Mi print exlcusion list */ static -int mi_print_exclusion(char **names) +int mi_print_exclusion(const struct lttng_dynamic_pointer_array *exclusions) { - int i, ret; - int count = names ? strutils_array_of_strings_len(names) : 0; + int ret; + size_t i; + const size_t count = lttng_dynamic_pointer_array_get_count(exclusions); assert(writer); @@ -204,14 +205,18 @@ int mi_print_exclusion(char **names) ret = 0; goto end; } + ret = mi_lttng_writer_open_element(writer, config_element_exclusions); if (ret) { goto end; } for (i = 0; i < count; i++) { + const char *exclusion = lttng_dynamic_pointer_array_get_pointer( + exclusions, i); + ret = mi_lttng_writer_write_element_string(writer, - config_element_exclusion, names[i]); + config_element_exclusion, exclusion); if (ret) { goto end; } @@ -228,21 +233,24 @@ end: * Return allocated string for pretty-printing exclusion names. */ static -char *print_exclusions(char **names) +char *print_exclusions(const struct lttng_dynamic_pointer_array *exclusions) { int length = 0; - int i; + size_t i; const char preamble[] = " excluding "; char *ret; - int count = names ? strutils_array_of_strings_len(names) : 0; + const size_t count = lttng_dynamic_pointer_array_get_count(exclusions); if (count == 0) { return strdup(""); } - /* calculate total required length */ + /* Calculate total required length. */ for (i = 0; i < count; i++) { - length += strlen(names[i]) + 4; + const char *exclusion = lttng_dynamic_pointer_array_get_pointer( + exclusions, i); + + length += strlen(exclusion) + 4; } length += sizeof(preamble); @@ -250,10 +258,14 @@ char *print_exclusions(char **names) if (!ret) { return NULL; } + strncpy(ret, preamble, length); for (i = 0; i < count; i++) { + const char *exclusion = lttng_dynamic_pointer_array_get_pointer( + exclusions, i); + strcat(ret, "\""); - strcat(ret, names[i]); + strcat(ret, exclusion); strcat(ret, "\""); if (i != count - 1) { strcat(ret, ", "); @@ -323,16 +335,8 @@ end: return ret; } -/* - * FIXME: find a good place to declare this since add trigger also uses it - */ -LTTNG_HIDDEN -int validate_exclusion_list( - const char *event_name, const char *const *exclusions); - -LTTNG_HIDDEN -int validate_exclusion_list( - const char *event_name, const char *const *exclusions) +int validate_exclusion_list(const char *event_name, + const struct lttng_dynamic_pointer_array *exclusions) { int ret; @@ -349,12 +353,18 @@ int validate_exclusion_list( * all exclusions are passed to the session daemon. */ if (strutils_is_star_at_the_end_only_glob_pattern(event_name)) { - const char *const *exclusion; + size_t i, num_exclusions; + + num_exclusions = lttng_dynamic_pointer_array_get_count(exclusions); + + for (i = 0; i < num_exclusions; i++) { + const char *exclusion = + lttng_dynamic_pointer_array_get_pointer( + exclusions, i); - for (exclusion = exclusions; *exclusion; exclusion++) { - if (!strutils_is_star_glob_pattern(*exclusion) || - strutils_is_star_at_the_end_only_glob_pattern(*exclusion)) { - ret = check_exclusion_subsets(event_name, *exclusion); + if (!strutils_is_star_glob_pattern(exclusion) || + strutils_is_star_at_the_end_only_glob_pattern(exclusion)) { + ret = check_exclusion_subsets(event_name, exclusion); if (ret) { goto error; } @@ -374,43 +384,43 @@ end: static int create_exclusion_list_and_validate(const char *event_name, const char *exclusions_arg, - char ***exclusion_list) + struct lttng_dynamic_pointer_array *exclusions) { int ret = 0; - char **exclusions = NULL; /* Split exclusions. */ - exclusions = strutils_split(exclusions_arg, ',', true); - if (!exclusions) { + ret = strutils_split(exclusions_arg, ',', true, exclusions); + if (ret < 0) { goto error; } - if (validate_exclusion_list(event_name, (const char **) exclusions) != + if (validate_exclusion_list(event_name, exclusions) != 0) { goto error; } - *exclusion_list = exclusions; - goto end; error: ret = -1; - strutils_free_null_terminated_array_of_strings(exclusions); + lttng_dynamic_pointer_array_reset(exclusions); end: return ret; } -static void warn_on_truncated_exclusion_names(char * const *exclusion_list, +static void warn_on_truncated_exclusion_names(const struct lttng_dynamic_pointer_array *exclusions, int *warn) { - char * const *exclusion; + size_t i; + const size_t num_exclusions = lttng_dynamic_pointer_array_get_count(exclusions); + + for (i = 0; i < num_exclusions; i++) { + const char * const exclusion = lttng_dynamic_pointer_array_get_pointer(exclusions, i); - for (exclusion = exclusion_list; *exclusion; exclusion++) { - if (strlen(*exclusion) >= LTTNG_SYMBOL_NAME_LEN) { + if (strlen(exclusion) >= LTTNG_SYMBOL_NAME_LEN) { WARN("Event exclusion \"%s\" will be truncated", - *exclusion); + exclusion); *warn = 1; } } @@ -426,11 +436,11 @@ static int enable_events(char *session_name) int error_holder = CMD_SUCCESS, warn = 0, error = 0, success = 1; char *event_name, *channel_name = NULL; struct lttng_event *ev; - struct lttng_domain dom; - char **exclusion_list = NULL; + struct lttng_domain dom = {}; + struct lttng_dynamic_pointer_array exclusions; struct lttng_userspace_probe_location *uprobe_loc = NULL; - memset(&dom, 0, sizeof(dom)); + lttng_dynamic_pointer_array_init(&exclusions, NULL); ev = lttng_event_create(); if (!ev) { @@ -589,22 +599,22 @@ static int enable_events(char *session_name) if (opt_exclude) { ret = create_exclusion_list_and_validate("*", - opt_exclude, &exclusion_list); + opt_exclude, &exclusions); if (ret) { ret = CMD_ERROR; goto error; } ev->exclusion = 1; - warn_on_truncated_exclusion_names(exclusion_list, + warn_on_truncated_exclusion_names(&exclusions, &warn); } if (!opt_filter) { ret = lttng_enable_event_with_exclusions(handle, ev, channel_name, NULL, - exclusion_list ? strutils_array_of_strings_len(exclusion_list) : 0, - exclusion_list); + lttng_dynamic_pointer_array_get_count(&exclusions), + (char **) exclusions.array.buffer.data); if (ret < 0) { switch (-ret) { case LTTNG_ERR_KERN_EVENT_EXIST: @@ -638,7 +648,7 @@ static int enable_events(char *session_name) switch (opt_event_type) { case LTTNG_EVENT_TRACEPOINT: if (opt_loglevel && dom.type != LTTNG_DOMAIN_KERNEL) { - char *exclusion_string = print_exclusions(exclusion_list); + char *exclusion_string = print_exclusions(&exclusions); if (!exclusion_string) { PERROR("Cannot allocate exclusion_string"); @@ -652,7 +662,7 @@ static int enable_events(char *session_name) opt_loglevel); free(exclusion_string); } else { - char *exclusion_string = print_exclusions(exclusion_list); + char *exclusion_string = print_exclusions(&exclusions); if (!exclusion_string) { PERROR("Cannot allocate exclusion_string"); @@ -675,7 +685,7 @@ static int enable_events(char *session_name) break; case LTTNG_EVENT_ALL: if (opt_loglevel && dom.type != LTTNG_DOMAIN_KERNEL) { - char *exclusion_string = print_exclusions(exclusion_list); + char *exclusion_string = print_exclusions(&exclusions); if (!exclusion_string) { PERROR("Cannot allocate exclusion_string"); @@ -689,7 +699,7 @@ static int enable_events(char *session_name) opt_loglevel); free(exclusion_string); } else { - char *exclusion_string = print_exclusions(exclusion_list); + char *exclusion_string = print_exclusions(&exclusions); if (!exclusion_string) { PERROR("Cannot allocate exclusion_string"); @@ -714,9 +724,9 @@ static int enable_events(char *session_name) if (opt_filter) { command_ret = lttng_enable_event_with_exclusions(handle, ev, channel_name, - opt_filter, - exclusion_list ? strutils_array_of_strings_len(exclusion_list) : 0, - exclusion_list); + opt_filter, + lttng_dynamic_pointer_array_get_count(&exclusions), + (char **) exclusions.array.buffer.data); if (command_ret < 0) { switch (-command_ret) { case LTTNG_ERR_FILTER_EXIST: @@ -776,7 +786,7 @@ static int enable_events(char *session_name) } /* print exclusion */ - ret = mi_print_exclusion(exclusion_list); + ret = mi_print_exclusion(&exclusions); if (ret) { ret = CMD_ERROR; goto error; @@ -911,20 +921,18 @@ static int enable_events(char *session_name) ret = CMD_ERROR; goto error; } - /* Free previously allocated items */ - strutils_free_null_terminated_array_of_strings( - exclusion_list); - exclusion_list = NULL; + /* Free previously allocated items. */ + lttng_dynamic_pointer_array_reset(&exclusions); ret = create_exclusion_list_and_validate( event_name, opt_exclude, - &exclusion_list); + &exclusions); if (ret) { ret = CMD_ERROR; goto error; } warn_on_truncated_exclusion_names( - exclusion_list, &warn); + &exclusions, &warn); } ev->loglevel_type = opt_loglevel_type; @@ -999,9 +1007,9 @@ static int enable_events(char *session_name) command_ret = lttng_enable_event_with_exclusions(handle, ev, channel_name, NULL, - exclusion_list ? strutils_array_of_strings_len(exclusion_list) : 0, - exclusion_list); - exclusion_string = print_exclusions(exclusion_list); + lttng_dynamic_pointer_array_get_count(&exclusions), + (char **) exclusions.array.buffer.data); + exclusion_string = print_exclusions(&exclusions); if (!exclusion_string) { PERROR("Cannot allocate exclusion_string"); error = 1; @@ -1083,9 +1091,9 @@ static int enable_events(char *session_name) command_ret = lttng_enable_event_with_exclusions(handle, ev, channel_name, opt_filter, - exclusion_list ? strutils_array_of_strings_len(exclusion_list) : 0, - exclusion_list); - exclusion_string = print_exclusions(exclusion_list); + lttng_dynamic_pointer_array_get_count(&exclusions), + (char **) exclusions.array.buffer.data); + exclusion_string = print_exclusions(&exclusions); if (!exclusion_string) { PERROR("Cannot allocate exclusion_string"); error = 1; @@ -1148,7 +1156,7 @@ static int enable_events(char *session_name) } /* print exclusion */ - ret = mi_print_exclusion(exclusion_list); + ret = mi_print_exclusion(&exclusions); if (ret) { ret = CMD_ERROR; goto error; @@ -1194,7 +1202,7 @@ error: ret = CMD_ERROR; } lttng_destroy_handle(handle); - strutils_free_null_terminated_array_of_strings(exclusion_list); + lttng_dynamic_pointer_array_reset(&exclusions); lttng_userspace_probe_location_destroy(uprobe_loc); /* Overwrite ret with error_holder if there was an actual error with diff --git a/src/bin/lttng/uprobe.c b/src/bin/lttng/uprobe.c index c51c44af0..6af20bbde 100644 --- a/src/bin/lttng/uprobe.c +++ b/src/bin/lttng/uprobe.c @@ -200,22 +200,24 @@ int parse_userspace_probe_opts(const char *opt, struct lttng_userspace_probe_location **probe_location) { int ret = CMD_SUCCESS; - int num_token; - char **tokens = NULL; + size_t num_token = 0; char *target_path = NULL; char *unescaped_target_path = NULL; char *real_target_path = NULL; char *symbol_name = NULL, *probe_name = NULL, *provider_name = NULL; struct lttng_userspace_probe_location *probe_location_local = NULL; struct lttng_userspace_probe_location_lookup_method *lookup_method = NULL; + struct lttng_dynamic_pointer_array tokens; assert(opt); /* * userspace probe fields are separated by ':'. */ - tokens = strutils_split(opt, ':', 1); - num_token = strutils_array_of_strings_len(tokens); + ret = strutils_split(opt, ':', true, &tokens); + if (ret == 0) { + num_token = lttng_dynamic_pointer_array_get_count(&tokens); + } /* * Early sanity check that the number of parameter is between 2 and 4 @@ -224,7 +226,7 @@ int parse_userspace_probe_opts(const char *opt, * std:PATH:PROVIDER_NAME:PROBE_NAME * PATH:SYMBOL (same behavior as ELF) */ - if (num_token < 2 || num_token > 4) { + if (ret < 0 || num_token < 2 || num_token > 4) { ret = CMD_ERROR; goto end; } @@ -237,12 +239,12 @@ int parse_userspace_probe_opts(const char *opt, case 2: /* When the probe type is omitted we assume ELF for now. */ case 3: - if (num_token == 3 && strcmp(tokens[0], "elf") == 0) { - target_path = tokens[1]; - symbol_name = tokens[2]; + if (num_token == 3 && strcmp(lttng_dynamic_pointer_array_get_pointer(&tokens, 0), "elf") == 0) { + target_path = lttng_dynamic_pointer_array_get_pointer(&tokens, 1); + symbol_name = lttng_dynamic_pointer_array_get_pointer(&tokens, 2); } else if (num_token == 2) { - target_path = tokens[0]; - symbol_name = tokens[1]; + target_path = lttng_dynamic_pointer_array_get_pointer(&tokens, 0); + symbol_name = lttng_dynamic_pointer_array_get_pointer(&tokens, 1); } else { ret = CMD_ERROR; goto end; @@ -256,10 +258,10 @@ int parse_userspace_probe_opts(const char *opt, } break; case 4: - if (strcmp(tokens[0], "sdt") == 0) { - target_path = tokens[1]; - provider_name = tokens[2]; - probe_name = tokens[3]; + if (strcmp(lttng_dynamic_pointer_array_get_pointer(&tokens, 0), "sdt") == 0) { + target_path = lttng_dynamic_pointer_array_get_pointer(&tokens, 1); + provider_name = lttng_dynamic_pointer_array_get_pointer(&tokens, 2); + probe_name = lttng_dynamic_pointer_array_get_pointer(&tokens, 3); } else { ret = CMD_ERROR; goto end; @@ -375,7 +377,7 @@ int parse_userspace_probe_opts(const char *opt, end: lttng_userspace_probe_location_destroy(probe_location_local); lttng_userspace_probe_location_lookup_method_destroy(lookup_method); - strutils_free_null_terminated_array_of_strings(tokens); + lttng_dynamic_pointer_array_reset(&tokens); /* * Freeing both char * here makes the error handling simplier. free() * performs not action if the pointer is NULL. diff --git a/src/bin/lttng/utils.h b/src/bin/lttng/utils.h index a51c8579a..97fb202ba 100644 --- a/src/bin/lttng/utils.h +++ b/src/bin/lttng/utils.h @@ -9,7 +9,8 @@ #define _LTTNG_UTILS_H #include -#include "common/argpar/argpar.h" +#include +#include #include @@ -60,4 +61,7 @@ int print_trace_archive_location( const struct lttng_trace_archive_location *location, const char *session_name); +int validate_exclusion_list(const char *event_name, + const struct lttng_dynamic_pointer_array *exclusions); + #endif /* _LTTNG_UTILS_H */ diff --git a/src/common/Makefile.am b/src/common/Makefile.am index df278fa47..e6a88a1fa 100644 --- a/src/common/Makefile.am +++ b/src/common/Makefile.am @@ -180,6 +180,12 @@ noinst_HEADERS = \ uri.h \ utils.h +noinst_PROGRAMS = filter-grammar-test +filter_grammar_test_SOURCES = filter-grammar-test.c +filter_grammar_test_LDADD = \ + libcommon.la +filter_grammar_test_CFLAGS = -I $(top_builddir)/src/common + all-local: @if [ x"$(srcdir)" != x"$(builddir)" ]; then \ for script in $(EXTRA_DIST); do \ diff --git a/src/common/filter/filter-grammar-test.c b/src/common/filter-grammar-test.c similarity index 95% rename from src/common/filter/filter-grammar-test.c rename to src/common/filter-grammar-test.c index 349dc475a..bd21cf877 100644 --- a/src/common/filter/filter-grammar-test.c +++ b/src/common/filter-grammar-test.c @@ -18,8 +18,13 @@ #include #include "common/bytecode/bytecode.h" -#include "filter-ast.h" -#include "filter-parser.h" +#include "filter/filter-ast.h" +#include "filter/filter-parser.h" + +/* For error.h */ +int lttng_opt_quiet = 1; +int lttng_opt_verbose; +int lttng_opt_mi; int main(int argc, char **argv) { diff --git a/src/common/filter/Makefile.am b/src/common/filter/Makefile.am index 261acc086..629280356 100644 --- a/src/common/filter/Makefile.am +++ b/src/common/filter/Makefile.am @@ -2,7 +2,6 @@ AM_CPPFLAGS += -I$(srcdir) -I$(builddir) -noinst_PROGRAMS = filter-grammar-test noinst_LTLIBRARIES = libfilter.la noinst_HEADERS = filter-ast.h \ filter-symbols.h @@ -60,8 +59,3 @@ filter-lexer.c: filter-lexer.l all-local: filter-lexer.c endif # HAVE_FLEX - -filter_grammar_test_SOURCES = filter-grammar-test.c -filter_grammar_test_LDADD = \ - libfilter.la \ - ../bytecode/libbytecode.la diff --git a/src/common/string-utils/string-utils.c b/src/common/string-utils/string-utils.c index e77ddfedb..669f7d05f 100644 --- a/src/common/string-utils/string-utils.c +++ b/src/common/string-utils/string-utils.c @@ -194,11 +194,9 @@ void strutils_free_null_terminated_array_of_strings(char **array) /* * Splits the input string `input` using the given delimiter `delim`. * - * The return value is an allocated null-terminated array of the - * resulting substrings (also allocated). You can free this array and - * its content with strutils_free_null_terminated_array_of_strings(). You - * can get the number of substrings in it with - * strutils_array_of_strings_len(). + * The return value is a dynamic pointer array that is assumed to be empty. The + * array must be discarded by the caller by invoking + * lttng_dynamic_pointer_array_reset(). * * Empty substrings are part of the result. For example: * @@ -241,21 +239,25 @@ void strutils_free_null_terminated_array_of_strings(char **array) * `\` * `hi` * - * Returns NULL if there's an error. + * Returns -1 if there's an error. */ LTTNG_HIDDEN -char **strutils_split(const char *input, char delim, bool escape_delim) +int strutils_split(const char *input, + char delim, + bool escape_delim, + struct lttng_dynamic_pointer_array *out_strings) { + int ret; size_t at; size_t number_of_substrings = 1; size_t longest_substring_len = 0; const char *s; const char *last; - char **substrings = NULL; assert(input); assert(!(escape_delim && delim == '\\')); assert(delim != '\0'); + lttng_dynamic_pointer_array_init(out_strings, free); /* First pass: count the number of substrings. */ for (s = input, last = input - 1; *s != '\0'; s++) { @@ -285,18 +287,20 @@ char **strutils_split(const char *input, char delim, bool escape_delim) longest_substring_len = s - last - 1; } - substrings = calloc(number_of_substrings + 1, sizeof(*substrings)); - if (!substrings) { - goto error; - } - /* Second pass: actually split and copy substrings. */ for (at = 0, s = input; at < number_of_substrings; at++) { const char *ss; char *d; + char *substring = zmalloc(longest_substring_len + 1); + + if (!substring) { + goto error; + } - substrings[at] = zmalloc(longest_substring_len + 1); - if (!substrings[at]) { + ret = lttng_dynamic_pointer_array_add_pointer( + out_strings, substring); + if (ret) { + free(substring); goto error; } @@ -304,7 +308,7 @@ char **strutils_split(const char *input, char delim, bool escape_delim) * Copy characters to substring until we find the next * delimiter or the end of the input string. */ - for (ss = s, d = substrings[at]; *ss != '\0'; ss++) { + for (ss = s, d = substring; *ss != '\0'; ss++) { if (escape_delim && *ss == '\\') { if (ss[1] == delim) { /* @@ -343,13 +347,13 @@ char **strutils_split(const char *input, char delim, bool escape_delim) s = ss + 1; } + ret = 0; goto end; error: - strutils_free_null_terminated_array_of_strings(substrings); - substrings = NULL; + ret = -1; end: - return substrings; + return ret; } LTTNG_HIDDEN diff --git a/src/common/string-utils/string-utils.h b/src/common/string-utils/string-utils.h index 3c1a413ce..9d287f3d9 100644 --- a/src/common/string-utils/string-utils.h +++ b/src/common/string-utils/string-utils.h @@ -10,6 +10,7 @@ #include #include +#include LTTNG_HIDDEN void strutils_normalize_star_glob_pattern(char *pattern); @@ -24,7 +25,8 @@ LTTNG_HIDDEN char *strutils_unescape_string(const char *input, char only_char); LTTNG_HIDDEN -char **strutils_split(const char *input, char delim, bool escape_delim); +int strutils_split(const char *input, char delim, bool escape_delim, + struct lttng_dynamic_pointer_array *out_strings); LTTNG_HIDDEN void strutils_free_null_terminated_array_of_strings(char **array); diff --git a/tests/unit/test_string_utils.c b/tests/unit/test_string_utils.c index b11091637..1d61bc41f 100644 --- a/tests/unit/test_string_utils.c +++ b/tests/unit/test_string_utils.c @@ -16,38 +16,49 @@ /* Number of TAP tests in this file */ #define NUM_TESTS 69 +/* For error.h */ +int lttng_opt_quiet = 1; +int lttng_opt_verbose; +int lttng_opt_mi; + static void test_one_split(const char *input, char delim, int escape_delim, ...) { va_list vl; - char **substrings; - char * const *substring; bool all_ok = true; + struct lttng_dynamic_pointer_array strings; + int split_ret; + size_t i, string_count; - substrings = strutils_split(input, delim, escape_delim); - assert(substrings); + split_ret = strutils_split(input, delim, escape_delim, &strings); + assert(split_ret == 0); va_start(vl, escape_delim); - for (substring = substrings; *substring; substring++) { + string_count = lttng_dynamic_pointer_array_get_count(&strings); + + for (i = 0; i < string_count; i++) { const char *expected_substring = va_arg(vl, const char *); + const char *substring = + lttng_dynamic_pointer_array_get_pointer( + &strings, i); - diag(" got `%s`, expecting `%s`", *substring, expected_substring); + diag(" got `%s`, expecting `%s`", substring, expected_substring); if (!expected_substring) { all_ok = false; break; } - if (strcmp(*substring, expected_substring) != 0) { + if (strcmp(substring, expected_substring) != 0) { all_ok = false; break; } } - strutils_free_null_terminated_array_of_strings(substrings); + lttng_dynamic_pointer_array_reset(&strings); va_end(vl); ok(all_ok, "strutils_split() produces the expected substrings: `%s` (delim. `%c`, escape `%d`)", - input, delim, escape_delim); + input, delim, escape_delim); } static void test_split(void) -- 2.34.1