On Thu, Dec 22, 2022 at 09:58:01AM +0100, Ævar Arnfjörð Bjarmason wrote: > > 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@xxxxxxxxx/ > > 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 that will help. The most common strbuf problem in Coverity is "this string isn't NUL terminated". And having walked through their step-by-step analysis, I think what is going on is that it sees that: strbuf_addstr(&sb, "foo"); is doing: memcpy(sb->buf, "foo", strlen(foo)); under the hood, and it says "aha, this is an anti-pattern where you forgot to copy the NUL byte!" and creates a warning. And it ignores completely the fact that the next line is calling strbuf_setlen() and adding the NUL byte. Now there may be other false positives around strbufs (like not realizing the buffer grows), but this is the one I feel like I've seen the most. > 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. Yeah, I didn't mean to say that there can't be real problems with strbufs. I just meant that there are many false positives where the code is correct, but the tool doesn't realize it. -Peff
