[PATCH v3 0/2] avoid unnecessary strbuf_split*() and strbuf-by-value usage
- From
Deveshi Dwivedi <deveshigurgaon@gmail.com>
- Date
- Mar 11, 2026, 17:33 UTC
- Message-ID
- <20260311173336.8395-1-deveshigurgaon@gmail.com>
- In-Reply-To
- <20260311132041.12044-1-deveshigurgaon@gmail.com>
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 v2:
* Patch 1/2: no changes.
* Patch 2/2: Incorporate review feedback from Junio C Hamano.
- Reuse a single strbuf instead of xmemdupz()/free() per
iteration to avoid repeated allocations.
- Skip empty sub-specs consistently (e.g. "foo++bar" or "foo+")
instead of only handling the trailing case.
- Trim commit message to focus less on implementation details.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 | 40 ++++++++++++++--------------- t/t6112-rev-list-filters-objects.sh | 4 --- worktree.c | 22 ++++++++-------- worktree.h | 2 +- 5 files changed, 33 insertions(+), 37 deletions(-)
Range-diff against v2:
1: ee6b7d1e6a = 1: ee6b7d1e6a worktree: do not pass strbuf by value
2: c04ddaeb95 ! 2: 9f8690b9c7 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 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 *.
+ 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.
@@ list-objects-filter-options.c: static int parse_combine_filter(
- 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]) {
@@ list-objects-filter-options.c: static int parse_combine_filter(
- 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);
++ 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;
-- 2.52.0.230.gd8af7cadaa