Re: [RFC PATCH 07/15] strbuf.c: placate -fanalyzer in strbuf_grow()

From: René Scharfe <l.s.r@web.de>
Date: Jun 4, 2022, 12:24 UTC
Message-ID: <bb05e942-eccd-5f88-d3c7-de903464d866@web.de>
In-Reply-To: <RFC-patch-07.15-cf1a5f3ed0f-20220603T183608Z-avarab@gmail.com>

Am 03.06.22 um 20:37 schrieb Ævar Arnfjörð Bjarmason:

Show 24 quoted lines

> Change the strbuf_grow() function so that GCC v12's -fanalyze doesn't
> yell at us about sb->buf[0] dereferencing NULL, this also makes this
> code easier to follow.
>
> This BUG() should be unreachable since the state of our "sb->buf" and
> "sb->alloc" goes hand-in-hand, but -fanalyzer isn't smart enough to
> know that, and adding the BUG() also makes it clearer to human readers
> that that's what happens here.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  strbuf.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/strbuf.c b/strbuf.c
> index dd9eb85527a..61c4630aeeb 100644
> --- a/strbuf.c
> +++ b/strbuf.c
> @@ -97,6 +97,8 @@ void strbuf_grow(struct strbuf *sb, size_t extra)
>  	if (new_buf)
>  		sb->buf = NULL;
>  	ALLOC_GROW(sb->buf, sb->len + extra + 1, sb->alloc);
> +	if (new_buf && !sb->buf)
> +		BUG("for a new buffer ALLOC_GROW() should always do work!");

new_buf is !sb->alloc. ALLOC_GROW sets buf if sb->len + extra + 1 is bigger than sb->alloc, which is always true because the variables in the sum are unsigned, so we end up with at least 1 > 0. That's easy enough to see; I wonder why the compiler doesn't agree. Am I missing something?

Does setting the attribute returns_nonnull for xrealloc help? Specifying it explicitly makes sense to me -- expecting the compiler to infer it automatically across compilation units is probably a bit too much to ask for, or at least needlessly expensive.

>  	if (new_buf)
>  		sb->buf[0] = '\0';
>  }