Re: [RFC PATCH 4/5] hw/i386/q35: Wire virtual SMI# lines to ICH9 chipset

[Peter Maydell]

On Fri, 8 Mar 2024 at 16:06, Thomas Huth <thuth@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