staging: fsl-dpaa2: Fix multiple assignments should be avoided

PATCH In review Joshua Abraham 5 participants 7 messages Nov 8, 2017 Staging

lore

Share & tools

lore

Joshua Abraham

author

2017-11-08 00:45 UTC #0 lore raw

PATCH 1/1 staging: fsl-dpaa2: Fix multiple assignments should be avoided

This patch fixes the checkpatch.pl warning:
"CHECK: multiple assignments should be avoided"

Signed-off-by Joshua Abraham <[email protected]>

---
 drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c +2 -1

index 0d8ed002adcb..384218946108 100644
@@ -1661,7 +1661,8 @@ static void set_fq_affinity(struct dpaa2_eth_priv *priv)
 	 * This may well change at runtime, either through irqbalance or
 	 * through direct user intervention.
 	 */
-	rx_cpu = txc_cpu = cpumask_first(&priv->dpio_cpumask);
+	rx_cpu = cpumask_first(&priv->dpio_cpumask);
+	txc_cpu = rx_cpu;
 
 	for (i = 0; i < priv->num_fqs; i++) {
 		fq = &priv->fq[i];

Signature · 1 line

2.15.0

Greg KH

maintainer

2017-11-08 09:20 UTC #1 lore raw

Quoted 13 lines · Joshua Abraham

This patch fixes the checkpatch.pl warning:
"CHECK: multiple assignments should be avoided"
Signed-off-by Joshua Abraham <[email protected]>
---
 drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c +0 -0
index 0d8ed002adcb..384218946108 100644

drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c static void set_fq_affinity(struct dpaa2_eth_priv *priv) · line 1665

Quoted 6 lines

drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c +2 -1

@@ -1661,7 +1661,8 @@ static void set_fq_affinity(struct dpaa2_eth_priv *priv)
 	 * This may well change at runtime, either through irqbalance or
 	 * through direct user intervention.
 	 */
-	rx_cpu = txc_cpu = cpumask_first(&priv->dpio_cpumask);
+	rx_cpu = cpumask_first(&priv->dpio_cpumask);
+	txc_cpu = rx_cpu;

The original code here makes much more sense, doesn't it?

Sometimes checkpatch is wrong :)

thanks,

greg k-h

Dan Carpenter

2017-11-08 09:40 UTC #2 lore raw

Quoted 23 lines · Greg KH

Quoted 13 lines · Joshua Abraham
This patch fixes the checkpatch.pl warning:
"CHECK: multiple assignments should be avoided"
Signed-off-by Joshua Abraham <[email protected]>
---
 drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c +0 -0
index 0d8ed002adcb..384218946108 100644
drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c static void set_fq_affinity(struct dpaa2_eth_priv *priv) · line 1665
Quoted 6 lines
drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c +2 -1
@@ -1661,7 +1661,8 @@ static void set_fq_affinity(struct dpaa2_eth_priv *priv)
 	 * This may well change at runtime, either through irqbalance or
 	 * through direct user intervention.
 	 */
-	rx_cpu = txc_cpu = cpumask_first(&priv->dpio_cpumask);
+	rx_cpu = cpumask_first(&priv->dpio_cpumask);
+	txc_cpu = rx_cpu;
The original code here makes much more sense, doesn't it?

Quoted 1 line

Sometimes checkpatch is wrong :)

It feels like the majority of these multiple assignment warnings are wrong. I thought it would be a good idea at first but after looking at a bunch of the patches it feels like we should just remove the check.

regards,
dan carpenter

Joe Perches

Acked-by Jesper Juhl Acked-by Jan Engelhardt

2017-11-08 11:39 UTC #3 lore raw

Quoted 24 lines · Dan Carpenter

Quoted 23 lines · Greg KH
Quoted 13 lines · Joshua Abraham
This patch fixes the checkpatch.pl warning:
"CHECK: multiple assignments should be avoided"
Signed-off-by Joshua Abraham <[email protected]>
---
 drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c +0 -0
index 0d8ed002adcb..384218946108 100644
drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c static void set_fq_affinity(struct dpaa2_eth_priv *priv) · line 1665
Quoted 6 lines
drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c +2 -1
@@ -1661,7 +1661,8 @@ static void set_fq_affinity(struct dpaa2_eth_priv *priv)
 	 * This may well change at runtime, either through irqbalance or
 	 * through direct user intervention.
 	 */
-	rx_cpu = txc_cpu = cpumask_first(&priv->dpio_cpumask);
+	rx_cpu = cpumask_first(&priv->dpio_cpumask);
+	txc_cpu = rx_cpu;
The original code here makes much more sense, doesn't it?

Quoted 5 lines

Quoted 1 line
Sometimes checkpatch is wrong :)
It feels like the majority of these multiple assignment warnings are wrong. I thought it would be a good idea at first but after looking at a bunch of the patches it feels like we should just remove the check.

I don't have a particular opinion one way or another.

That bit was added to CodingStyle by Randy Dunlap back in 2006 by

commit b3fc9941fbc6efe5cb77728adb0fb12be363e73e

Author: Randy Dunlap <[email protected]>
Date:   Sun Dec 10 02:18:56 2006 -0800

[PATCH] CodingStyle updates

Add some kernel coding style comments, mostly pulled from emails by Andrew Morton, Jesper Juhl, and Randy Dunlap.

- add paragraph on switch/case indentation (with fixes)
- add paragraph on multiple-assignments
- add more on Braces
- add section on Spaces; add typeof, alignof, & __attribute__ with sizeof; add more on postfix/prefix increment/decrement operators
- add paragraph on function breaks in source files; add info on function prototype parameter names
- add paragraph on EXPORT_SYMBOL placement
- add section on /*-comment style, long-comment style, and data declarations and comments
- correct some chapter number references that were missed when chapters were renumbered

Signed-off-by Randy Dunlap <[email protected]>

Acked-by Jesper Juhl <[email protected]>

Acked-by Jan Engelhardt <[email protected]>

Signed-off-by Andrew Morton <[email protected]>

Randy Dunlap

2017-11-08 17:16 UTC #4 lore raw

Quoted 58 lines · Joe Perches

Quoted 24 lines · Dan Carpenter
Quoted 23 lines · Greg KH
Quoted 13 lines · Joshua Abraham
This patch fixes the checkpatch.pl warning:
"CHECK: multiple assignments should be avoided"
Signed-off-by Joshua Abraham <[email protected]>
---
 drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c +0 -0
index 0d8ed002adcb..384218946108 100644
drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c static void set_fq_affinity(struct dpaa2_eth_priv *priv) · line 1665
Quoted 6 lines
drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c +2 -1
@@ -1661,7 +1661,8 @@ static void set_fq_affinity(struct dpaa2_eth_priv *priv)
 	 * This may well change at runtime, either through irqbalance or
 	 * through direct user intervention.
 	 */
-	rx_cpu = txc_cpu = cpumask_first(&priv->dpio_cpumask);
+	rx_cpu = cpumask_first(&priv->dpio_cpumask);
+	txc_cpu = rx_cpu;
The original code here makes much more sense, doesn't it?
Quoted 5 lines
Quoted 1 line
Sometimes checkpatch is wrong :)
It feels like the majority of these multiple assignment warnings are wrong. I thought it would be a good idea at first but after looking at a bunch of the patches it feels like we should just remove the check.
I don't have a particular opinion one way or another.
That bit was added to CodingStyle by Randy Dunlap back in 2006 by
commit b3fc9941fbc6efe5cb77728adb0fb12be363e73e
Author: Randy Dunlap <[email protected]>
Date:   Sun Dec 10 02:18:56 2006 -0800
[PATCH] CodingStyle updates
Add some kernel coding style comments, mostly pulled from emails by Andrew Morton, Jesper Juhl, and Randy Dunlap.
- add paragraph on switch/case indentation (with fixes)
- add paragraph on multiple-assignments
- add more on Braces
- add section on Spaces; add typeof, alignof, & __attribute__ with sizeof; add more on postfix/prefix increment/decrement operators
- add paragraph on function breaks in source files; add info on function prototype parameter names
- add paragraph on EXPORT_SYMBOL placement
- add section on /*-comment style, long-comment style, and data declarations and comments
- correct some chapter number references that were missed when chapters were renumbered

Quoted 4 lines

Signed-off-by Randy Dunlap <[email protected]>
Acked-by Jesper Juhl <[email protected]>
Acked-by Jan Engelhardt <[email protected]>
Signed-off-by Andrew Morton <[email protected]>

So it was. I would certainly support removing it.

I suspect I copied that from someone else's email. My personal style would be to allow that.

Signature · 1 line

~Randy

Joe Perches

2017-11-08 17:30 UTC #5 lore raw

Quoted 6 lines · Randy Dunlap

Quoted 5 lines · Joe Perches
Quoted 4 lines · Dan Carpenter
Quoted 3 lines · Greg KH
Quoted 2 lines · Joshua Abraham
This patch fixes the checkpatch.pl warning:
"CHECK: multiple assignments should be avoided"

[]

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
Quoted 1 line
drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c +0 -0

[]

Quoted 21 lines

Quoted 21 lines
Quoted 10 lines
Quoted 10 lines
drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c static void set_fq_affinity(struct dpaa2_eth_priv *priv) · line 1665
Quoted 7 lines
drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c +2 -1
@@ -1661,7 +1661,8 @@ static void set_fq_affinity(struct dpaa2_eth_priv *priv)
 	 * This may well change at runtime, either through irqbalance or
 	 * through direct user intervention.
 	 */
-	rx_cpu = txc_cpu = cpumask_first(&priv->dpio_cpumask);
+	rx_cpu = cpumask_first(&priv->dpio_cpumask);
+	txc_cpu = rx_cpu;
The original code here makes much more sense, doesn't it?
Quoted 5 lines
Quoted 1 line
Sometimes checkpatch is wrong :)
It feels like the majority of these multiple assignment warnings are wrong. I thought it would be a good idea at first but after looking at a bunch of the patches it feels like we should just remove the check.
I don't have a particular opinion one way or another.
That bit was added to CodingStyle by Randy Dunlap back in 2006 by

Quoted 8 lines

Quoted 8 lines
commit b3fc9941fbc6efe5cb77728adb0fb12be363e73e
Author: Randy Dunlap <[email protected]>
Date:   Sun Dec 10 02:18:56 2006 -0800
[PATCH] CodingStyle updates
Add some kernel coding style comments, mostly pulled from emails by Andrew Morton, Jesper Juhl, and Randy Dunlap.

[]

Quoted 1 line

Quoted 1 line
- add paragraph on multiple-assignments

[]

Quoted 9 lines

Quoted 4 lines
Signed-off-by Randy Dunlap <[email protected]>
Acked-by Jesper Juhl <[email protected]>
Acked-by Jan Engelhardt <[email protected]>
Signed-off-by Andrew Morton <[email protected]>
So it was. I would certainly support removing it.
I suspect I copied that from someone else's email. My personal style would be to allow that.

My personal style too actually.

	foo = bar = baz();

is rather more sensible to me than

	bar = baz();
	foo = bar;

I think this style stems from the desire to be similar to single line definitions, but if the identifier lengths are short it really doesn't make the code better to read.

Though I personally do prefer

	int i;
	int *j;
over
	int i, *j;

Josh Abraham

author

2017-11-08 14:59 UTC #6 lore raw

In reply to Greg KH

Quoted 23 lines · Greg KH

Quoted 13 lines · Joshua Abraham
This patch fixes the checkpatch.pl warning:
"CHECK: multiple assignments should be avoided"
Signed-off-by Joshua Abraham <[email protected]>
---
 drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c +0 -0
index 0d8ed002adcb..384218946108 100644
drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c static void set_fq_affinity(struct dpaa2_eth_priv *priv) · line 1665
Quoted 6 lines
drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c +2 -1
@@ -1661,7 +1661,8 @@ static void set_fq_affinity(struct dpaa2_eth_priv *priv)
 	 * This may well change at runtime, either through irqbalance or
 	 * through direct user intervention.
 	 */
-	rx_cpu = txc_cpu = cpumask_first(&priv->dpio_cpumask);
+	rx_cpu = cpumask_first(&priv->dpio_cpumask);
+	txc_cpu = rx_cpu;
The original code here makes much more sense, doesn't it?

Quoted 5 lines

Sometimes checkpatch is wrong :)
thanks,
greg k-h

It does make a lot more sense.

I will trust checkpatch less, and my eyes more :)

-Josh