On Fri, 8 Mar 2024 at 16:06, Thomas Huth <th...@redhat.com> wrote:
>
> On 08/03/2024 09.08, Philippe Mathieu-Daudé wrote:
> > This form isn't recommended as it confuses static analyzers,
> > considering ICH9_VIRT_SMI_COUNT as part of the enum.
>
> Never heard of that before. We're using it all over the place, e.g.:
>
> typedef enum {
> THROTTLE_BPS_TOTAL,
> THROTTLE_BPS_READ,
> THROTTLE_BPS_WRITE,
> THROTTLE_OPS_TOTAL,
> THROTTLE_OPS_READ,
> THROTTLE_OPS_WRITE,
> BUCKETS_COUNT,
> } BucketType;
>
> ... and even in our generated QAPI code, e.g.:
>
> typedef enum QCryptoHashAlgorithm {
> QCRYPTO_HASH_ALG_MD5,
> QCRYPTO_HASH_ALG_SHA1,
> QCRYPTO_HASH_ALG_SHA224,
> QCRYPTO_HASH_ALG_SHA256,
> QCRYPTO_HASH_ALG_SHA384,
> QCRYPTO_HASH_ALG_SHA512,
> QCRYPTO_HASH_ALG_RIPEMD160,
> QCRYPTO_HASH_ALG__MAX,
> } QCryptoHashAlgorithm;
>
> Where did you see here a problem with static analyzers?
Coverity tends to dislike this pattern if the enum is used
as an index into an array; for example commit b12635ff08ab2
("migration: fix coverity migrate_mode finding") is
essentially a workaround for the way the QAPI generated code
puts the MAX value inside the enum. Coverity assumes that
if you have a variable foo which is a SomeEnum then it can take
any of the valid values of the enum, so if you use foo
as an index into an array that was defined as
array[SOME_ENUM_MAX] where SOME_ENUM_MAX is a value of the
enum type, and you don't explicitly check that foo
is not SOME_ENUM_MAX, then it is an overrun.
thanks
-- PMM
