git/list[1] front-page[2] threads[3] people[4] search[5] about
wed 2026-10-07 18:10 UTC

Re: [PATCH] trailer: change strbuf in-place in unfold_value()

From
Ramsay Jones <ramsay@ramsayjones.plus.com>
Date
May 14, 2026, 21:30 UTC
Message-ID
<a4da346d-3800-40ea-8828-970b15088bf3@ramsayjones.plus.com>
In-Reply-To
<9629b0c1-b28f-4cd2-8d59-67d909ca9052@web.de>
On 14/05/2026 7:40 pm, René Scharfe wrote:
Show 38 quoted lines
> Avoid an allocation by doing s/\n\s*/ /g (replacing NL and any following
> whitespace with a SP) right in the strbuf instead of copying the result
> to a temporary one and swapping them in the end.  We can safely do that
> because the replacement is never longer than the original string.
> 
> Signed-off-by: René Scharfe <l.s.r@web.de>
> ---
> Formatted with --function-context for easier review.
> Inspired by https://lore.kernel.org/git/20260513185408.GA147423@coredump.intra.peff.net/
> 
>  trailer.c | 16 ++++++----------
>  1 file changed, 6 insertions(+), 10 deletions(-)
> 
> diff --git a/trailer.c b/trailer.c
> index 470f86a4a2..b89fa12fe7 100644
> --- a/trailer.c
> +++ b/trailer.c
> @@ -988,29 +988,25 @@ static int ends_with_blank_line(const char *buf, size_t len)
>  
>  static void unfold_value(struct strbuf *val)
>  {
> -	struct strbuf out = STRBUF_INIT;
>  	size_t i;
> +	size_t pos = 0;
>  
> -	strbuf_grow(&out, val->len);
>  	i = 0;
>  	while (i < val->len) {
>  		char c = val->buf[i++];
>  		if (c == '\n') {
>  			/* Collapse continuation down to a single space. */
>  			while (i < val->len && isspace(val->buf[i]))
>  				i++;
> -			strbuf_addch(&out, ' ');
> -		} else {
> -			strbuf_addch(&out, c);
> +			val->buf[pos++] = ' ';
> +		} else if (pos != i) {

Hmm, isn't 'pos' strictly (always) less than 'i' here? (note the post update of 'i' when setting 'c' at the head of the loop).

> +			val->buf[pos++] = c;
So, this (non-newline-or-'trailing'-space char) is always copied.

Not that it matters much (depending on how long the first line is, I doubt the difference is measurable :) ).

[Unless I'm not reading it correctly, of course - in which case, oops!]

ATB, Ramsay Jones

Show 14 quoted lines
>  		}
>  	}
> +	strbuf_setlen(val, pos);
>  
>  	/* Empty lines may have left us with whitespace cruft at the edges */
> -	strbuf_trim(&out);
> -
> -	/* output goes back to val as if we modified it in-place */
> -	strbuf_swap(&out, val);
> -	strbuf_release(&out);
> +	strbuf_trim(val);
>  }
>  
>  static struct trailer_block *trailer_block_new(void)
Previous: René ScharfeNext: Jeff King
Message 2 of 6 in “trailer: change strbuf in-place in unfold_value()”
  1. trailer: change strbuf in-place in unfold_value()René Scharfe, May 14, 2026
  2. Ramsay JonesMay 14, 2026
  3. Jeff KingMay 15, 2026
  4. Jeff KingMay 15, 2026
  5. René ScharfeMay 15, 2026
  6. trailer: change strbuf in-place in unfold_value()René Scharfe, May 15, 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.