All of lore.kernel.org
 help / color / mirror / Atom feed
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


             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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.