From: Deveshi Dwivedi <deveshigurgaon@gmail.com>
To: git@vger.kernel.org
Cc: gitster@pobox.com, peff@peff.net,
Deveshi Dwivedi <deveshigurgaon@gmail.com>
Subject: [PATCH v2 2/2] list-objects-filter-options: avoid strbuf_split_str()
Date: Wed, 11 Mar 2026 13:20:41 +0000 [thread overview]
Message-ID: <20260311132041.12044-3-deveshigurgaon@gmail.com> (raw)
In-Reply-To: <20260311132041.12044-1-deveshigurgaon@gmail.com>
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 '+'.
strchrnul() returns a pointer to the terminating '\0' when the
delimiter is not found, so no separate "found separator?" branch is
needed. Copy each sub-spec into a temporary buffer using end - p,
which naturally excludes the '+', so the separator is always stripped
cleanly. A trailing '+' causes the outer while (*p) test to fail on
the next iteration rather than passing an empty string to the parser.
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>
---
list-objects-filter-options.c | 35 +++++++++++++----------------
t/t6112-rev-list-filters-objects.sh | 4 ----
2 files changed, 15 insertions(+), 24 deletions(-)
diff --git a/list-objects-filter-options.c b/list-objects-filter-options.c
index 7f3e7b8f50..616c6c7faa 100644
--- 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,29 @@ 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;
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, '+');
+ char *sub = xmemdupz(p, end - p);
+
+ result = parse_combine_subfilter(filter_options, sub, errbuf);
+ free(sub);
+ if (!*end)
+ break;
+ p = end + 1;
}
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 0387f35a32..39211ef989 100755
--- 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 &&
--
2.52.0.230.gd8af7cadaa
next prev parent reply other threads:[~2026-03-11 13:21 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-11 13:20 [PATCH v2 0/2] avoid unnecessary strbuf_split*() and strbuf-by-value usage Deveshi Dwivedi
2026-03-11 13:20 ` [PATCH v2 1/2] worktree: do not pass strbuf by value Deveshi Dwivedi
2026-03-11 13:20 ` Deveshi Dwivedi [this message]
2026-03-11 16:28 ` [PATCH v2 2/2] list-objects-filter-options: avoid strbuf_split_str() Junio C Hamano
2026-03-11 17:45 ` Jeff King
2026-03-11 18:07 ` Junio C Hamano
2026-03-11 17:33 ` [PATCH v3 0/2] avoid unnecessary strbuf_split*() and strbuf-by-value usage Deveshi Dwivedi
2026-03-11 17:33 ` [PATCH v3 1/2] worktree: do not pass strbuf by value Deveshi Dwivedi
2026-03-11 17:33 ` [PATCH v3 2/2] list-objects-filter-options: avoid strbuf_split_str() Deveshi Dwivedi
2026-03-11 17:48 ` Jeff King
2026-03-11 18:13 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260311132041.12044-3-deveshigurgaon@gmail.com \
--to=deveshigurgaon@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.