From: Patrick Steinhardt <ps@pks.im>
Date: Thu, 19 Mar 2026 07:57:55 GMT
Subject: Re: [PATCH] rerere: update to modern representation of empty strbufs
Message-ID: <abusg-X1vtd48UpZ@pks.im>
In-Reply-To: <xmqq341wnvbk.fsf@gitster.g>

On Thu, Mar 19, 2026 at 12:15:59AM -0700, Junio C Hamano wrote:
> Back when b4833a2c (rerere: Fix use of an empty strbuf.buf,
> 2007-09-26) was written, a freshly initialized empty strbuf
> had NULL in its .buf member, with .len set to 0.  The code this
> patch touches in rerere.c was written to _fix_ the original code
> that assumed that the .buf member is always pointing at a NUL-terminated
> string, even for an empty string, which did not hold back then.
> 
> That changed in b315c5c0 (strbuf change: be sure ->buf is never ever
> NULL., 2007-09-27), and it has again become safe to assume that .buf
> is never NULL, and .buf[0] has '\0' for an empty string (i.e., a
> strbuf with its .len member set to 0).

Right. We always require strbufs to be initialized, and that will always
cause us to set the `.buf` member.

> diff --git a/rerere.c b/rerere.c
> index 6ec55964e2..0296700f9f 100644
> --- a/rerere.c
> +++ b/rerere.c
> @@ -403,12 +403,8 @@ static int handle_conflict(struct strbuf *out, struct rerere_io *io,
>  			strbuf_addbuf(out, &two);
>  			rerere_strbuf_putconflict(out, '>', marker_size);
>  			if (ctx) {
> -				git_hash_update(ctx, one.buf ?
> -						one.buf : "",
> -						one.len + 1);
> -				git_hash_update(ctx, two.buf ?
> -						two.buf : "",
> -						two.len + 1);
> +				git_hash_update(ctx, one.buf, one.len + 1);
> +				git_hash_update(ctx, two.buf, two.len + 1);
>  			}

Yup, this should be indeed equivalent given that we declare the `.buf`
thing as `char strbuf_slopbuf[1]`. So we'll get a single NUL byte here,
which is the same as the empty string.

So this looks good to me, thanks!

Patrick