[Mesa-dev] [PATCH 2/2] svga: fix coverity MIXED_ENUMS warning

Roland Scheidegger sroland at vmware.com
Tue May 31 16:14:40 UTC 2016


Am 31.05.2016 um 16:33 schrieb Erik Faye-Lund:
> On Tue, May 31, 2016 at 3:46 PM, Rob Clark <robdclark at gmail.com> wrote:
>> On Tue, May 31, 2016 at 9:29 AM, Brian Paul <brianp at vmware.com> wrote:
>>> On 05/31/2016 07:10 AM, Brian Paul wrote:
>>>>
>>>> On 05/29/2016 10:32 AM, Rob Clark wrote:
>>>>>
>>>>> From: Rob Clark <robclark at freedesktop.org>
>>>>>
>>>>> Another pipe_resource_usage vs pipe_transfer_usage mixup.
>>>>>
>>>>> CID 1362169, 1362168
>>>>>
>>>>> Signed-off-by: Rob Clark <robclark at freedesktop.org>
>>>>> ---
>>>>>   src/gallium/drivers/svga/svga_resource_buffer.c | 2 +-
>>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/src/gallium/drivers/svga/svga_resource_buffer.c
>>>>> b/src/gallium/drivers/svga/svga_resource_buffer.c
>>>>> index d91497c..77b35b3 100644
>>>>> --- a/src/gallium/drivers/svga/svga_resource_buffer.c
>>>>> +++ b/src/gallium/drivers/svga/svga_resource_buffer.c
>>>>> @@ -69,7 +69,7 @@ static void *
>>>>>   svga_buffer_transfer_map(struct pipe_context *pipe,
>>>>>                            struct pipe_resource *resource,
>>>>>                            unsigned level,
>>>>> -                         enum pipe_resource_usage usage,
>>>>> +                         enum pipe_transfer_usage usage,
>>>>>                            const struct pipe_box *box,
>>>>>                            struct pipe_transfer **ptransfer)
>>>>>   {
>>>>>
>>>>
>>>> Reviewed-by: Brian Paul <brianp at vmware.com>
>>>>
>>>
>>> Actually, maybe that should be reverted to 'unsigned'.
>>>
>>> The parameter is actually a bitmask of the PIPE_TRANSFER_x flags.  We define
>>> those flags with the pipe_transfer_usage enum type.
>>>
>>> But, IIRC, some compilers complain about using enums as bitmasks.  I think
>>> this came up in the past but I don't recall the specifics.
>>
>> hmm, there are other places were we use enum bitmasks (like nir_variable_mode)..
>>
>> (that said, pretty much all I use is gcc so don't claim to know too
>> much about msvc, etc)
> 
> I don't think I've seen MSVC complain about this in C code, but it's
> illegal in C++, AFAIK no matter what compiler.
> 
> Just tested to make sure; this works fine in VS2015 in C ode but not
> int C++ code (the or promotes to int, and won't implicitly convert
> back).
> 
> A common way to fix this, is to override the or operator. Here's an example:
> 
> ---8<---
> enum my_enum {
>    my_foo = 1,
>    my_bar = 2
> };
> 
> #ifdef __cplusplus
> inline my_enum operator|(my_enum a, my_enum b)
> {
>    return (my_enum)((int)a | (int)b);
> }
> #endif
> 
> /* just a test to show that or'ing works as expected */
> enum my_enum test(enum my_enum a, enum my_enum b)
> {
>    return a | b;
> }
> ---8<---

Isn't that a bit confusing in general using enums for bitmasks? I mean
where's the enumeration... Someone might see it's an enum and figure it
can only really be one of the enum values.

Roland




More information about the mesa-dev mailing list