list-objects-filter-options: avoid strbuf_split_str()

parse_combine_filter() splits a combine: filter spec at '+' using strbuf_split_str(), which yields an array of strbufs with the delimiter left at the end of each non-final piece. The code then mutates each non-final piece to strip the trailing '+' before parsing. Allocating an array of strbufs is unnecessary. The function processes one sub-spec at a time and does not use strbuf editing on the pieces. The two helpers it calls, has_reserved_character() and parse_combine_subfilter(), only read the string content of the strbuf they receive. Walk the input string directly with strchrnul() to find each '+', copying each sub-spec into a reusable temporary buffer. The '+' delimiter is naturally excluded. Empty sub-specs (e.g. from a trailing '+') are silently skipped for consistency. Change the helpers to take const char * instead of struct strbuf *. The test that expected an error on a trailing '+' is removed, since that behavior was incorrect. Signed-off-by: Deveshi Dwivedi <deveshigurgaon@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Deveshi Dwivedi committed Mar 11, 2026 at 17:33 UTC f21967e5415673824d501b318a252c6e8a91d6fb
2 files changed +20 -24
list-objects-filter-options.c
+20 -20
@@ -125,9 +125,9 @@ int gently_parse_list_objects_filter(
125 static const char *RESERVED_NON_WS = "~`!@#$^&*()[]{}\\;'\",<>?";
126
127 static int has_reserved_character(
128 - struct strbuf *sub_spec, struct strbuf *errbuf)
128 + const char *sub_spec, struct strbuf *errbuf)
129 {
130 - const char *c = sub_spec->buf;
130 + const char *c = sub_spec;
131 while (*c) {
132 if (*c <= ' ' || strchr(RESERVED_NON_WS, *c)) {
133 strbuf_addf(
@@ -144,7 +144,7 @@ static int has_reserved_character(
144
145 static int parse_combine_subfilter(
146 struct list_objects_filter_options *filter_options,
147 - struct strbuf *subspec,
147 + const char *subspec,
148 struct strbuf *errbuf)
149 {
150 size_t new_index = filter_options->sub_nr;
@@ -155,7 +155,7 @@ static int parse_combine_subfilter(
155 filter_options->sub_alloc);
156 list_objects_filter_init(&filter_options->sub[new_index]);
157
158 - decoded = url_percent_decode(subspec->buf);
158 + decoded = url_percent_decode(subspec);
159
160 result = has_reserved_character(subspec, errbuf);
161 if (result)
@@ -182,34 +182,34 @@ static int parse_combine_filter(
182 const char *arg,
183 struct strbuf *errbuf)
184 {
185 - struct strbuf **subspecs = strbuf_split_str(arg, '+', 0);
186 - size_t sub;
185 + const char *p = arg;
186 + struct strbuf sub = STRBUF_INIT;
187 int result = 0;
188
189 - if (!subspecs[0]) {
189 + if (!*p) {
190 strbuf_addstr(errbuf, _("expected something after combine:"));
191 result = 1;
192 goto cleanup;
193 }
194
195 - for (sub = 0; subspecs[sub] && !result; sub++) {
196 - if (subspecs[sub + 1]) {
197 - /*
198 - * This is not the last subspec. Remove trailing "+" so
199 - * we can parse it.
200 - */
201 - size_t last = subspecs[sub]->len - 1;
202 - assert(subspecs[sub]->buf[last] == '+');
203 - strbuf_remove(subspecs[sub], last, 1);
204 - }
205 - result = parse_combine_subfilter(
206 - filter_options, subspecs[sub], errbuf);
195 + while (*p && !result) {
196 + const char *end = strchrnul(p, '+');
197 +
198 + strbuf_reset(&sub);
199 + strbuf_add(&sub, p, end - p);
200 +
201 + if (sub.len)
202 + result = parse_combine_subfilter(filter_options, sub.buf, errbuf);
203 +
204 + if (!*end)
205 + break;
206 + p = end + 1;
207 }
208 + strbuf_release(&sub);
209
210 filter_options->choice = LOFC_COMBINE;
211
212 cleanup:
212 - strbuf_list_free(subspecs);
213 if (result)
214 list_objects_filter_release(filter_options);
215 return result;
t/t6112-rev-list-filters-objects.sh
-4
@@ -483,10 +483,6 @@ test_expect_success 'combine:... with non-encoded reserved chars' '
483 "must escape char in sub-filter-spec: .\~."
484 '
485
486 -test_expect_success 'validate err msg for "combine:<valid-filter>+"' '
487 - expect_invalid_filter_spec combine:tree:2+ "expected .tree:<depth>."
488 -'
489 -
486 test_expect_success 'combine:... with edge-case hex digits: Ff Aa 0 9' '
487 git -C r3 rev-list --objects --filter="combine:tree:2+bl%6Fb:n%6fne" \
488 HEAD >actual &&