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>

diff --git a/list-objects-filter-options.c b/list-objects-filter-options.c
index 7f3e7b8f505ebc5eabde4fb35bfe4821efafa5e8..cef67e591954a90ffd0340e42ee3358f5599d0e2 100644 (file)
--- a/list-objects-filter-options.c
+++ b/list-objects-filter-options.c
@@ -125,9 +125,9 @@ int gently_parse_list_objects_filter(
 static const char *RESERVED_NON_WS = "~`!@#$^&*()[]{}\\;'\",<>?";
 
 static int has_reserved_character(
-       struct strbuf *sub_spec, struct strbuf *errbuf)
+       const char *sub_spec, struct strbuf *errbuf)
 {
-       const char *c = sub_spec->buf;
+       const char *c = sub_spec;
        while (*c) {
                if (*c <= ' ' || strchr(RESERVED_NON_WS, *c)) {
                        strbuf_addf(
@@ -144,7 +144,7 @@ static int has_reserved_character(
 
 static int parse_combine_subfilter(
        struct list_objects_filter_options *filter_options,
-       struct strbuf *subspec,
+       const char *subspec,
        struct strbuf *errbuf)
 {
        size_t new_index = filter_options->sub_nr;
@@ -155,7 +155,7 @@ static int parse_combine_subfilter(
                      filter_options->sub_alloc);
        list_objects_filter_init(&filter_options->sub[new_index]);
 
-       decoded = url_percent_decode(subspec->buf);
+       decoded = url_percent_decode(subspec);
 
        result = has_reserved_character(subspec, errbuf);
        if (result)
@@ -182,34 +182,34 @@ static int parse_combine_filter(
        const char *arg,
        struct strbuf *errbuf)
 {
-       struct strbuf **subspecs = strbuf_split_str(arg, '+', 0);
-       size_t sub;
+       const char *p = arg;
+       struct strbuf sub = STRBUF_INIT;
        int result = 0;
 
-       if (!subspecs[0]) {
+       if (!*p) {
                strbuf_addstr(errbuf, _("expected something after combine:"));
                result = 1;
                goto cleanup;
        }
 
-       for (sub = 0; subspecs[sub] && !result; sub++) {
-               if (subspecs[sub + 1]) {
-                       /*
-                        * This is not the last subspec. Remove trailing "+" so
-                        * we can parse it.
-                        */
-                       size_t last = subspecs[sub]->len - 1;
-                       assert(subspecs[sub]->buf[last] == '+');
-                       strbuf_remove(subspecs[sub], last, 1);
-               }
-               result = parse_combine_subfilter(
-                       filter_options, subspecs[sub], errbuf);
+       while (*p && !result) {
+               const char *end = strchrnul(p, '+');
+
+               strbuf_reset(&sub);
+               strbuf_add(&sub, p, end - p);
+
+               if (sub.len)
+                       result = parse_combine_subfilter(filter_options, sub.buf, errbuf);
+
+               if (!*end)
+                       break;
+               p = end + 1;
        }
+       strbuf_release(&sub);
 
        filter_options->choice = LOFC_COMBINE;
 
 cleanup:
-       strbuf_list_free(subspecs);
        if (result)
                list_objects_filter_release(filter_options);
        return result;

diff --git a/t/t6112-rev-list-filters-objects.sh b/t/t6112-rev-list-filters-objects.sh
index 0387f35a326d741b26eb4795199bb42e3c4b8b7e..39211ef989cc58f10454f5beb3f2705602d39620 100755 (executable)
--- a/t/t6112-rev-list-filters-objects.sh
+++ b/t/t6112-rev-list-filters-objects.sh
@@ -483,10 +483,6 @@ test_expect_success 'combine:... with non-encoded reserved chars' '
                "must escape char in sub-filter-spec: .\~."
 '
 
-test_expect_success 'validate err msg for "combine:<valid-filter>+"' '
-       expect_invalid_filter_spec combine:tree:2+ "expected .tree:<depth>."
-'
-
 test_expect_success 'combine:... with edge-case hex digits: Ff Aa 0 9' '
        git -C r3 rev-list --objects --filter="combine:tree:2+bl%6Fb:n%6fne" \
                HEAD >actual &&