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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox