From: René Scharfe <l.s.r@web.de>
Date: Sat, 21 Mar 2026 20:47:18 GMT
Subject: Re: [RFC] cocci: .buf in a strbuf object can never be NULL
Message-ID: <3e387439-c066-4e45-b28b-43f77c8824d6@web.de>
In-Reply-To: <20260319233546.GA3632561@coredump.intra.peff.net>

On 3/20/26 12:35 AM, Jeff King wrote:
> 
> @@ -669,10 +673,13 @@ int strbuf_getwholeline(struct strbuf *sb, FILE *fp, int term)
>  	 * we can just re-init, but otherwise we should make sure that our
>  	 * length is empty, and that the result is NUL-terminated.
>  	 */
> -	if (!sb->buf)
> +	if (!buf)
>  		strbuf_init(sb, 0);
> -	else
> -		strbuf_reset(sb);
> +	else {
> +		sb->buf = buf;
> +		sb->alloc = alloc;
> +		strbuf_reset(&sb);
> +	}
>  	return EOF;
>  }
>  #else
> 
> So I don't know that it makes anything simpler. We have to copy the
> values back into the strbuf either way, and we still have to handle
> restoring the strbuf invariants. Even the strbuf_init() case is still
> needed, because we don't know whether getdelim() just didn't allocate
> (in which case we could leave the strbuf alone) or if it actually ate
> the allocation we passed in (which was just a copy of sb->buf).
And yet this function can turn an empty strbuf into an allocated one
without rolling it back on error, leaving code similar to this silly
example here leaking:

	int copy_one_line(FILE *in, FILE *out, int term)
	{
		struct strbuf sb = STRBUF_INIT;
		if (strbuf_getwholeline(&sb, in, term))
			return -1;
		fwrite(sb.buf, 1, sb.len, out);
		strbuf_release(&sb);
		return 0;
	}

Some strbuf functions restore the original state in such a case by
calling strbuf_release(), strbuf_getwholeline() doesn't.  If we are OK
with that then it could be simplified by growing the buffer upfront:

	int strbuf_getwholeline(struct strbuf *sb, FILE *fp, int term)
	{
		ssize_t r;

		strbuf_grow(sb, 0);
		errno = 0;
		r = getdelim(&sb->buf, &sb->alloc, term, fp);

		if (r > 0) {
			sb->len = r;
			return 0;
		}

		assert(r == -1);
		if (errno == ENOMEM)
			die("Out of memory, getdelim failed");
		strbuf_reset(sb);
		return EOF;
	}

René