Subject: Re: [PATCH] strbuf: stop out-of-boundary warnings from Coverity

From: Junio C Hamano <[email-masked]>
Date: Thu, 18 Jun 2015 09:46:09 -0700

Duy Nguyen <[email-masked]> writes:

> The last resort is simply filter out a whole class of warnings.
> Probably good enough if both patches look equally ugly.
>
> -- 8< --
> Subject: [PATCH] strbuf: kill strbuf_slopbuf, in favor of ""
>
> A lot of "out-of-bound access" warnings on scan.coverity.com is because
> it does not realize this strbuf_slopbuf[] is in fact initialized with a
> single and promised to never change. But that promise could be broken if
> some caller attempts to write to strbuf->buf[0] write after STRBUF_INIT.
>
> We really can't do much about it. But we can try to put strbuf_slopbuf
> in .rodata section, where writes will be caught by the OS with memory
> protection support. The only drawback is people can't do
> "buf->buf == strbuf_slopbuf" any more. Luckily nobody does that in the
> current code base.
> ---

Hmph, would declaring slopbuf as "const char [1]" (and sprinkling
the "(char *)" cast) have the same effect, I wonder?

> +static inline void strbuf_terminate(struct strbuf *sb)
> +{
> +	if (sb->buf[sb->len])
> +		sb->buf[sb->len] = '\0';
> +}

This is so that you can call things like strbuf_rtrim() immediately
after running strbuf_init() safely, but I think it needs a comment
to save people from wondering what is going on, e.g. "this is not an
optimization to avoid assigning NUL to a place that is already NUL;
a freshly initialized strbuf points at an unwritable piece of NUL
and we do not want to cause a SEGV".