git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v3 1/6] string-list: multi-delimiter `string_list_split_in_place()`

From
Taylor Blau <me@ttaylorr.com>
Date
Apr 24, 2023, 22:20 UTC
Message-ID
<59d3e778b6c80b6bd10c1126cdc3f7e86558f3e7.1682374789.git.me@ttaylorr.com>
In-Reply-To
<cover.1682374789.git.me@ttaylorr.com>

Enhance `string_list_split_in_place()` to accept multiple characters as delimiters instead of a single character.

Instead of using `strchr(2)` to locate the first occurrence of the given delimiter character, `string_list_split_in_place_multi()` uses `strcspn(2)` to move past the initial segment of characters comprised of any characters in the delimiting set.

When only a single delimiting character is provided, `strpbrk(2)` (which is implemented with `strcspn(2)`) has equivalent performance to `strchr(2)`. Modern `strcspn(2)` implementations treat an empty delimiter or the singleton delimiter as a special case and fall back to calling strchrnul(). Both glibc[1] and musl[2] implement `strcspn(2)` this way.

This change is one step to removing `strtok(2)` from the tree. Note that `string_list_split_in_place()` is not a strict replacement for `strtok()`, since it will happily turn sequential delimiter characters into empty entries in the resulting string_list. For example:

    string_list_split_in_place(&xs, "foo:;:bar:;:baz", ":;", -1)
would yield a string list of:
    ["foo", "", "", "bar", "", "", "baz"]

Callers that wish to emulate the behavior of strtok(2) more directly should call `string_list_remove_empty_items()` after splitting.

To avoid regressions for the new multi-character delimter cases, update t0063 in this patch as well.

[1]: https://sourceware.org/git/?p=glibc.git;a=blob;f=string/strcspn.c;hb=glibc-2.37#l35 [2]: https://git.musl-libc.org/cgit/musl/tree/src/string/strcspn.c?h=v1.2.3#n11

Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
 builtin/gc.c                |  4 +--
 diff.c                      |  2 +-
 notes.c                     |  2 +-
 refs/packed-backend.c       |  2 +-
 string-list.c               |  4 +--
 string-list.h               |  2 +-
 t/helper/test-string-list.c |  4 +--
 t/t0063-string-list.sh      | 51 +++++++++++++++++++++++++++++++++++++
 8 files changed, 61 insertions(+), 10 deletions(-)
diff --git a/builtin/gc.c b/builtin/gc.c
index edd98d35a5..f68e976704 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -1687,11 +1687,11 @@ static int get_schedule_cmd(const char **cmd, int *is_available)
 	if (is_available)
 		*is_available = 0;
 
-	string_list_split_in_place(&list, testing, ',', -1);
+	string_list_split_in_place(&list, testing, ",", -1);
 	for_each_string_list_item(item, &list) {
 		struct string_list pair = STRING_LIST_INIT_NODUP;
 
-		if (string_list_split_in_place(&pair, item->string, ':', 2) != 2)
+		if (string_list_split_in_place(&pair, item->string, ":", 2) != 2)
 			continue;
 
 		if (!strcmp(*cmd, pair.items[0].string)) {
diff --git a/diff.c b/diff.c
index 78b0fdd8ca..378a0248e1 100644
--- a/diff.c
+++ b/diff.c
@@ -134,7 +134,7 @@ static int parse_dirstat_params(struct diff_options *options, const char *params
 	int i;
 
 	if (*params_copy)
-		string_list_split_in_place(&params, params_copy, ',', -1);
+		string_list_split_in_place(&params, params_copy, ",", -1);
 	for (i = 0; i < params.nr; i++) {
 		const char *p = params.items[i].string;
 		if (!strcmp(p, "changes")) {
diff --git a/notes.c b/notes.c
index 45fb7f22d1..eee806f626 100644
--- a/notes.c
+++ b/notes.c
@@ -963,7 +963,7 @@ void string_list_add_refs_from_colon_sep(struct string_list *list,
 	char *globs_copy = xstrdup(globs);
 	int i;
 
-	string_list_split_in_place(&split, globs_copy, ':', -1);
+	string_list_split_in_place(&split, globs_copy, ":", -1);
 	string_list_remove_empty_items(&split, 0);
 
 	for (i = 0; i < split.nr; i++)
diff --git a/refs/packed-backend.c b/refs/packed-backend.c
index 1eba1015dd..cc903baa7e 100644
--- a/refs/packed-backend.c
+++ b/refs/packed-backend.c
@@ -650,7 +650,7 @@ static struct snapshot *create_snapshot(struct packed_ref_store *refs)
 					 snapshot->buf,
 					 snapshot->eof - snapshot->buf);
 
-		string_list_split_in_place(&traits, p, ' ', -1);
+		string_list_split_in_place(&traits, p, " ", -1);
 
 		if (unsorted_string_list_has_string(&traits, "fully-peeled"))
 			snapshot->peeled = PEELED_FULLY;
diff --git a/string-list.c b/string-list.c
index db473f273e..5f5b60fe1c 100644
--- a/string-list.c
+++ b/string-list.c
@@ -301,7 +301,7 @@ int string_list_split(struct string_list *list, const char *string,
 }
 
 int string_list_split_in_place(struct string_list *list, char *string,
-			       int delim, int maxsplit)
+			       const char *delim, int maxsplit)
 {
 	int count = 0;
 	char *p = string, *end;
@@ -315,7 +315,7 @@ int string_list_split_in_place(struct string_list *list, char *string,
 			string_list_append(list, p);
 			return count;
 		}
-		end = strchr(p, delim);
+		end = strpbrk(p, delim);
 		if (end) {
 			*end = '\0';
 			string_list_append(list, p);
diff --git a/string-list.h b/string-list.h
index c7b0d5d000..77854840f7 100644
--- a/string-list.h
+++ b/string-list.h
@@ -270,5 +270,5 @@ int string_list_split(struct string_list *list, const char *string,
  * list->strdup_strings must *not* be set.
  */
 int string_list_split_in_place(struct string_list *list, char *string,
-			       int delim, int maxsplit);
+			       const char *delim, int maxsplit);
 #endif /* STRING_LIST_H */
diff --git a/t/helper/test-string-list.c b/t/helper/test-string-list.c
index 2123dda85b..63df88575c 100644
--- a/t/helper/test-string-list.c
+++ b/t/helper/test-string-list.c
@@ -62,7 +62,7 @@ int cmd__string_list(int argc, const char **argv)
 		struct string_list list = STRING_LIST_INIT_NODUP;
 		int i;
 		char *s = xstrdup(argv[2]);
-		int delim = *argv[3];
+		const char *delim = argv[3];
 		int maxsplit = atoi(argv[4]);
 
 		i = string_list_split_in_place(&list, s, delim, maxsplit);
@@ -111,7 +111,7 @@ int cmd__string_list(int argc, const char **argv)
 		 */
 		if (sb.len && sb.buf[sb.len - 1] == '\n')
 			strbuf_setlen(&sb, sb.len - 1);
-		string_list_split_in_place(&list, sb.buf, '\n', -1);
+		string_list_split_in_place(&list, sb.buf, "\n", -1);
 
 		string_list_sort(&list);
 
diff --git a/t/t0063-string-list.sh b/t/t0063-string-list.sh
index 46d4839194..1fee6d9010 100755
--- a/t/t0063-string-list.sh
+++ b/t/t0063-string-list.sh
@@ -18,6 +18,14 @@ test_split () {
 	"
 }
 
+test_split_in_place() {
+	cat >expected &&
+	test_expect_success "split (in place) $1 at $2, max $3" "
+		test-tool string-list split_in_place '$1' '$2' '$3' >actual &&
+		test_cmp expected actual
+	"
+}
+
 test_split "foo:bar:baz" ":" "-1" <<EOF
 3
 [0]: "foo"
@@ -61,6 +69,49 @@ test_split ":" ":" "-1" <<EOF
 [1]: ""
 EOF
 
+test_split_in_place "foo:;:bar:;:baz:;:" ":;" "-1" <<EOF
+10
+[0]: "foo"
+[1]: ""
+[2]: ""
+[3]: "bar"
+[4]: ""
+[5]: ""
+[6]: "baz"
+[7]: ""
+[8]: ""
+[9]: ""
+EOF
+
+test_split_in_place "foo:;:bar:;:baz" ":;" "0" <<EOF
+1
+[0]: "foo:;:bar:;:baz"
+EOF
+
+test_split_in_place "foo:;:bar:;:baz" ":;" "1" <<EOF
+2
+[0]: "foo"
+[1]: ";:bar:;:baz"
+EOF
+
+test_split_in_place "foo:;:bar:;:baz" ":;" "2" <<EOF
+3
+[0]: "foo"
+[1]: ""
+[2]: ":bar:;:baz"
+EOF
+
+test_split_in_place "foo:;:bar:;:" ":;" "-1" <<EOF
+7
+[0]: "foo"
+[1]: ""
+[2]: ""
+[3]: "bar"
+[4]: ""
+[5]: ""
+[6]: ""
+EOF
+
 test_expect_success "test filter_string_list" '
 	test "x-" = "x$(test-tool string-list filter - y)" &&
 	test "x-" = "x$(test-tool string-list filter no y)" &&

-- 
2.40.0.380.gd2df7d2365
Previous: Taylor BlauNext: Taylor Blau
Message 33 of 47 in “banned: mark `strok()`, `strtok_r()` as banned”
  1. 0/5 banned: mark `strok()`, `strtok_r()` as bannedTaylor Blau, Apr 13, 2023
  2. 1/5 string-list: introduce `string_list_split_in_place_multi()`Taylor Blau, Apr 13, 2023
  3. Jeff KingApr 18, 2023
  4. Taylor BlauApr 18, 2023
  5. 2/5 t/helper/test-hashmap.c: avoid using `strtok()`Taylor Blau, Apr 13, 2023
  6. Jeff KingApr 18, 2023
  7. Taylor BlauApr 18, 2023
  8. 3/5 t/helper/test-oidmap.c: avoid using `strtok()`Taylor Blau, Apr 13, 2023
  9. 4/5 t/helper/test-json-writer.c: avoid using `strtok()`Taylor Blau, Apr 13, 2023
  10. 5/5 banned.h: mark `strtok()`, `strtok_r()` as bannedTaylor Blau, Apr 13, 2023
  11. Junio C HamanoApr 14, 2023
  12. Chris TorekApr 14, 2023
  13. Taylor BlauApr 14, 2023
  14. 0/6 banned: mark `strok()` as bannedTaylor Blau, Apr 18, 2023
  15. 1/6 string-list: introduce `string_list_split_in_place_multi()`Taylor Blau, Apr 18, 2023
  16. Junio C HamanoApr 18, 2023
  17. Taylor BlauApr 18, 2023
  18. Jeff KingApr 22, 2023
  19. René ScharfeApr 22, 2023
  20. Jeff KingApr 23, 2023
  21. Junio C HamanoApr 24, 2023
  22. Taylor BlauApr 23, 2023
  23. Taylor BlauApr 23, 2023
  24. 2/6 string-list: introduce `string_list_setlen()`Taylor Blau, Apr 18, 2023
  25. Jeff KingApr 22, 2023
  26. 3/6 t/helper/test-hashmap.c: avoid using `strtok()`Taylor Blau, Apr 18, 2023
  27. Jeff KingApr 22, 2023
  28. Taylor BlauApr 24, 2023
  29. 4/6 t/helper/test-oidmap.c: avoid using `strtok()`Taylor Blau, Apr 18, 2023
  30. 5/6 t/helper/test-json-writer.c: avoid using `strtok()`Taylor Blau, Apr 18, 2023
  31. 6/6 banned.h: mark `strtok()` as bannedTaylor Blau, Apr 18, 2023
  32. 0/6 banned: mark `strok()`, `strtok_r()` as bannedTaylor Blau, Apr 24, 2023
  33. 1/6 string-list: multi-delimiter `string_list_split_in_place()`Taylor Blau, Apr 24, 2023
  34. 2/6 string-list: introduce `string_list_setlen()`Taylor Blau, Apr 24, 2023
  35. Jeff KingApr 25, 2023
  36. Taylor BlauApr 25, 2023
  37. 3/6 t/helper/test-hashmap.c: avoid using `strtok()`Taylor Blau, Apr 24, 2023
  38. 4/6 t/helper/test-oidmap.c: avoid using `strtok()`Taylor Blau, Apr 24, 2023
  39. 5/6 t/helper/test-json-writer.c: avoid using `strtok()`Taylor Blau, Apr 24, 2023
  40. Jeff HostetlerApr 25, 2023
  41. 6/6 banned.h: mark `strtok()` and `strtok_r()` as bannedTaylor Blau, Apr 24, 2023
  42. Chris TorekApr 24, 2023
  43. Taylor BlauApr 24, 2023
  44. Jeff KingApr 25, 2023
  45. Taylor BlauApr 25, 2023
  46. Jeff KingApr 25, 2023
  47. Taylor BlauApr 25, 2023

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.