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 0/2] avoid unnecessary strbuf_split*() and strbuf-by-value usage
Date: Wed, 11 Mar 2026 13:20:39 +0000 [thread overview]
Message-ID: <20260311132041.12044-1-deveshigurgaon@gmail.com> (raw)
Junio's "do not overuse strbuf_split*()" series calls out remaining
uses of strbuf_split*() as leftover bits for others to continue.
This series picks up two of them.
write_worktree_linking_files() takes two struct strbuf parameters by
value even though it only needs plain path strings.
parse_combine_filter() in list-objects-filter-options.c uses
strbuf_split_str() to split a combine: filter spec at '+'. An array
of strbufs is unnecessary; walking the string directly with
strchrnul() is simpler and cleaner.
Changes since v1:
* Patch 1/2: no changes.
* Patch 2/2: Incorporate review feedback from Jeff King.
- Use strchrnul() instead of strchr() so the loop needs no
separate "found separator?" branch or strlen() fallback.
- Drop the unnecessary (size_t) cast on the length expression.
- Always exclude '+' from the sub-spec via end - p, removing the
incorrect conditional stripping. A trailing '+' now causes the
while (*p) condition to fail cleanly on the next iteration
rather than passing an empty string to the parser.
- Remove the test that expected an error on trailing '+', since
that behavior was incorrect.
Deveshi Dwivedi (2):
worktree: do not pass strbuf by value
list-objects-filter-options: avoid strbuf_split_str()
builtin/worktree.c | 2 +-
list-objects-filter-options.c | 35 +++++++++++++----------------
t/t6112-rev-list-filters-objects.sh | 4 ----
worktree.c | 22 +++++++++---------
worktree.h | 2 +-
5 files changed, 28 insertions(+), 37 deletions(-)
Range-diff against v1:
1: ee6b7d1e6a = 1: ee6b7d1e6a worktree: do not pass strbuf by value
2: 386aed0adf ! 2: c04ddaeb95 list-objects-filter-options: avoid strbuf_split_str()
@@ Commit message
parse_combine_subfilter(), only read the string content of the strbuf
they receive.
- Walk the input string directly with strchr() to find each '+'. Copy
- each sub-spec into a temporary buffer and strip the '+' only when
- another sub-spec follows. Change the helpers to take const char *
- instead of struct strbuf *.
+ 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: static int parse_combine_filter(
- result = parse_combine_subfilter(
- filter_options, subspecs[sub], errbuf);
+ while (*p && !result) {
-+ const char *sep = strchr(p, '+');
-+ size_t len = sep ? (size_t)(sep - p + 1) : strlen(p);
-+ char *sub = xmemdupz(p, len);
-+
-+ /* strip '+' separator, but only when more sub-specs follow */
-+ if (sep && *(sep + 1))
-+ sub[len - 1] = '\0';
++ const char *end = strchrnul(p, '+');
++ char *sub = xmemdupz(p, end - p);
+
+ result = parse_combine_subfilter(filter_options, sub, errbuf);
+ free(sub);
-+ if (!sep)
++ if (!*end)
+ break;
-+ p = sep + 1;
++ p = end + 1;
}
filter_options->choice = LOFC_COMBINE;
@@ list-objects-filter-options.c: static int parse_combine_filter(
if (result)
list_objects_filter_release(filter_options);
return result;
+
+ ## t/t6112-rev-list-filters-objects.sh ##
+@@ t/t6112-rev-list-filters-objects.sh: 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 reply other threads:[~2026-03-11 13:20 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-11 13:20 Deveshi Dwivedi [this message]
2026-03-11 13:20 ` [PATCH v2 1/2] worktree: do not pass strbuf by value Deveshi Dwivedi
2026-03-11 13:20 ` [PATCH v2 2/2] list-objects-filter-options: avoid strbuf_split_str() Deveshi Dwivedi
2026-03-11 16:28 ` 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-1-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