Merge branch 'jc/strbuf-split'

Arrays of strbuf is often a wrong data structure to use, and
strbuf_split*() family of functions that create them often have
better alternatives.

Update several code paths and replace strbuf_split*().

* jc/strbuf-split:
  trace2: do not use strbuf_split*()
  trace2: trim_trailing_newline followed by trim is a no-op
  sub-process: do not use strbuf_split*()
  environment: do not use strbuf_split*()
  config: do not use strbuf_split()
  notes: do not use strbuf_split*()
  merge-tree: do not use strbuf_split*()
  clean: do not use strbuf_split*() [part 2]
  clean: do not pass the whole structure when it is not necessary
  clean: do not use strbuf_split*() [part 1]
  clean: do not pass strbuf by value
  wt-status: avoid strbuf_split*()

builtin/clean.c

@@ -478,43 +478,39 @@ static int find_unique(const char *choice, struct menu_stuff *menu_stuff)
  */
 static int parse_choice(struct menu_stuff *menu_stuff,
 			int is_single,
-			struct strbuf input,
+			char *input,
 			int **chosen)
 {
-	struct strbuf **choice_list, **ptr;
+	struct string_list choice = STRING_LIST_INIT_NODUP;
+	struct string_list_item *item;
 	int nr = 0;
 	int i;
 
-	if (is_single) {
+	string_list_split_in_place_f(&choice, input,
-		choice_list = strbuf_split_max(&input, '\n', 0);
+				     is_single ? "\n" : ", ", -1,
-	} else {
+				     STRING_LIST_SPLIT_TRIM);
-		char *p = input.buf;
-		do {
-			if (*p == ',')
-				*p = ' ';
-		} while (*p++);
-		choice_list = strbuf_split_max(&input, ' ', 0);
-	}
 
-	for (ptr = choice_list; *ptr; ptr++) {
+	for_each_string_list_item(item, &choice) {
-		char *p;
+		const char *string;
-		int choose = 1;
+		int choose;
 		int bottom = 0, top = 0;
 		int is_range, is_number;
 
-		strbuf_trim(*ptr);
+		string = item->string;
-		if (!(*ptr)->len)
+		if (!*string)
 			continue;
 
 		/* Input that begins with '-'; unchoose */
-		if (*(*ptr)->buf == '-') {
+		if (string[0] == '-') {
 			choose = 0;
-			strbuf_remove((*ptr), 0, 1);
+			string++;
+		} else {
+			choose = 1;
 		}
 
 		is_range = 0;
 		is_number = 1;
-		for (p = (*ptr)->buf; *p; p++) {
+		for (const char *p = string; *p; p++) {
 			if ('-' == *p) {
 				if (!is_range) {
 					is_range = 1;
@@ -532,27 +528,27 @@ static int parse_choice(struct menu_stuff *menu_stuff,
 		}
 
 		if (is_number) {
-			bottom = atoi((*ptr)->buf);
+			bottom = atoi(string);
 			top = bottom;
 		} else if (is_range) {
-			bottom = atoi((*ptr)->buf);
+			bottom = atoi(string);
 			/* a range can be specified like 5-7 or 5- */
-			if (!*(strchr((*ptr)->buf, '-') + 1))
+			if (!*(strchr(string, '-') + 1))
 				top = menu_stuff->nr;
 			else
-				top = atoi(strchr((*ptr)->buf, '-') + 1);
+				top = atoi(strchr(string, '-') + 1);
-		} else if (!strcmp((*ptr)->buf, "*")) {
+		} else if (!strcmp(string, "*")) {
 			bottom = 1;
 			top = menu_stuff->nr;
 		} else {
-			bottom = find_unique((*ptr)->buf, menu_stuff);
+			bottom = find_unique(string, menu_stuff);
 			top = bottom;
 		}
 
 		if (top <= 0 || bottom <= 0 || top > menu_stuff->nr || bottom > top ||
 		    (is_single && bottom != top)) {
 			clean_print_color(CLEAN_COLOR_ERROR);
-			printf(_("Huh (%s)?\n"), (*ptr)->buf);
+			printf(_("Huh (%s)?\n"), string);
 			clean_print_color(CLEAN_COLOR_RESET);
 			continue;
 		}
@@ -561,7 +557,7 @@ static int parse_choice(struct menu_stuff *menu_stuff,
 			(*chosen)[i-1] = choose;
 	}
 
-	strbuf_list_free(choice_list);
+	string_list_clear(&choice, 0);
 
 	for (i = 0; i < menu_stuff->nr; i++)
 		nr += (*chosen)[i];
@@ -631,7 +627,7 @@ static int *list_and_choose(struct menu_opts *opts, struct menu_stuff *stuff)
 
 		nr = parse_choice(stuff,
 				  opts->flags & MENU_OPTS_SINGLETON,
-				  choice,
+				  choice.buf,
 				  &chosen);
 
 		if (opts->flags & MENU_OPTS_SINGLETON) {
@@ -679,12 +675,13 @@ static int filter_by_patterns_cmd(void)
 {
 	struct dir_struct dir = DIR_INIT;
 	struct strbuf confirm = STRBUF_INIT;
-	struct strbuf **ignore_list;
-	struct string_list_item *item;
 	struct pattern_list *pl;
 	int changed = -1, i;
 
 	for (;;) {
+		struct string_list ignore_list = STRING_LIST_INIT_NODUP;
+		struct string_list_item *item;
+
 		if (!del_list.nr)
 			break;
 
@@ -702,14 +699,15 @@ static int filter_by_patterns_cmd(void)
 			break;
 
 		pl = add_pattern_list(&dir, EXC_CMDL, "manual exclude");
-		ignore_list = strbuf_split_max(&confirm, ' ', 0);
 
-		for (i = 0; ignore_list[i]; i++) {
+		string_list_split_in_place_f(&ignore_list, confirm.buf, " ", -1,
-			strbuf_trim(ignore_list[i]);
+					     STRING_LIST_SPLIT_TRIM);
-			if (!ignore_list[i]->len)
+
+		for (i = 0; i < ignore_list.nr; i++) {
+			item = &ignore_list.items[i];
+			if (!*item->string)
 				continue;
-
+			add_pattern(item->string, "", 0, pl, -(i+1));
-			add_pattern(ignore_list[i]->buf, "", 0, pl, -(i+1));
 		}
 
 		changed = 0;
@@ -730,7 +728,7 @@ static int filter_by_patterns_cmd(void)
 			clean_print_color(CLEAN_COLOR_RESET);
 		}
 
-		strbuf_list_free(ignore_list);
+		string_list_clear(&ignore_list, 0);
 		dir_clear(&dir);
 	}
 

builtin/merge-tree.c

@@ -619,32 +619,34 @@ int cmd_merge_tree(int argc,
 			    "--merge-base", "--stdin");
 		line_termination = '\0';
 		while (strbuf_getline_lf(&buf, stdin) != EOF) {
-			struct strbuf **split;
+			struct string_list split = STRING_LIST_INIT_NODUP;
 			const char *input_merge_base = NULL;
 
-			split = strbuf_split(&buf, ' ');
+			string_list_split_in_place_f(&split, buf.buf, " ", -1,
-			if (!split[0] || !split[1])
+						     STRING_LIST_SPLIT_TRIM);
+
+			if (split.nr < 2)
 				die(_("malformed input line: '%s'."), buf.buf);
-			strbuf_rtrim(split[0]);
-			strbuf_rtrim(split[1]);
 
 			/* parse the merge-base */
-			if (!strcmp(split[1]->buf, "--")) {
+			if (!strcmp(split.items[1].string, "--")) {
-				input_merge_base = split[0]->buf;
+				input_merge_base = split.items[0].string;
 			}
 
-			if (input_merge_base && split[2] && split[3] && !split[4]) {
+			if (input_merge_base && split.nr == 4) {
-				strbuf_rtrim(split[2]);
+				real_merge(&o, input_merge_base,
-				strbuf_rtrim(split[3]);
+					   split.items[2].string, split.items[3].string,
-				real_merge(&o, input_merge_base, split[2]->buf, split[3]->buf, prefix);
+					   prefix);
-			} else if (!input_merge_base && !split[2]) {
+			} else if (!input_merge_base && split.nr == 2) {
-				real_merge(&o, NULL, split[0]->buf, split[1]->buf, prefix);
+				real_merge(&o, NULL,
+					   split.items[0].string, split.items[1].string,
+					   prefix);
 			} else {
 				die(_("malformed input line: '%s'."), buf.buf);
 			}
 			maybe_flush_or_die(stdout, "stdout");
 
-			strbuf_list_free(split);
+			string_list_clear(&split, 0);
 		}
 		strbuf_release(&buf);
 

builtin/notes.c

@@ -376,18 +376,19 @@ static int notes_copy_from_stdin(int force, const char *rewrite_cmd)
 
 	while (strbuf_getline_lf(&buf, stdin) != EOF) {
 		struct object_id from_obj, to_obj;
-		struct strbuf **split;
+		struct string_list split = STRING_LIST_INIT_NODUP;
 		int err;
 
-		split = strbuf_split(&buf, ' ');
+		string_list_split_in_place_f(&split, buf.buf, " ", -1,
-		if (!split[0] || !split[1])
+					     STRING_LIST_SPLIT_TRIM);
+		if (split.nr < 2)
 			die(_("malformed input line: '%s'."), buf.buf);
-		strbuf_rtrim(split[0]);
+		if (repo_get_oid(the_repository, split.items[0].string, &from_obj))
-		strbuf_rtrim(split[1]);
+			die(_("failed to resolve '%s' as a valid ref."),
-		if (repo_get_oid(the_repository, split[0]->buf, &from_obj))
+			    split.items[0].string);
-			die(_("failed to resolve '%s' as a valid ref."), split[0]->buf);
+		if (repo_get_oid(the_repository, split.items[1].string, &to_obj))
-		if (repo_get_oid(the_repository, split[1]->buf, &to_obj))
+			die(_("failed to resolve '%s' as a valid ref."),
-			die(_("failed to resolve '%s' as a valid ref."), split[1]->buf);
+			    split.items[1].string);
 
 		if (rewrite_cmd)
 			err = copy_note_for_rewrite(c, &from_obj, &to_obj);
@@ -397,11 +398,11 @@ static int notes_copy_from_stdin(int force, const char *rewrite_cmd)
 
 		if (err) {
 			error(_("failed to copy notes from '%s' to '%s'"),
-			      split[0]->buf, split[1]->buf);
+			      split.items[0].string, split.items[1].string);
 			ret = 1;
 		}
 
-		strbuf_list_free(split);
+		string_list_clear(&split, 0);
 	}
 
 	if (!rewrite_cmd) {

config.c

@@ -629,31 +629,28 @@ int git_config_parse_parameter(const char *text,
 			       config_fn_t fn, void *data)
 {
 	const char *value;
-	struct strbuf **pair;
+	struct string_list pair = STRING_LIST_INIT_DUP;
 	int ret;
 	struct key_value_info kvi = KVI_INIT;
 
 	kvi_from_param(&kvi);
 
-	pair = strbuf_split_str(text, '=', 2);
+	string_list_split(&pair, text, "=", 1);
-	if (!pair[0])
+	if (!pair.nr)
 		return error(_("bogus config parameter: %s"), text);
 
-	if (pair[0]->len && pair[0]->buf[pair[0]->len - 1] == '=') {
+	if (pair.nr == 1)
-		strbuf_setlen(pair[0], pair[0]->len - 1);
-		value = pair[1] ? pair[1]->buf : "";
-	} else {
 		value = NULL;
-	}
+	else
+		value = pair.items[1].string;
 
-	strbuf_trim(pair[0]);
+	if (!*pair.items[0].string) {
-	if (!pair[0]->len) {
+		string_list_clear(&pair, 0);
-		strbuf_list_free(pair);
 		return error(_("bogus config parameter: %s"), text);
 	}
 
-	ret = config_parse_pair(pair[0]->buf, value, &kvi, fn, data);
+	ret = config_parse_pair(pair.items[0].string, value, &kvi, fn, data);
-	strbuf_list_free(pair);
+	string_list_clear(&pair, 0);
 	return ret;
 }
 

environment.c

@@ -175,10 +175,10 @@ int have_git_dir(void)
 const char *get_git_namespace(void)
 {
 	static const char *namespace;
-
 	struct strbuf buf = STRBUF_INIT;
-	struct strbuf **components, **c;
 	const char *raw_namespace;
+	struct string_list components = STRING_LIST_INIT_DUP;
+	struct string_list_item *item;
 
 	if (namespace)
 		return namespace;
@@ -190,12 +190,17 @@ const char *get_git_namespace(void)
 	}
 
 	strbuf_addstr(&buf, raw_namespace);
-	components = strbuf_split(&buf, '/');
+
+	string_list_split(&components, buf.buf, "/", -1);
 	strbuf_reset(&buf);
-	for (c = components; *c; c++)
+
-		if (strcmp((*c)->buf, "/") != 0)
+	for_each_string_list_item(item, &components) {
-			strbuf_addf(&buf, "refs/namespaces/%s", (*c)->buf);
+		if (item->string[0])
-	strbuf_list_free(components);
+			strbuf_addf(&buf, "refs/namespaces/%s/", item->string);
+	}
+	string_list_clear(&components, 0);
+
+	strbuf_trim_trailing_dir_sep(&buf);
 	if (check_refname_format(buf.buf, 0))
 		die(_("bad git namespace path \"%s\""), raw_namespace);
 	strbuf_addch(&buf, '/');

sub-process.c

@@ -30,23 +30,20 @@ struct subprocess_entry *subprocess_find_entry(struct hashmap *hashmap, const ch
 
 int subprocess_read_status(int fd, struct strbuf *status)
 {
-	struct strbuf **pair;
-	char *line;
 	int len;
 
 	for (;;) {
+		char *line;
+		const char *value;
+
 		len = packet_read_line_gently(fd, NULL, &line);
 		if ((len < 0) || !line)
 			break;
-		pair = strbuf_split_str(line, '=', 2);
+		if (skip_prefix(line, "status=", &value)) {
-		if (pair[0] && pair[0]->len && pair[1]) {
 			/* the last "status=<foo>" line wins */
-			if (!strcmp(pair[0]->buf, "status=")) {
+			strbuf_reset(status);
-				strbuf_reset(status);
+			strbuf_addstr(status, value);
-				strbuf_addbuf(status, pair[1]);
-			}
 		}
-		strbuf_list_free(pair);
 	}
 
 	return (len < 0) ? len : 0;

trace2/tr2_cfg.c

@@ -8,89 +8,65 @@
 #include "trace2/tr2_sysenv.h"
 #include "wildmatch.h"
 
-static struct strbuf **tr2_cfg_patterns;
+static struct string_list tr2_cfg_patterns = STRING_LIST_INIT_DUP;
-static int tr2_cfg_count_patterns;
 static int tr2_cfg_loaded;
 
-static struct strbuf **tr2_cfg_env_vars;
+static struct string_list tr2_cfg_env_vars = STRING_LIST_INIT_DUP;
-static int tr2_cfg_env_vars_count;
 static int tr2_cfg_env_vars_loaded;
 
 /*
  * Parse a string containing a comma-delimited list of config keys
- * or wildcard patterns into a list of strbufs.
+ * or wildcard patterns into a string list.
  */
-static int tr2_cfg_load_patterns(void)
+static size_t tr2_cfg_load_patterns(void)
 {
-	struct strbuf **s;
 	const char *envvar;
 
 	if (tr2_cfg_loaded)
-		return tr2_cfg_count_patterns;
+		return tr2_cfg_patterns.nr;
 	tr2_cfg_loaded = 1;
 
 	envvar = tr2_sysenv_get(TR2_SYSENV_CFG_PARAM);
 	if (!envvar || !*envvar)
-		return tr2_cfg_count_patterns;
+		return tr2_cfg_patterns.nr;
 
-	tr2_cfg_patterns = strbuf_split_buf(envvar, strlen(envvar), ',', -1);
+	string_list_split_f(&tr2_cfg_patterns, envvar, ",", -1,
-	for (s = tr2_cfg_patterns; *s; s++) {
+			    STRING_LIST_SPLIT_TRIM);
-		struct strbuf *buf = *s;
+	return tr2_cfg_patterns.nr;
-
-		if (buf->len && buf->buf[buf->len - 1] == ',')
-			strbuf_setlen(buf, buf->len - 1);
-		strbuf_trim_trailing_newline(*s);
-		strbuf_trim(*s);
-	}
-
-	tr2_cfg_count_patterns = s - tr2_cfg_patterns;
-	return tr2_cfg_count_patterns;
 }
 
 void tr2_cfg_free_patterns(void)
 {
-	if (tr2_cfg_patterns)
+	if (tr2_cfg_patterns.nr)
-		strbuf_list_free(tr2_cfg_patterns);
+		string_list_clear(&tr2_cfg_patterns, 0);
-	tr2_cfg_count_patterns = 0;
 	tr2_cfg_loaded = 0;
 }
 
 /*
  * Parse a string containing a comma-delimited list of environment variable
- * names into a list of strbufs.
+ * names into a string list.
  */
-static int tr2_load_env_vars(void)
+static size_t tr2_load_env_vars(void)
 {
-	struct strbuf **s;
 	const char *varlist;
 
 	if (tr2_cfg_env_vars_loaded)
-		return tr2_cfg_env_vars_count;
+		return tr2_cfg_env_vars.nr;
 	tr2_cfg_env_vars_loaded = 1;
 
 	varlist = tr2_sysenv_get(TR2_SYSENV_ENV_VARS);
 	if (!varlist || !*varlist)
-		return tr2_cfg_env_vars_count;
+		return tr2_cfg_env_vars.nr;
 
-	tr2_cfg_env_vars = strbuf_split_buf(varlist, strlen(varlist), ',', -1);
+	string_list_split_f(&tr2_cfg_env_vars, varlist, ",", -1,
-	for (s = tr2_cfg_env_vars; *s; s++) {
+			    STRING_LIST_SPLIT_TRIM);
-		struct strbuf *buf = *s;
+	return tr2_cfg_env_vars.nr;
-
-		if (buf->len && buf->buf[buf->len - 1] == ',')
-			strbuf_setlen(buf, buf->len - 1);
-		strbuf_trim_trailing_newline(*s);
-		strbuf_trim(*s);
-	}
-
-	tr2_cfg_env_vars_count = s - tr2_cfg_env_vars;
-	return tr2_cfg_env_vars_count;
 }
 
 void tr2_cfg_free_env_vars(void)
 {
-	if (tr2_cfg_env_vars)
+	if (tr2_cfg_env_vars.nr)
-		strbuf_list_free(tr2_cfg_env_vars);
+		string_list_clear(&tr2_cfg_env_vars, 0);
-	tr2_cfg_env_vars_count = 0;
 	tr2_cfg_env_vars_loaded = 0;
 }
 
@@ -105,12 +81,11 @@ struct tr2_cfg_data {
 static int tr2_cfg_cb(const char *key, const char *value,
 		      const struct config_context *ctx, void *d)
 {
-	struct strbuf **s;
+	struct string_list_item *item;
 	struct tr2_cfg_data *data = (struct tr2_cfg_data *)d;
 
-	for (s = tr2_cfg_patterns; *s; s++) {
+	for_each_string_list_item(item, &tr2_cfg_patterns) {
-		struct strbuf *buf = *s;
+		int wm = wildmatch(item->string, key, WM_CASEFOLD);
-		int wm = wildmatch(buf->buf, key, WM_CASEFOLD);
 		if (wm == WM_MATCH) {
 			trace2_def_param_fl(data->file, data->line, key, value,
 					    ctx->kvi);
@@ -132,17 +107,16 @@ void tr2_cfg_list_config_fl(const char *file, int line)
 void tr2_list_env_vars_fl(const char *file, int line)
 {
 	struct key_value_info kvi = KVI_INIT;
-	struct strbuf **s;
+	struct string_list_item *item;
 
 	kvi_from_param(&kvi);
 	if (tr2_load_env_vars() <= 0)
 		return;
 
-	for (s = tr2_cfg_env_vars; *s; s++) {
+	for_each_string_list_item(item, &tr2_cfg_env_vars) {
-		struct strbuf *buf = *s;
+		const char *val = getenv(item->string);
-		const char *val = getenv(buf->buf);
 		if (val && *val)
-			trace2_def_param_fl(file, line, buf->buf, val, &kvi);
+			trace2_def_param_fl(file, line, item->string, val, &kvi);
 	}
 }
 

wt-status.c

@@ -1352,8 +1352,8 @@ static int split_commit_in_progress(struct wt_status *s)
  */
 static void abbrev_oid_in_line(struct strbuf *line)
 {
-	struct strbuf **split;
+	struct string_list split = STRING_LIST_INIT_DUP;
-	int i;
+	struct object_id oid;
 
 	if (starts_with(line->buf, "exec ") ||
 	    starts_with(line->buf, "x ") ||
@@ -1361,26 +1361,15 @@ static void abbrev_oid_in_line(struct strbuf *line)
 	    starts_with(line->buf, "l "))
 		return;
 
-	split = strbuf_split_max(line, ' ', 3);
+	if ((2 <= string_list_split(&split, line->buf, " ", 2)) &&
-	if (split[0] && split[1]) {
+	    !repo_get_oid(the_repository, split.items[1].string, &oid)) {
-		struct object_id oid;
+		strbuf_reset(line);
-
+		strbuf_addf(line, "%s ", split.items[0].string);
-		/*
+		strbuf_add_unique_abbrev(line, &oid, DEFAULT_ABBREV);
-		 * strbuf_split_max left a space. Trim it and re-add
+		for (size_t i = 2; i < split.nr; i++)
-		 * it after abbreviation.
+			strbuf_addf(line, " %s", split.items[i].string);
-		 */
-		strbuf_trim(split[1]);
-		if (!repo_get_oid(the_repository, split[1]->buf, &oid)) {
-			strbuf_reset(split[1]);
-			strbuf_add_unique_abbrev(split[1], &oid,
-						 DEFAULT_ABBREV);
-			strbuf_addch(split[1], ' ');
-			strbuf_reset(line);
-			for (i = 0; split[i]; i++)
-				strbuf_addbuf(line, split[i]);
-		}
 	}
-	strbuf_list_free(split);
+	string_list_clear(&split, 0);
 }
 
 static int read_rebase_todolist(const char *fname, struct string_list *lines)