[Cocci] [PATCH] crypto: cavium: zip: Remove unnecessary parentheses

[Joe Perches]

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?

[Varsha Rao]

Post by Joe Perches

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

[Joe Perches]

Post by Varsha Rao

Post by Joe Perches

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\)
-)

[Julia Lawall]

Post by Joe Perches

Post by Varsha Rao

Post by Joe Perches

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

[Julia Lawall]

Post by Varsha Rao

Post by Joe Perches

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?

julia

Post by Varsha Rao
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
https://systeme.lip6.fr/mailman/listinfo/cocci

[Varsha Rao]

Post by Julia Lawall

Post by Varsha Rao

Post by Joe Perches

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?

Thanks,
Varsha

Post by Julia Lawall
julia

Post by Varsha Rao
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
https://systeme.lip6.fr/mailman/listinfo/cocci

[Julia Lawall]

Post by Varsha Rao

Post by Julia Lawall

Post by Varsha Rao

Post by Joe Perches

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