Re: [PATCH] vfio/cdx: Add parentheses between bitwise AND expression and logical NOT

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 3/10/23 19:20, Alex Williamson wrote:
> On Tue, 3 Oct 2023 08:27:39 -0700
> Nathan Chancellor <nathan@kernel.org> wrote:
> 
>> On Tue, Oct 03, 2023 at 09:40:02AM +0200, Philippe Mathieu-Daudé wrote:
>>> Hi Nathan,
>>>
>>> On 2/10/23 19:53, Nathan Chancellor wrote:
>>>> When building with clang, there is a warning (or error with
>>>> CONFIG_WERROR=y) due to a bitwise AND and logical NOT in
>>>> vfio_cdx_bm_ctrl():
>>>>
>>>>     drivers/vfio/cdx/main.c:77:6: error: logical not is only applied to the left hand side of this bitwise operator [-Werror,-Wlogical-not-parentheses]
>>>>        77 |         if (!vdev->flags & BME_SUPPORT)
>>>>           |             ^            ~
>>>>     drivers/vfio/cdx/main.c:77:6: note: add parentheses after the '!' to evaluate the bitwise operator first
>>>>        77 |         if (!vdev->flags & BME_SUPPORT)
>>>>           |             ^
>>>>           |              (                        )
>>>>     drivers/vfio/cdx/main.c:77:6: note: add parentheses around left hand side expression to silence this warning
>>>>        77 |         if (!vdev->flags & BME_SUPPORT)
>>>>           |             ^
>>>>           |             (           )
>>>>     1 error generated.
>>>>
>>>> Add the parentheses as suggested in the first note, which is clearly
>>>> what was intended here.
>>>>
>>>> Closes: https://github.com/ClangBuiltLinux/linux/issues/1939
>>>> Fixes: 8a97ab9b8b31 ("vfio-cdx: add bus mastering device feature support")
>>>
>>> My current /master points to commit ce36c8b14987 which doesn't include
>>> 8a97ab9b8b31, so maybe this can be squashed / reordered in the VFIO tree
>>> (where I assume this commit is). That said, the fix is correct, so:
>>
>> Yes, this is a -next only issue at the moment and I don't mind this
>> change being squashed into the original if Alex rebases his tree (some
>> maintainers don't).
> 
> Right, where practical we try not to change commit hashes once
> something has been included into linux-next, preferring to layer fixes
> or even reverts, but occasionally something will come up where it makes
> sense to rebase.  This is not such a case :)  Thanks,

Got it, thanks!

> 
> Alex
> 
>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>
>> Thanks a lot for taking a look!
>>
>> Cheers,
>> Nathan