diff mbox series

vfio/cdx: Add parentheses between bitwise AND expression and logical NOT

Message ID 20231002-vfio-cdx-logical-not-parentheses-v1-1-a8846c7adfb6@kernel.org (mailing list archive)
State New, archived
Headers show
Series vfio/cdx: Add parentheses between bitwise AND expression and logical NOT | expand

Commit Message

Nathan Chancellor Oct. 2, 2023, 5:53 p.m. UTC 
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")
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
 drivers/vfio/cdx/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


---
base-commit: fcb2f2ed4a80cfe383d87da75caba958516507e9
change-id: 20231002-vfio-cdx-logical-not-parentheses-aca8fbd6b278

Best regards,

Comments

Nick Desaulniers Oct. 2, 2023, 5:57 p.m. UTC | #1 
On Mon, Oct 2, 2023 at 10:53 AM Nathan Chancellor <nathan@kernel.org> 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")
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>

Thanks for the patch!
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

> ---
>  drivers/vfio/cdx/main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/vfio/cdx/main.c b/drivers/vfio/cdx/main.c
> index a437630be354..a63744302b5e 100644
> --- a/drivers/vfio/cdx/main.c
> +++ b/drivers/vfio/cdx/main.c
> @@ -74,7 +74,7 @@ static int vfio_cdx_bm_ctrl(struct vfio_device *core_vdev, u32 flags,
>         struct vfio_device_feature_bus_master ops;
>         int ret;
>
> -       if (!vdev->flags & BME_SUPPORT)
> +       if (!(vdev->flags & BME_SUPPORT))
>                 return -ENOTTY;
>
>         ret = vfio_check_feature(flags, argsz, VFIO_DEVICE_FEATURE_SET,
>
> ---
> base-commit: fcb2f2ed4a80cfe383d87da75caba958516507e9
> change-id: 20231002-vfio-cdx-logical-not-parentheses-aca8fbd6b278
>
> Best regards,
> --
> Nathan Chancellor <nathan@kernel.org>
>
>
Agarwal, Nikhil Oct. 3, 2023, 4:17 a.m. UTC | #2 
[AMD Official Use Only - General]

> -----Original Message-----
> From: Nathan Chancellor <nathan@kernel.org>
> Sent: Monday, October 2, 2023 11:23 PM
> To: Gupta, Nipun <Nipun.Gupta@amd.com>; Agarwal, Nikhil
> <nikhil.agarwal@amd.com>; alex.williamson@redhat.com
> Cc: ndesaulniers@google.com; trix@redhat.com; Rohila, Shubham
> <shubham.rohila@amd.com>; kvm@vger.kernel.org; llvm@lists.linux.dev;
> patches@lists.linux.dev; Nathan Chancellor <nathan@kernel.org>
> Subject: [PATCH] vfio/cdx: Add parentheses between bitwise AND expression
> and logical NOT
>
> 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")
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> ---
Acked-by: Nikhil Agarwal <nikhil.agarwal@amd.com>
Philippe Mathieu-Daudé Oct. 3, 2023, 7:40 a.m. UTC | #3 
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:

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Regards,

Phil.

> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> ---
>   drivers/vfio/cdx/main.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/vfio/cdx/main.c b/drivers/vfio/cdx/main.c
> index a437630be354..a63744302b5e 100644
> --- a/drivers/vfio/cdx/main.c
> +++ b/drivers/vfio/cdx/main.c
> @@ -74,7 +74,7 @@ static int vfio_cdx_bm_ctrl(struct vfio_device *core_vdev, u32 flags,
>   	struct vfio_device_feature_bus_master ops;
>   	int ret;
>   
> -	if (!vdev->flags & BME_SUPPORT)
> +	if (!(vdev->flags & BME_SUPPORT))
>   		return -ENOTTY;
>   
>   	ret = vfio_check_feature(flags, argsz, VFIO_DEVICE_FEATURE_SET,
> 
> ---
> base-commit: fcb2f2ed4a80cfe383d87da75caba958516507e9
> change-id: 20231002-vfio-cdx-logical-not-parentheses-aca8fbd6b278
> 
> Best regards,
Nathan Chancellor Oct. 3, 2023, 3:27 p.m. UTC | #4 
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).

> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Thanks a lot for taking a look!

Cheers,
Nathan

> > Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> > ---
> >   drivers/vfio/cdx/main.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/vfio/cdx/main.c b/drivers/vfio/cdx/main.c
> > index a437630be354..a63744302b5e 100644
> > --- a/drivers/vfio/cdx/main.c
> > +++ b/drivers/vfio/cdx/main.c
> > @@ -74,7 +74,7 @@ static int vfio_cdx_bm_ctrl(struct vfio_device *core_vdev, u32 flags,
> >   	struct vfio_device_feature_bus_master ops;
> >   	int ret;
> > -	if (!vdev->flags & BME_SUPPORT)
> > +	if (!(vdev->flags & BME_SUPPORT))
> >   		return -ENOTTY;
> >   	ret = vfio_check_feature(flags, argsz, VFIO_DEVICE_FEATURE_SET,
> > 
> > ---
> > base-commit: fcb2f2ed4a80cfe383d87da75caba958516507e9
> > change-id: 20231002-vfio-cdx-logical-not-parentheses-aca8fbd6b278
> > 
> > Best regards,
>
Alex Williamson Oct. 3, 2023, 5:20 p.m. UTC | #5 
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,

Alex

> > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>  
> 
> Thanks a lot for taking a look!
> 
> Cheers,
> Nathan
> 
> > > Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> > > ---
> > >   drivers/vfio/cdx/main.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/vfio/cdx/main.c b/drivers/vfio/cdx/main.c
> > > index a437630be354..a63744302b5e 100644
> > > --- a/drivers/vfio/cdx/main.c
> > > +++ b/drivers/vfio/cdx/main.c
> > > @@ -74,7 +74,7 @@ static int vfio_cdx_bm_ctrl(struct vfio_device *core_vdev, u32 flags,
> > >   	struct vfio_device_feature_bus_master ops;
> > >   	int ret;
> > > -	if (!vdev->flags & BME_SUPPORT)
> > > +	if (!(vdev->flags & BME_SUPPORT))
> > >   		return -ENOTTY;
> > >   	ret = vfio_check_feature(flags, argsz, VFIO_DEVICE_FEATURE_SET,
> > > 
> > > ---
> > > base-commit: fcb2f2ed4a80cfe383d87da75caba958516507e9
> > > change-id: 20231002-vfio-cdx-logical-not-parentheses-aca8fbd6b278
> > > 
> > > Best regards,  
> >   
>
Alex Williamson Oct. 3, 2023, 6:01 p.m. UTC | #6 
On Mon, 02 Oct 2023 10:53:13 -0700
Nathan Chancellor <nathan@kernel.org> 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")
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> ---
>  drivers/vfio/cdx/main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/vfio/cdx/main.c b/drivers/vfio/cdx/main.c
> index a437630be354..a63744302b5e 100644
> --- a/drivers/vfio/cdx/main.c
> +++ b/drivers/vfio/cdx/main.c
> @@ -74,7 +74,7 @@ static int vfio_cdx_bm_ctrl(struct vfio_device *core_vdev, u32 flags,
>  	struct vfio_device_feature_bus_master ops;
>  	int ret;
>  
> -	if (!vdev->flags & BME_SUPPORT)
> +	if (!(vdev->flags & BME_SUPPORT))
>  		return -ENOTTY;
>  
>  	ret = vfio_check_feature(flags, argsz, VFIO_DEVICE_FEATURE_SET,
> 
> ---
> base-commit: fcb2f2ed4a80cfe383d87da75caba958516507e9
> change-id: 20231002-vfio-cdx-logical-not-parentheses-aca8fbd6b278
> 
> Best regards,

Applied to vfio next branch for v6.7.  Thanks!

Alex
Philippe Mathieu-Daudé Oct. 4, 2023, 6:48 a.m. UTC | #7 
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
diff mbox series

Patch

diff --git a/drivers/vfio/cdx/main.c b/drivers/vfio/cdx/main.c
index a437630be354..a63744302b5e 100644
--- a/drivers/vfio/cdx/main.c
+++ b/drivers/vfio/cdx/main.c
@@ -74,7 +74,7 @@  static int vfio_cdx_bm_ctrl(struct vfio_device *core_vdev, u32 flags,
 	struct vfio_device_feature_bus_master ops;
 	int ret;
 
-	if (!vdev->flags & BME_SUPPORT)
+	if (!(vdev->flags & BME_SUPPORT))
 		return -ENOTTY;
 
 	ret = vfio_check_feature(flags, argsz, VFIO_DEVICE_FEATURE_SET,