Re: Covierty Integration / Improvement

From: Theodore Ts'o <tytso@mit.edu>
Date: Apr 6, 2022, 17:55 UTC
Message-ID: <Yk3UAz3sn9KhMnyf@mit.edu>
In-Reply-To: <nycvar.QRO.7.76.6.2204061620110.379@tvgsbejvaqbjf.bet>

On Wed, Apr 06, 2022 at 05:08:37PM +0200, Johannes Schindelin wrote:

Show 6 quoted lines

> 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:

#ifndef __nonstring #ifdef __has_attribute #if __has_attribute(__nonstring__) #define __nonstring __attribute__((__nonstring__)) #else #define __nonstring #endif /* __has_attribute(__nonstring__) */ #else # define __nonstring #endif /* __has_attribute */ #endif /* __nonstring */

struct ext2_super_block {
       ...
/*068*/	__u8	s_uuid[16] __nonstring;		/* 128-bit uuid for volume */
/*078*/	__u8	s_volume_name[EXT2_LABEL_LEN] __nonstring;	/* volume name */
       ...
};

(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.

Cheers,

						- Ted