[ovs-dev] [PATCH] ofp-meter: Fix ds_put_format that treats enum type as short integer

Ben Pfaff blp at ovn.org
Wed Jun 27 01:16:46 UTC 2018


On Tue, Jun 26, 2018 at 10:30:30AM -0700, Yifeng Sun wrote:
> Travis job fails because of the below error:
> 
> lib/ofp-meter.c:340:48: error: format specifies type 'unsigned short'
> but the argument has underlying type 'unsigned int' [-Werror,-Wformat]
>         ds_put_format(s, "flags:0x%"PRIx16" ", flags);
> 
> This patch fixes it by treating enum type as int type.
> (6.7.2.2 Enumerationspecifiers)

Thank you for the patch.

This citation from C99 looks like a misunderstanding.  In standard C
(all editions), the members of an enumeration (the constants) have type
int, as paragraph 3 says:

    The identifiers in an enumerator list are declared as constants that
    have type int and may appear wherever such are permitted.

However, the enumeration type itself is not necessarily int.  Instead,
paragraph 4 says:

    Each enumerated type shall be compatible with char, a signed integer
    type, or an unsigned integer type. The choice of type is
    implementation-defined...

This means that, from a standards perspective, which is the correct type
here will differ from one implementation to another.  It's not even
necessarily a type that promotes to 'int' (it could be "unsigned int" or
even wider than int).  It's a weird choice but that's what the standard
says.

The particular patch here doesn't seem to match up to the description,
either.  To treat them as int, one would want to use %x, since that's
the format specifier for (unsigned) int, but this uses PRIx32, which
isn't necessarily for int (it could be for long int, for example).

>      if (flags) {
> -        ds_put_format(s, "flags:0x%"PRIx16" ", flags);
> +        ds_put_format(s, "flags:0x%"PRIx32" ", flags);
>      }

I think that a correct fix here would be to use %u plus a cast to
unsigned.

Thanks,

Ben.


More information about the dev mailing list