Re: [Cocci] [PATCH] crypto: cavium: zip: Remove unnecessary parentheses
On Sat, 31 Mar 2018, Varsha Rao wrote:
> On Sat, Mar 31, 2018 at 11:48 AM, Julia Lawall wrote:
> > On Thu, 29 Mar 2018, Varsha Rao wrote:
> >
> >> On Wed, Mar 28, 2018 at 11:41 PM, Joe Perches wrote:
> >> >
> >> > On Wed, 2018-03-28 at 23:27, Varsha Rao wrote:
> >> > > This patch fixes the clang warning of extraneous parentheses, with the
> >> > > following coccinelle script.
> >> > >
> >> > > @@
> >> > > identifier i;
> >> > > constant c;
> >> > > @@
> >> > > (
> >> > > -((i == c))
> >> > > +i == c
> >> > > >
> >> > >
> >> > > -((i <= c))
> >> > > +i <= c
> >> >
> >> > Why just the "==" and "<=" cases?
> >> > Why not "<", ">" and ">=" too?
> >> >
> >> > Why not expression instead of constant?
> >>
> >> Initially I had the other cases too and used expression instead of
> >> constant. But the results included only "==" and "<=" cases with
> >> constant. Along with one false positive case.
> >>
> >> --- a/drivers/crypto/cavium/zip/zip_main.c
> >> +++ b/drivers/crypto/cavium/zip/zip_main.c
> >> @@ -99,7 +99,7 @@ static struct zip_device *zip_alloc_devi
> >> */
> >> struct zip_device *zip_get_device(int node)
> >> {
> >> -if ((node < MAX_ZIP_DEVICES) && (node >= 0))
> >> +if (node < MAX_ZIP_DEVICES && node >= 0)
> >
> > Why is it a false positive?
>
> The parentheses around multiple expressions in if statement is not
> considered extra, right?
< and >= should bind tighter than &&. But perhaps one could fine the
original code to be more readable.
julia
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] crypto: cavium: zip: Remove unnecessary parentheses
On Fri, 30 Mar 2018, Joe Perches wrote: > On Thu, 2018-03-29 at 21:03 +0530, Varsha Rao wrote: > > On Wed, Mar 28, 2018 at 11:41 PM, Joe Perches wrote: > > > > > > On Wed, 2018-03-28 at 23:27, Varsha Rao wrote: > > > > This patch fixes the clang warning of extraneous parentheses, with the > > > > following coccinelle script. > > > > > > > > @@ > > > > identifier i; > > > > constant c; > > > > @@ > > > > ( > > > > -((i == c)) > > > > +i == c > > > > > > > > > > > > > -((i <= c)) > > > > +i <= c > > > > > > Why just the "==" and "<=" cases? > > > Why not "<", ">" and ">=" too? > > > > > > Why not expression instead of constant? > > > > Initially I had the other cases too and used expression instead of > > constant. But the results included only "==" and "<=" cases with > > constant. Along with one false positive case. > hmm > Perhaps you should use something like this? > @@ > identifier i; > constant c; > @@ > > -( > \(i == c\|i <= c\|i < c\|i >= c\|i > c\) > -) This is not safe with respect to !. The following seems to address this problem: @@ identifier i; constant c; expression e; @@ ( !(e) | -( \(i == c\|i <= c\|i < c\|i >= c\|i > c\) -) ) julia ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] crypto: cavium: zip: Remove unnecessary parentheses
On Thu, 2018-03-29 at 21:03 +0530, Varsha Rao wrote: > On Wed, Mar 28, 2018 at 11:41 PM, Joe Perches wrote: > > > > On Wed, 2018-03-28 at 23:27, Varsha Rao wrote: > > > This patch fixes the clang warning of extraneous parentheses, with the > > > following coccinelle script. > > > > > > @@ > > > identifier i; > > > constant c; > > > @@ > > > ( > > > -((i == c)) > > > +i == c > > > > > > > > > > -((i <= c)) > > > +i <= c > > > > Why just the "==" and "<=" cases? > > Why not "<", ">" and ">=" too? > > > > Why not expression instead of constant? > > Initially I had the other cases too and used expression instead of > constant. But the results included only "==" and "<=" cases with > constant. Along with one false positive case. hmm Perhaps you should use something like this? @@ identifier i; constant c; @@ -( \(i == c\|i <= c\|i < c\|i >= c\|i > c\) -) ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] crypto: cavium: zip: Remove unnecessary parentheses
On Wed, Mar 28, 2018 at 11:41 PM, Joe Perches wrote:
>
> On Wed, 2018-03-28 at 23:27, Varsha Rao wrote:
> > This patch fixes the clang warning of extraneous parentheses, with the
> > following coccinelle script.
> >
> > @@
> > identifier i;
> > constant c;
> > @@
> > (
> > -((i == c))
> > +i == c
> > >
> >
> > -((i <= c))
> > +i <= c
>
> Why just the "==" and "<=" cases?
> Why not "<", ">" and ">=" too?
>
> Why not expression instead of constant?
Initially I had the other cases too and used expression instead of
constant. But the results included only "==" and "<=" cases with
constant. Along with one false positive case.
--- a/drivers/crypto/cavium/zip/zip_main.c
+++ b/drivers/crypto/cavium/zip/zip_main.c
@@ -99,7 +99,7 @@ static struct zip_device *zip_alloc_devi
*/
struct zip_device *zip_get_device(int node)
{
-if ((node < MAX_ZIP_DEVICES) && (node >= 0))
+if (node < MAX_ZIP_DEVICES && node >= 0)
return zip_dev[node];
zip_err("ZIP device not found for node id %d\n", node);
I checked if there was any case of extra parentheses around relational
operators left, but there were none. Hence, in the script I included
only the cases present in the result.
Thanks,
Varsha
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] crypto: cavium: zip: Remove unnecessary parentheses
On Wed, 2018-03-28 at 23:27 +0530, Varsha Rao wrote: > This patch fixes the clang warning of extraneous parentheses, with the > following coccinelle script. > > @@ > identifier i; > constant c; > @@ > ( > -((i == c)) > +i == c > > > > -((i <= c)) > +i <= c Why just the "==" and "<=" cases? Why not "<", ">" and ">=" too? Why not expression instead of constant? ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
