* [PATCH] media: atomisp: silence "dubious: !x | !y" warning @ 2021-04-17 15:36 Ashish Kalra 2021-04-17 18:56 ` Mauro Carvalho Chehab 0 siblings, 1 reply; 9+ messages in thread From: Ashish Kalra @ 2021-04-17 15:36 UTC (permalink / raw) To: Mauro Carvalho Chehab, Sakari Ailus, Greg Kroah-Hartman, Ashish Kalra, linux-media, devel, linux-kernel Cc: eashishkalra Upon running sparse, "warning: dubious: !x | !y" is brought to notice for this file. Logical and bitwise OR are basically the same in this context so it doesn't cause a runtime bug. But let's change it to logical OR to make it cleaner and silence the Sparse warning. Signed-off-by: Ashish Kalra <eashishkalra@gmail.com> --- .../media/atomisp/pci/isp/kernels/vf/vf_1.0/ia_css_vf.host.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/media/atomisp/pci/isp/kernels/vf/vf_1.0/ia_css_vf.host.c b/drivers/staging/media/atomisp/pci/isp/kernels/vf/vf_1.0/ia_css_vf.host.c index 358cb7d2cd4c..3b850bb2d39d 100644 --- a/drivers/staging/media/atomisp/pci/isp/kernels/vf/vf_1.0/ia_css_vf.host.c +++ b/drivers/staging/media/atomisp/pci/isp/kernels/vf/vf_1.0/ia_css_vf.host.c @@ -58,7 +58,7 @@ sh_css_vf_downscale_log2( unsigned int ds_log2 = 0; unsigned int out_width; - if ((!out_info) | (!vf_info)) + if ((!out_info) || (!vf_info)) return -EINVAL; out_width = out_info->res.width; -- 2.25.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] media: atomisp: silence "dubious: !x | !y" warning 2021-04-17 15:36 [PATCH] media: atomisp: silence "dubious: !x | !y" warning Ashish Kalra @ 2021-04-17 18:56 ` Mauro Carvalho Chehab 2021-04-17 21:31 ` David Laight 2021-04-18 1:26 ` Ashish Kalra 0 siblings, 2 replies; 9+ messages in thread From: Mauro Carvalho Chehab @ 2021-04-17 18:56 UTC (permalink / raw) To: Ashish Kalra Cc: Sakari Ailus, Greg Kroah-Hartman, linux-media, devel, linux-kernel Em Sat, 17 Apr 2021 21:06:27 +0530 Ashish Kalra <eashishkalra@gmail.com> escreveu: > Upon running sparse, "warning: dubious: !x | !y" is brought to notice > for this file. Logical and bitwise OR are basically the same in this > context so it doesn't cause a runtime bug. But let's change it to > logical OR to make it cleaner and silence the Sparse warning. > > Signed-off-by: Ashish Kalra <eashishkalra@gmail.com> > --- > .../media/atomisp/pci/isp/kernels/vf/vf_1.0/ia_css_vf.host.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/staging/media/atomisp/pci/isp/kernels/vf/vf_1.0/ia_css_vf.host.c b/drivers/staging/media/atomisp/pci/isp/kernels/vf/vf_1.0/ia_css_vf.host.c > index 358cb7d2cd4c..3b850bb2d39d 100644 > --- a/drivers/staging/media/atomisp/pci/isp/kernels/vf/vf_1.0/ia_css_vf.host.c > +++ b/drivers/staging/media/atomisp/pci/isp/kernels/vf/vf_1.0/ia_css_vf.host.c > @@ -58,7 +58,7 @@ sh_css_vf_downscale_log2( > unsigned int ds_log2 = 0; > unsigned int out_width; > > - if ((!out_info) | (!vf_info)) > + if ((!out_info) || (!vf_info)) While here, please get rid of the unneeded parenthesis: if (!out_info || !vf_info) > return -EINVAL; > > out_width = out_info->res.width; Thanks, Mauro ^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH] media: atomisp: silence "dubious: !x | !y" warning 2021-04-17 18:56 ` Mauro Carvalho Chehab @ 2021-04-17 21:31 ` David Laight 2021-04-18 1:15 ` Ashish Kalra ` (2 more replies) 2021-04-18 1:26 ` Ashish Kalra 1 sibling, 3 replies; 9+ messages in thread From: David Laight @ 2021-04-17 21:31 UTC (permalink / raw) To: 'Mauro Carvalho Chehab', Ashish Kalra Cc: Sakari Ailus, Greg Kroah-Hartman, linux-media, devel, linux-kernel From: Mauro Carvalho Chehab > Sent: 17 April 2021 19:56 > > Em Sat, 17 Apr 2021 21:06:27 +0530 > Ashish Kalra <eashishkalra@gmail.com> escreveu: > > > Upon running sparse, "warning: dubious: !x | !y" is brought to notice > > for this file. Logical and bitwise OR are basically the same in this > > context so it doesn't cause a runtime bug. But let's change it to > > logical OR to make it cleaner and silence the Sparse warning. The old code is very likely to by slightly more efficient. It may not matter here, but it might in a really hot path. Since !x | !y and !x || !y always have the same value why is sparse complaining at all. David > > > > Signed-off-by: Ashish Kalra <eashishkalra@gmail.com> > > --- > > .../media/atomisp/pci/isp/kernels/vf/vf_1.0/ia_css_vf.host.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/staging/media/atomisp/pci/isp/kernels/vf/vf_1.0/ia_css_vf.host.c > b/drivers/staging/media/atomisp/pci/isp/kernels/vf/vf_1.0/ia_css_vf.host.c > > index 358cb7d2cd4c..3b850bb2d39d 100644 > > --- a/drivers/staging/media/atomisp/pci/isp/kernels/vf/vf_1.0/ia_css_vf.host.c > > +++ b/drivers/staging/media/atomisp/pci/isp/kernels/vf/vf_1.0/ia_css_vf.host.c > > @@ -58,7 +58,7 @@ sh_css_vf_downscale_log2( > > unsigned int ds_log2 = 0; > > unsigned int out_width; > > > > - if ((!out_info) | (!vf_info)) > > + if ((!out_info) || (!vf_info)) > > > While here, please get rid of the unneeded parenthesis: > > if (!out_info || !vf_info) > > > > return -EINVAL; > > > > out_width = out_info->res.width; > > > > Thanks, > Mauro - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales) ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] media: atomisp: silence "dubious: !x | !y" warning 2021-04-17 21:31 ` David Laight @ 2021-04-18 1:15 ` Ashish Kalra 2021-04-18 21:59 ` Luc Van Oostenryck 2021-04-20 10:27 ` Dan Carpenter 2 siblings, 0 replies; 9+ messages in thread From: Ashish Kalra @ 2021-04-18 1:15 UTC (permalink / raw) To: David Laight Cc: 'Mauro Carvalho Chehab', Ashish Kalra, Sakari Ailus, Greg Kroah-Hartman, linux-media, devel, linux-kernel On Sat, Apr 17, 2021 at 09:31:32PM +0000, David Laight wrote: > From: Mauro Carvalho Chehab > > Sent: 17 April 2021 19:56 > > > > Em Sat, 17 Apr 2021 21:06:27 +0530 > > Ashish Kalra <eashishkalra@gmail.com> escreveu: > > > > > Upon running sparse, "warning: dubious: !x | !y" is brought to notice > > > for this file. Logical and bitwise OR are basically the same in this > > > context so it doesn't cause a runtime bug. But let's change it to > > > logical OR to make it cleaner and silence the Sparse warning. > > The old code is very likely to by slightly more efficient. > > It may not matter here, but it might in a really hot path. > > Since !x | !y and !x || !y always have the same value > why is sparse complaining at all. > > David This warning is coming from sparse as per below explanation As the name suggests, a "bitwise" expression is one that is restricted to only a certain "bitwise" operations that make sense within that class. In particular, you can't mix a "bitwise" class with a normal integer expression Warning: int __bitwise i; int __bitwise j; the two variables "i" and "j" are _not_ compatible, simply because they were declared separately, while in the case of int __bitwise i, j; they _are_ compatible. https://yarchive.net/comp/linux/sparse.html > > > > > > > Signed-off-by: Ashish Kalra <eashishkalra@gmail.com> > > > --- > > > .../media/atomisp/pci/isp/kernels/vf/vf_1.0/ia_css_vf.host.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/staging/media/atomisp/pci/isp/kernels/vf/vf_1.0/ia_css_vf.host.c > > b/drivers/staging/media/atomisp/pci/isp/kernels/vf/vf_1.0/ia_css_vf.host.c > > > index 358cb7d2cd4c..3b850bb2d39d 100644 > > > --- a/drivers/staging/media/atomisp/pci/isp/kernels/vf/vf_1.0/ia_css_vf.host.c > > > +++ b/drivers/staging/media/atomisp/pci/isp/kernels/vf/vf_1.0/ia_css_vf.host.c > > > @@ -58,7 +58,7 @@ sh_css_vf_downscale_log2( > > > unsigned int ds_log2 = 0; > > > unsigned int out_width; > > > > > > - if ((!out_info) | (!vf_info)) > > > + if ((!out_info) || (!vf_info)) > > > > > > While here, please get rid of the unneeded parenthesis: > > > > if (!out_info || !vf_info) > > > > > > > return -EINVAL; > > > > > > out_width = out_info->res.width; > > > > > > > > Thanks, > > Mauro > > - > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK > Registration No: 1397386 (Wales) > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] media: atomisp: silence "dubious: !x | !y" warning 2021-04-17 21:31 ` David Laight 2021-04-18 1:15 ` Ashish Kalra @ 2021-04-18 21:59 ` Luc Van Oostenryck 2021-04-20 10:27 ` Dan Carpenter 2 siblings, 0 replies; 9+ messages in thread From: Luc Van Oostenryck @ 2021-04-18 21:59 UTC (permalink / raw) To: David Laight Cc: 'Mauro Carvalho Chehab', Ashish Kalra, Sakari Ailus, Greg Kroah-Hartman, linux-media, devel, linux-kernel On Sat, Apr 17, 2021 at 09:31:32PM +0000, David Laight wrote: > > > Upon running sparse, "warning: dubious: !x | !y" is brought to notice > > > for this file. Logical and bitwise OR are basically the same in this > > > context so it doesn't cause a runtime bug. But let's change it to > > > logical OR to make it cleaner and silence the Sparse warning. > > The old code is very likely to by slightly more efficient. > > It may not matter here, but it might in a really hot path. > > Since !x | !y and !x || !y always have the same value > why is sparse complaining at all. They both will have the same value here and any half-decent compiler know that and thus generate the same code, so no worries about efficiency. Sparse complains because the programmer's intention is not clear. Was a boolean context or a bitwise context that was meant? Maybe '||' was meant and the RHS had to be short cut? Maybe what was meant was '~x | ~y'? -- Luc ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] media: atomisp: silence "dubious: !x | !y" warning 2021-04-17 21:31 ` David Laight 2021-04-18 1:15 ` Ashish Kalra 2021-04-18 21:59 ` Luc Van Oostenryck @ 2021-04-20 10:27 ` Dan Carpenter 2021-04-20 10:36 ` David Laight 2 siblings, 1 reply; 9+ messages in thread From: Dan Carpenter @ 2021-04-20 10:27 UTC (permalink / raw) To: David Laight Cc: 'Mauro Carvalho Chehab', Ashish Kalra, devel, Greg Kroah-Hartman, linux-kernel, Sakari Ailus, linux-media On Sat, Apr 17, 2021 at 09:31:32PM +0000, David Laight wrote: > From: Mauro Carvalho Chehab > > Sent: 17 April 2021 19:56 > > > > Em Sat, 17 Apr 2021 21:06:27 +0530 > > Ashish Kalra <eashishkalra@gmail.com> escreveu: > > > > > Upon running sparse, "warning: dubious: !x | !y" is brought to notice > > > for this file. Logical and bitwise OR are basically the same in this > > > context so it doesn't cause a runtime bug. But let's change it to > > > logical OR to make it cleaner and silence the Sparse warning. > > The old code is very likely to by slightly more efficient. > > It may not matter here, but it might in a really hot path. > > Since !x | !y and !x || !y always have the same value > why is sparse complaining at all. > Smatch doesn't warn about | vs || if both sides are true/false. But I've occasionally asked people if they were trying to do a fast path optimization but it's always just a typo. regards, dan carpenter ^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH] media: atomisp: silence "dubious: !x | !y" warning 2021-04-20 10:27 ` Dan Carpenter @ 2021-04-20 10:36 ` David Laight 0 siblings, 0 replies; 9+ messages in thread From: David Laight @ 2021-04-20 10:36 UTC (permalink / raw) To: 'Dan Carpenter' Cc: 'Mauro Carvalho Chehab', Ashish Kalra, devel, Greg Kroah-Hartman, linux-kernel, Sakari Ailus, linux-media From: Dan Carpenter > Sent: 20 April 2021 11:28 > > On Sat, Apr 17, 2021 at 09:31:32PM +0000, David Laight wrote: > > From: Mauro Carvalho Chehab > > > Sent: 17 April 2021 19:56 > > > > > > Em Sat, 17 Apr 2021 21:06:27 +0530 > > > Ashish Kalra <eashishkalra@gmail.com> escreveu: > > > > > > > Upon running sparse, "warning: dubious: !x | !y" is brought to notice > > > > for this file. Logical and bitwise OR are basically the same in this > > > > context so it doesn't cause a runtime bug. But let's change it to > > > > logical OR to make it cleaner and silence the Sparse warning. > > > > The old code is very likely to by slightly more efficient. > > > > It may not matter here, but it might in a really hot path. > > > > Since !x | !y and !x || !y always have the same value > > why is sparse complaining at all. > > > > Smatch doesn't warn about | vs || if both sides are true/false. But > I've occasionally asked people if they were trying to do a fast path > optimization but it's always just a typo. The problem is with people blindly patching code to 'fix' these warnings. It might just be a fast path optimisation - which they break. Trying to beat the compiler into submission can be hard though. Getting it to 'or' together the outputs from a series of x86 'setne' instructions isn't for the faint hearted. Not helped by the instruction only setting %al. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales) ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] media: atomisp: silence "dubious: !x | !y" warning 2021-04-17 18:56 ` Mauro Carvalho Chehab 2021-04-17 21:31 ` David Laight @ 2021-04-18 1:26 ` Ashish Kalra 2021-04-20 12:04 ` Hans Verkuil 1 sibling, 1 reply; 9+ messages in thread From: Ashish Kalra @ 2021-04-18 1:26 UTC (permalink / raw) To: Mauro Carvalho Chehab Cc: Ashish Kalra, Sakari Ailus, Greg Kroah-Hartman, linux-media, devel, linux-kernel [-- Attachment #1: Type: text/plain, Size: 1460 bytes --] On Sat, Apr 17, 2021 at 08:56:13PM +0200, Mauro Carvalho Chehab wrote: > Em Sat, 17 Apr 2021 21:06:27 +0530 > Ashish Kalra <eashishkalra@gmail.com> escreveu: > > > Upon running sparse, "warning: dubious: !x | !y" is brought to notice > > for this file. Logical and bitwise OR are basically the same in this > > context so it doesn't cause a runtime bug. But let's change it to > > logical OR to make it cleaner and silence the Sparse warning. > > > > Signed-off-by: Ashish Kalra <eashishkalra@gmail.com> > > --- > > .../media/atomisp/pci/isp/kernels/vf/vf_1.0/ia_css_vf.host.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/staging/media/atomisp/pci/isp/kernels/vf/vf_1.0/ia_css_vf.host.c b/drivers/staging/media/atomisp/pci/isp/kernels/vf/vf_1.0/ia_css_vf.host.c > > index 358cb7d2cd4c..3b850bb2d39d 100644 > > --- a/drivers/staging/media/atomisp/pci/isp/kernels/vf/vf_1.0/ia_css_vf.host.c > > +++ b/drivers/staging/media/atomisp/pci/isp/kernels/vf/vf_1.0/ia_css_vf.host.c > > @@ -58,7 +58,7 @@ sh_css_vf_downscale_log2( > > unsigned int ds_log2 = 0; > > unsigned int out_width; > > > > - if ((!out_info) | (!vf_info)) > > + if ((!out_info) || (!vf_info)) > > > While here, please get rid of the unneeded parenthesis: > > if (!out_info || !vf_info) > > > > return -EINVAL; > > > > out_width = out_info->res.width; > > > > Thanks, > Mauro Updated Patch as per your feedback Thanks Ashish [-- Attachment #2: 0001-media-atomisp-silence-dubious-x-y-warning.patch --] [-- Type: text/x-diff, Size: 1257 bytes --] From 770317157c3a7abf2fda1d71b0bd651c33bf0bfa Mon Sep 17 00:00:00 2001 From: Ashish Kalra <eashishkalra@gmail.com> Date: Sun, 18 Apr 2021 06:52:03 +0530 Subject: [PATCH] media: atomisp: silence "dubious: !x | !y" warning Upon running sparse, "warning: dubious: !x | !y" is brought to notice for this file. Logical and bitwise OR are basically the same in this context so it doesn't cause a runtime bug. But let's change it to logical OR to make it cleaner and silence the Sparse warning. Signed-off-by: Ashish Kalra <eashishkalra@gmail.com> --- .../media/atomisp/pci/isp/kernels/vf/vf_1.0/ia_css_vf.host.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/media/atomisp/pci/isp/kernels/vf/vf_1.0/ia_css_vf.host.c b/drivers/staging/media/atomisp/pci/isp/kernels/vf/vf_1.0/ia_css_vf.host.c index 358cb7d2cd4c..71c3e7dac052 100644 --- a/drivers/staging/media/atomisp/pci/isp/kernels/vf/vf_1.0/ia_css_vf.host.c +++ b/drivers/staging/media/atomisp/pci/isp/kernels/vf/vf_1.0/ia_css_vf.host.c @@ -58,7 +58,7 @@ sh_css_vf_downscale_log2( unsigned int ds_log2 = 0; unsigned int out_width; - if ((!out_info) | (!vf_info)) + if (!out_info || !vf_info) return -EINVAL; out_width = out_info->res.width; -- 2.25.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] media: atomisp: silence "dubious: !x | !y" warning 2021-04-18 1:26 ` Ashish Kalra @ 2021-04-20 12:04 ` Hans Verkuil 0 siblings, 0 replies; 9+ messages in thread From: Hans Verkuil @ 2021-04-20 12:04 UTC (permalink / raw) To: Ashish Kalra, Mauro Carvalho Chehab Cc: Sakari Ailus, Greg Kroah-Hartman, linux-media, devel, linux-kernel Hi Ashish, On 18/04/2021 03:26, Ashish Kalra wrote: > On Sat, Apr 17, 2021 at 08:56:13PM +0200, Mauro Carvalho Chehab wrote: >> Em Sat, 17 Apr 2021 21:06:27 +0530 >> Ashish Kalra <eashishkalra@gmail.com> escreveu: >> >>> Upon running sparse, "warning: dubious: !x | !y" is brought to notice >>> for this file. Logical and bitwise OR are basically the same in this >>> context so it doesn't cause a runtime bug. But let's change it to >>> logical OR to make it cleaner and silence the Sparse warning. >>> >>> Signed-off-by: Ashish Kalra <eashishkalra@gmail.com> >>> --- >>> .../media/atomisp/pci/isp/kernels/vf/vf_1.0/ia_css_vf.host.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/staging/media/atomisp/pci/isp/kernels/vf/vf_1.0/ia_css_vf.host.c b/drivers/staging/media/atomisp/pci/isp/kernels/vf/vf_1.0/ia_css_vf.host.c >>> index 358cb7d2cd4c..3b850bb2d39d 100644 >>> --- a/drivers/staging/media/atomisp/pci/isp/kernels/vf/vf_1.0/ia_css_vf.host.c >>> +++ b/drivers/staging/media/atomisp/pci/isp/kernels/vf/vf_1.0/ia_css_vf.host.c >>> @@ -58,7 +58,7 @@ sh_css_vf_downscale_log2( >>> unsigned int ds_log2 = 0; >>> unsigned int out_width; >>> >>> - if ((!out_info) | (!vf_info)) >>> + if ((!out_info) || (!vf_info)) >> >> >> While here, please get rid of the unneeded parenthesis: >> >> if (!out_info || !vf_info) >> >> >>> return -EINVAL; >>> >>> out_width = out_info->res.width; >> >> >> >> Thanks, >> Mauro > Updated Patch as per your feedback Please don't post patches as an attachment. Just post it inline as you did the first time, but with Subject prefix [PATCHv2]. Thanks! Hans ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-04-20 12:04 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-04-17 15:36 [PATCH] media: atomisp: silence "dubious: !x | !y" warning Ashish Kalra 2021-04-17 18:56 ` Mauro Carvalho Chehab 2021-04-17 21:31 ` David Laight 2021-04-18 1:15 ` Ashish Kalra 2021-04-18 21:59 ` Luc Van Oostenryck 2021-04-20 10:27 ` Dan Carpenter 2021-04-20 10:36 ` David Laight 2021-04-18 1:26 ` Ashish Kalra 2021-04-20 12:04 ` Hans Verkuil
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).