git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[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
Previous: Junio C HamanoNext: Deveshi Dwivedi
Message 7 of 11 in “avoid unnecessary strbuf_split*() and strbuf-by-value usage”
  1. 0/2 avoid unnecessary strbuf_split*() and strbuf-by-value usageDeveshi Dwivedi, Mar 11, 2026
  2. 1/2 worktree: do not pass strbuf by valueDeveshi Dwivedi, Mar 11, 2026
  3. 2/2 list-objects-filter-options: avoid strbuf_split_str()Deveshi Dwivedi, Mar 11, 2026
  4. Junio C HamanoMar 11, 2026
  5. Jeff KingMar 11, 2026
  6. Junio C HamanoMar 11, 2026
  7. 0/2 avoid unnecessary strbuf_split*() and strbuf-by-value usageDeveshi Dwivedi, Mar 11, 2026
  8. 1/2 worktree: do not pass strbuf by valueDeveshi Dwivedi, Mar 11, 2026
  9. 2/2 list-objects-filter-options: avoid strbuf_split_str()Deveshi Dwivedi, Mar 11, 2026
  10. Jeff KingMar 11, 2026
  11. Junio C HamanoMar 11, 2026

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.