Re: Covierty Integration / Improvement

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 6, 2022, 20:20 UTC
Message-ID: <xmqqfsmqx8ud.fsf@gitster.g>
In-Reply-To: <Yk3UAz3sn9KhMnyf@mit.edu>

"Theodore Ts'o" <tytso@mit.edu> writes:

Show 18 quoted lines

> On Wed, Apr 06, 2022 at 05:08:37PM +0200, Johannes Schindelin wrote:
>> I have fixed Git for Windows' Coverity build and started to sift through
>> the 154 new defects reported as of v2.36.0-rc0.
>> 
>> Sadly, there is now a new class of overwhelming false positives: Coverity
>> claims that "strbuf_addstr does not [NUL-]terminate", which is of course
>> false.
>
> It should be possible to suppress this by uploading a Coverity model
> file.  See[1] for more details:
>
> [1] https://community.synopsys.com/s/article/practical-example-of-coverity-function-model
>
> I've suppressed a similar issue by using the attribute __nonstring,
> but I don't think that will work for git, because strbuf->buf really
> *is* a NUL-terminated string, where as in ext4 we have some fields
> which are designed to be NUL padded, but it is *not* guaranteed to be
> NUL-terminated:

That is very much expected from filesystem code, for which strncmp() and friends were invented for ;-)

Show 5 quoted lines

> (This is needed to suppress warnings by Clang as well.)
>
> Using __nonstring will result in attempts to use s_volume_name in "C"
> string context to give a warning, which is why this isn't right for
> strbuf->buf.

Indeed. We do aim to make reading .buf member as NUL-terminated string safe, so it would make it very inconvenient to warn against such uses.

Thanks.