From: Deveshi Dwivedi <deveshigurgaon@gmail.com>
Date: Wed, 11 Mar 2026 13:20:39 GMT
Subject: [PATCH v2 0/2] avoid unnecessary strbuf_split*() and strbuf-by-value usage
Message-ID: <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 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