Re: What's cooking in git.git (Oct 2022, #03; Mon, 10)

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Dec 22, 2022, 08:58 UTC
Message-ID: <221222.867cyjx0d3.gmgdl@evledraar.gmail.com>
In-Reply-To: <Y6OTR2iwcORPsTxz@coredump.intra.peff.net>

On Wed, Dec 21 2022, Jeff King wrote:

Show 8 quoted lines

> I'm not sure how you saw a hundred new issues, though. My dashboard has
> 10 unresolved issues total since the beginning of September, which is
> before 2.38 was released, and I think I sent 2 fixes since then (which
> are not counted, since they're now resolved, so 2/12).
>
> I do think it would be less noisy if we could somehow convince Coverity
> that yes, strbuf really does NUL-terminate the result. But I haven't
> wanted to sink time into figuring out how to annotate it.

I don't have Coverity set up, but perhaps it's satisfied by the same thing that placeted GCC's -fanalyzers in strbuf.c:

	https://lore.kernel.org/git/RFC-patch-07.15-cf1a5f3ed0f-20220603T183608Z-avarab@gmail.com/

I run my local build with a version of that branch, I'd still like to follow-up on it (and as that RFC thread shows others had some alternate suggestions, e.g. for this strbuf case).

I don't think it's true that a strbuf "really does NUL-terminate the result" the way an analyzer like -fanalyzer sees it. I.e. if you do:

	struct strbuf sb = { .alloc = 123 };
	strbuf_addstr(&sb, "blah");

You'll segfault because the sb->buf isn't the slopbuf, nor '\0'-terminated, it's just NULL.

Now, we know we always init it with STRBUF_INIT or equivalent, but I think it's correct to flag that if you're analyzing strbuf.c in isolation, as -fanalyze (and presumably Coverity) is doing.