@matheusbsbs / matheus-bagliano / commits / 52acddf36c

string-list: multi-delimiter `string_list_split_in_place()`

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> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Taylor Blau committed Apr 24, 2023 at 18:20 UTC 52acddf36c8cb3778ab2098a0d95cc2e375a4069

8 files changed +61 -10

builtin/gc.c

+2 -2

@@ -1687,11 +1687,11 @@ static int get_schedule_cmd(const char **cmd, int *is_available)

1687 if (is_available)

1688 *is_available = 0;

1689

1690 - string_list_split_in_place(&list, testing, ',', -1);

1690 + string_list_split_in_place(&list, testing, ",", -1);

1691 for_each_string_list_item(item, &list) {

1692 struct string_list pair = STRING_LIST_INIT_NODUP;

1693

1694 - if (string_list_split_in_place(&pair, item->string, ':', 2) != 2)

1694 + if (string_list_split_in_place(&pair, item->string, ":", 2) != 2)

1695 continue;

1696

1697 if (!strcmp(*cmd, pair.items[0].string)) {

diff.c

+1 -1

@@ -134,7 +134,7 @@ static int parse_dirstat_params(struct diff_options *options, const char *params

134 int i;

135

136 if (*params_copy)

137 - string_list_split_in_place(&params, params_copy, ',', -1);

137 + string_list_split_in_place(&params, params_copy, ",", -1);

138 for (i = 0; i < params.nr; i++) {

139 const char *p = params.items[i].string;

140 if (!strcmp(p, "changes")) {

notes.c

+1 -1

@@ -963,7 +963,7 @@ void string_list_add_refs_from_colon_sep(struct string_list *list,

963 char *globs_copy = xstrdup(globs);

964 int i;

965

966 - string_list_split_in_place(&split, globs_copy, ':', -1);

966 + string_list_split_in_place(&split, globs_copy, ":", -1);

967 string_list_remove_empty_items(&split, 0);

968

969 for (i = 0; i < split.nr; i++)

refs/packed-backend.c

+1 -1

@@ -650,7 +650,7 @@ static struct snapshot *create_snapshot(struct packed_ref_store *refs)

650 snapshot->buf,

651 snapshot->eof - snapshot->buf);

652

653 - string_list_split_in_place(&traits, p, ' ', -1);

653 + string_list_split_in_place(&traits, p, " ", -1);

654

655 if (unsorted_string_list_has_string(&traits, "fully-peeled"))

656 snapshot->peeled = PEELED_FULLY;

string-list.c

+2 -2

@@ -301,7 +301,7 @@ int string_list_split(struct string_list *list, const char *string,

301 }

302

303 int string_list_split_in_place(struct string_list *list, char *string,

304 - int delim, int maxsplit)

304 + const char *delim, int maxsplit)

305 {

306 int count = 0;

307 char *p = string, *end;

@@ -315,7 +315,7 @@ int string_list_split_in_place(struct string_list *list, char *string,

315 string_list_append(list, p);

316 return count;

317 }

318 - end = strchr(p, delim);

318 + end = strpbrk(p, delim);

319 if (end) {

320 *end = '\0';

321 string_list_append(list, p);

string-list.h

+1 -1

@@ -270,5 +270,5 @@ int string_list_split(struct string_list *list, const char *string,

270 * list->strdup_strings must *not* be set.

271 */

272 int string_list_split_in_place(struct string_list *list, char *string,

273 - int delim, int maxsplit);

273 + const char *delim, int maxsplit);

274 #endif /* STRING_LIST_H */

t/helper/test-string-list.c

+2 -2

@@ -62,7 +62,7 @@ int cmd__string_list(int argc, const char **argv)

62 struct string_list list = STRING_LIST_INIT_NODUP;

63 int i;

64 char *s = xstrdup(argv[2]);

65 - int delim = *argv[3];

65 + const char *delim = argv[3];

66 int maxsplit = atoi(argv[4]);

68 i = string_list_split_in_place(&list, s, delim, maxsplit);

@@ -111,7 +111,7 @@ int cmd__string_list(int argc, const char **argv)

111 */

112 if (sb.len && sb.buf[sb.len - 1] == '\n')

113 strbuf_setlen(&sb, sb.len - 1);

114 - string_list_split_in_place(&list, sb.buf, '\n', -1);

114 + string_list_split_in_place(&list, sb.buf, "\n", -1);

115

116 string_list_sort(&list);

117

t/t0063-string-list.sh

+51

@@ -18,6 +18,14 @@ test_split () {

18 "

19 }

21 +test_split_in_place() {

22 + cat >expected &&

23 + test_expect_success "split (in place) $1 at $2, max $3" "

24 + test-tool string-list split_in_place '$1' '$2' '$3' >actual &&

25 + test_cmp expected actual

26 + "

27 +}

28 +

29 test_split "foo:bar:baz" ":" "-1" <<EOF

30 3

31 [0]: "foo"

@@ -61,6 +69,49 @@ test_split ":" ":" "-1" <<EOF

69 [1]: ""

70 EOF

72 +test_split_in_place "foo:;:bar:;:baz:;:" ":;" "-1" <<EOF

73 +10

74 +[0]: "foo"

75 +[1]: ""

76 +[2]: ""

77 +[3]: "bar"

78 +[4]: ""

79 +[5]: ""

80 +[6]: "baz"

81 +[7]: ""

82 +[8]: ""

83 +[9]: ""

84 +EOF

85 +

86 +test_split_in_place "foo:;:bar:;:baz" ":;" "0" <<EOF

87 +1

88 +[0]: "foo:;:bar:;:baz"

89 +EOF

90 +

91 +test_split_in_place "foo:;:bar:;:baz" ":;" "1" <<EOF

92 +2

93 +[0]: "foo"

94 +[1]: ";:bar:;:baz"

95 +EOF

96 +

97 +test_split_in_place "foo:;:bar:;:baz" ":;" "2" <<EOF

98 +3

99 +[0]: "foo"

100 +[1]: ""

101 +[2]: ":bar:;:baz"

102 +EOF

103 +

104 +test_split_in_place "foo:;:bar:;:" ":;" "-1" <<EOF

105 +7

106 +[0]: "foo"

107 +[1]: ""

108 +[2]: ""

109 +[3]: "bar"

110 +[4]: ""

111 +[5]: ""

112 +[6]: ""

113 +EOF

114 +

115 test_expect_success "test filter_string_list" '

116 test "x-" = "x$(test-tool string-list filter - y)" &&

117 test "x-" = "x$(test-tool string-list filter no y)" &&