Skip to contentSkip to conversation

staging: Fix spacing between function name and parentheses

PATCH New Nicholas Krause 14 participants 26 messages Oct 11, 2014 – Nov 4, 2014 Staging

lore
Share & tools
lore

Nicholas Krause

author
#0 lore raw

PATCH 1/1 staging: Fix spacing between function name and parentheses

Fixes checkpatch coding style warning about unneeded space between function name an parentheses.

Signed-off-by Nicholas Krause <[email protected]>

---
Untested

 drivers/staging/octeon-usb/octeon-hcd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/staging/octeon-usb/octeon-hcd.c +1 -1
index 5f9db4c..bbeb0cc 100644
@@ -412,7 +412,7 @@ struct octeon_hcd {
 		type c;							    \
 		while (1) {						    \
 			c.u32 = __cvmx_usb_read_csr32(usb, address);	    \
-			if (c.s.field op (value)) {			    \
+			if (c.s.field op(value)) {			    \
 				result = 0;				    \
 				break;					    \
 			} else if (cvmx_get_cycle() > done) {		    \
Signature · 1 line
1.9.1
#1 lore raw

PATCH 1/1 staging: Fix spacing between function name and parentheses

Quoted 14 lines · Nicholas Krause

Fixes checkpatch coding style warning about unneeded space between function name an parentheses.

Signed-off-by Nicholas Krause <[email protected]>

---
Untested

 drivers/staging/octeon-usb/octeon-hcd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/staging/octeon-usb/octeon-hcd.c +0 -0
index 5f9db4c..bbeb0cc 100644

drivers/staging/octeon-usb/octeon-hcd.c struct octeon_hcd { · line 415

Quoted 5 lines
drivers/staging/octeon-usb/octeon-hcd.c +1 -1
@@ -412,7 +412,7 @@ struct octeon_hcd {
 		type c;							    \
 		while (1) {						    \
 			c.u32 = __cvmx_usb_read_csr32(usb, address);	    \
-			if (c.s.field op (value)) {			    \
+			if (c.s.field op(value)) {			    \

have you read the code before modifying it?
this is not a function.
have you seen how CVMX_WAIT_FOR_FIELD32 is being called?
on every call of CVMX_WAIT_FOR_FIELD32 op is the operator "=="
so when called the macro will be c.s.field == (value).
if your patch is applied then it will become c.s.field ==(value) .. will that be correct ?

thanks
sudip

Quoted 11 lines
drivers/staging/octeon-usb/octeon-hcd.c +0 -0
 				result = 0;				    \
 				break;					    \
 			} else if (cvmx_get_cycle() > done) {		    \
Signature · 7 lines
1.9.1


_______________________________________________
Kernelnewbies mailing list
Kernelnewbies at kernelnewbies.org
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
#2 lore raw

PATCH 1/1 staging: Fix spacing between function name and parentheses

It also works as value is surrounded by (), though I do not think the patch itself is right or useful.

Dave Tian
dave.jing.tian at gmail.com

Quoted 47 lines · Sudip Mukherjee
Quoted 14 lines · Nicholas Krause

Fixes checkpatch coding style warning about unneeded space between function name an parentheses.

Signed-off-by Nicholas Krause <[email protected]>

---
Untested
drivers/staging/octeon-usb/octeon-hcd.c | 2 +-

1 file changed, 1 insertion(+), 1 deletion(-)
drivers/staging/octeon-usb/octeon-hcd.c +0 -0
index 5f9db4c..bbeb0cc 100644

drivers/staging/octeon-usb/octeon-hcd.c struct octeon_hcd { · line 415

Quoted 5 lines
drivers/staging/octeon-usb/octeon-hcd.c +1 -1
@@ -412,7 +412,7 @@ struct octeon_hcd {
 		type c;							    \
 		while (1) {						    \
 			c.u32 = __cvmx_usb_read_csr32(usb, address);	    \
-			if (c.s.field op (value)) {			    \
+			if (c.s.field op(value)) {			    \

have you read the code before modifying it?
this is not a function.
have you seen how CVMX_WAIT_FOR_FIELD32 is being called?
on every call of CVMX_WAIT_FOR_FIELD32 op is the operator "=="
so when called the macro will be c.s.field == (value).
if your patch is applied then it will become c.s.field ==(value) .. will that be correct ?

thanks
sudip

Quoted 11 lines
drivers/staging/octeon-usb/octeon-hcd.c +0 -0
 				result = 0;				    \
 				break;					    \
 			} else if (cvmx_get_cycle() > done) {		    \
Signature · 7 lines
1.9.1


_______________________________________________
Kernelnewbies mailing list
Kernelnewbies at kernelnewbies.org
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies at kernelnewbies.org
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

#3 lore raw

PATCH 1/1 staging: Fix spacing between function name and parentheses

Hi Dave,
It will work. But my point of saying that was c.s.field ==(value) is again not according to the style.

thanks
sudip

Quoted 57 lines · Dave Tian

It also works as value is surrounded by (), though I do not think the patch itself is right or useful.

Dave Tian
dave.jing.tian at gmail.com

Quoted 47 lines · Sudip Mukherjee
Quoted 14 lines · Nicholas Krause

Fixes checkpatch coding style warning about unneeded space between function name an parentheses.

Signed-off-by Nicholas Krause <[email protected]>

---
Untested
drivers/staging/octeon-usb/octeon-hcd.c | 2 +-

1 file changed, 1 insertion(+), 1 deletion(-)
drivers/staging/octeon-usb/octeon-hcd.c +0 -0
index 5f9db4c..bbeb0cc 100644

drivers/staging/octeon-usb/octeon-hcd.c struct octeon_hcd { · line 415

Quoted 5 lines
drivers/staging/octeon-usb/octeon-hcd.c +1 -1
@@ -412,7 +412,7 @@ struct octeon_hcd {
             type c;                                                     \
             while (1) {                                                 \
                     c.u32 = __cvmx_usb_read_csr32(usb, address);        \
-                    if (c.s.field op (value)) {                         \
+                    if (c.s.field op(value)) {                          \

have you read the code before modifying it?
this is not a function.
have you seen how CVMX_WAIT_FOR_FIELD32 is being called?
on every call of CVMX_WAIT_FOR_FIELD32 op is the operator "=="
so when called the macro will be c.s.field == (value).
if your patch is applied then it will become c.s.field ==(value) .. will that be correct ?

thanks
sudip

Quoted 11 lines
drivers/staging/octeon-usb/octeon-hcd.c +0 -1
                             result = 0;                                 \
                             break;                                      \
                     } else if (cvmx_get_cycle() > done) {               \
--

1.9.1

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies at kernelnewbies.org
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies at kernelnewbies.org
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

#4 lore raw

PATCH 1/1 staging: Fix spacing between function name and parentheses

Agreed - that is why I mentioned the patch is neither right nor useful:)

-daveti

Quoted 65 lines · Sudip Mukherjee

Hi Dave,
It will work. But my point of saying that was c.s.field ==(value) is again not according to the style.

thanks
sudip

Quoted 57 lines · Dave Tian

It also works as value is surrounded by (), though I do not think the patch itself is right or useful.

Dave Tian
dave.jing.tian at gmail.com

Quoted 47 lines · Sudip Mukherjee
Quoted 14 lines · Nicholas Krause

Fixes checkpatch coding style warning about unneeded space between function name an parentheses.

Signed-off-by Nicholas Krause <[email protected]>

---
Untested
drivers/staging/octeon-usb/octeon-hcd.c | 2 +-

1 file changed, 1 insertion(+), 1 deletion(-)
drivers/staging/octeon-usb/octeon-hcd.c +0 -0
index 5f9db4c..bbeb0cc 100644

drivers/staging/octeon-usb/octeon-hcd.c struct octeon_hcd { · line 415

Quoted 5 lines
drivers/staging/octeon-usb/octeon-hcd.c +1 -1
@@ -412,7 +412,7 @@ struct octeon_hcd {
            type c;                                                     \
            while (1) {                                                 \
                    c.u32 = __cvmx_usb_read_csr32(usb, address);        \
-                    if (c.s.field op (value)) {                         \
+                    if (c.s.field op(value)) {                          \

have you read the code before modifying it?
this is not a function.
have you seen how CVMX_WAIT_FOR_FIELD32 is being called?
on every call of CVMX_WAIT_FOR_FIELD32 op is the operator "=="
so when called the macro will be c.s.field == (value).
if your patch is applied then it will become c.s.field ==(value) .. will that be correct ?

thanks
sudip

Quoted 11 lines
drivers/staging/octeon-usb/octeon-hcd.c +0 -1
                            result = 0;                                 \
                            break;                                      \
                    } else if (cvmx_get_cycle() > done) {               \
--

1.9.1

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies at kernelnewbies.org
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies at kernelnewbies.org
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

#5 lore raw

PATCH 1/1 staging: Fix spacing between function name and parentheses

I think that, in this case, checkpatch.pl contributed:

$ ./scripts/checkpatch.pl -f drivers/staging/octeon-usb/octeon-hcd.c
WARNING: space prohibited between function name and open parenthesis '('
#415: FILE: drivers/staging/octeon-usb/octeon-hcd.c:415:
+                       if (c.s.field op (value)) {                         \
Quoted 78 lines · Dave Tian

Agreed - that is why I mentioned the patch is neither right nor useful:)

-daveti

Quoted 4 lines · Sudip Mukherjee

Hi Dave,
It will work. But my point of saying that was c.s.field ==(value) is again not according to the style.

Quoted 61 lines

thanks
sudip

Quoted 57 lines · Dave Tian

It also works as value is surrounded by (), though I do not think the patch itself is right or useful.

Dave Tian
dave.jing.tian at gmail.com

Quoted 47 lines · Sudip Mukherjee
Quoted 14 lines · Nicholas Krause

Fixes checkpatch coding style warning about unneeded space between function name an parentheses.

Signed-off-by Nicholas Krause <[email protected]>

---
Untested
drivers/staging/octeon-usb/octeon-hcd.c | 2 +-

1 file changed, 1 insertion(+), 1 deletion(-)
drivers/staging/octeon-usb/octeon-hcd.c +0 -0
index 5f9db4c..bbeb0cc 100644

drivers/staging/octeon-usb/octeon-hcd.c struct octeon_hcd { · line 415

Quoted 5 lines
drivers/staging/octeon-usb/octeon-hcd.c +1 -1
@@ -412,7 +412,7 @@ struct octeon_hcd {
            type c;                                                     \
            while (1) {                                                 \
                    c.u32 = __cvmx_usb_read_csr32(usb, address);        \
-                    if (c.s.field op (value)) {                         \
+                    if (c.s.field op(value)) {                          \

have you read the code before modifying it?
this is not a function.
have you seen how CVMX_WAIT_FOR_FIELD32 is being called?
on every call of CVMX_WAIT_FOR_FIELD32 op is the operator "=="
so when called the macro will be c.s.field == (value).
if your patch is applied then it will become c.s.field ==(value) .. will that be correct ?

thanks
sudip

Quoted 11 lines
drivers/staging/octeon-usb/octeon-hcd.c +0 -1
                            result = 0;                                 \
                            break;                                      \
                    } else if (cvmx_get_cycle() > done) {               \
--

1.9.1

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies at kernelnewbies.org
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies at kernelnewbies.org
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies at kernelnewbies.org
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Signature · 1 line
Peter
#6 lore raw

PATCH 1/1 staging: Fix spacing between function name and parentheses

I agree. But in my opinion checkpatch is here to help us fix style problems , but we should not blindly act on checkpatch warnings.

thanks
sudip

Quoted 92 lines · Peter Senna Tschudin

I think that, in this case, checkpatch.pl contributed:

$ ./scripts/checkpatch.pl -f drivers/staging/octeon-usb/octeon-hcd.c
WARNING: space prohibited between function name and open parenthesis '('
#415: FILE: drivers/staging/octeon-usb/octeon-hcd.c:415:
+                       if (c.s.field op (value)) {                         \
Quoted 74 lines · Dave Tian

Agreed - that is why I mentioned the patch is neither right nor useful:)

-daveti

Quoted 4 lines · Sudip Mukherjee

Hi Dave,
It will work. But my point of saying that was c.s.field ==(value) is again not according to the style.

Quoted 61 lines

thanks
sudip

Quoted 57 lines · Dave Tian

It also works as value is surrounded by (), though I do not think the patch itself is right or useful.

Dave Tian
dave.jing.tian at gmail.com

Quoted 47 lines · Sudip Mukherjee
Quoted 14 lines · Nicholas Krause

Fixes checkpatch coding style warning about unneeded space between function name an parentheses.

Signed-off-by Nicholas Krause <[email protected]>

---
Untested
drivers/staging/octeon-usb/octeon-hcd.c | 2 +-

1 file changed, 1 insertion(+), 1 deletion(-)
drivers/staging/octeon-usb/octeon-hcd.c +0 -0
index 5f9db4c..bbeb0cc 100644

drivers/staging/octeon-usb/octeon-hcd.c struct octeon_hcd { · line 415

Quoted 5 lines
drivers/staging/octeon-usb/octeon-hcd.c +1 -1
@@ -412,7 +412,7 @@ struct octeon_hcd {
            type c;                                                     \
            while (1) {                                                 \
                    c.u32 = __cvmx_usb_read_csr32(usb, address);        \
-                    if (c.s.field op (value)) {                         \
+                    if (c.s.field op(value)) {                          \

have you read the code before modifying it?
this is not a function.
have you seen how CVMX_WAIT_FOR_FIELD32 is being called?
on every call of CVMX_WAIT_FOR_FIELD32 op is the operator "=="
so when called the macro will be c.s.field == (value).
if your patch is applied then it will become c.s.field ==(value) .. will that be correct ?

thanks
sudip

Quoted 11 lines
drivers/staging/octeon-usb/octeon-hcd.c +0 -1
                            result = 0;                                 \
                            break;                                      \
                    } else if (cvmx_get_cycle() > done) {               \
--

1.9.1

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies at kernelnewbies.org
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies at kernelnewbies.org
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Quoted 4 lines

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies at kernelnewbies.org
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

--
Peter

#7 lore raw

PATCH 1/1 staging: Fix spacing between function name and parentheses

Even if you use checkpath you _should_ understand what you are changing. The output of checkpatch merely there to help.

In this case you can see that this is a macro just a few lines up in the code.

Quoted 114 lines · Sudip Mukherjee

I agree. But in my opinion checkpatch is here to help us fix style problems , but we should not blindly act on checkpatch warnings.

thanks
sudip

Quoted 6 lines · Peter Senna Tschudin

I think that, in this case, checkpatch.pl contributed:

$ ./scripts/checkpatch.pl -f drivers/staging/octeon-usb/octeon-hcd.c
WARNING: space prohibited between function name and open parenthesis '('
#415: FILE: drivers/staging/octeon-usb/octeon-hcd.c:415:
+                       if (c.s.field op (value)) {

\

Quoted 3 lines

On Sat, Oct 11, 2014 at 8:11 AM, Dave Tian <[email protected]>

Quoted 6 lines · wrote:
Quoted 6 lines

Agreed - that is why I mentioned the patch is neither right nor useful:)

-daveti

On Oct 11, 2014, at 2:08 PM, Sudip Mukherjee <

Quoted 9 lines · sudipm.mukherjee at gmail.com> wrote:
Quoted 9 lines
Quoted 9 lines

Hi Dave,
It will work. But my point of saying that was c.s.field ==(value) is again not according to the style.

thanks
sudip

On Sat, Oct 11, 2014 at 10:53 AM, Dave Tian <[email protected]>

Quoted 1 line · wrote:
Quoted 1 line
Quoted 1 line
Quoted 1 line

It also works as value is surrounded by (), though I do not think the

patch itself is right or useful.

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines

Dave Tian
dave.jing.tian at gmail.com

On Oct 11, 2014, at 12:58 PM, Sudip Mukherjee <

Quoted 5 lines · sudipm.mukherjee at gmail.com> wrote:
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines · Nicholas Krause
Quoted 3 lines

Fixes checkpatch coding style warning about unneeded space between function name an parentheses.

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines

Review

Quoted 7 lines
Signed-off-by Nicholas Krause <[email protected]>

---
Untested
drivers/staging/octeon-usb/octeon-hcd.c | 2 +-

1 file changed, 1 insertion(+), 1 deletion(-)
(quoted diff) +0 -0

b/drivers/staging/octeon-usb/octeon-hcd.c

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

drivers/staging/octeon-usb/octeon-hcd.c struct octeon_hcd { · line 412

Quoted 5 lines

index 5f9db4c..bbeb0cc 100644

drivers/staging/octeon-usb/octeon-hcd.c +0 -0
@@ -412,7 +412,7 @@ struct octeon_hcd {
            type c;

     \
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

drivers/staging/octeon-usb/octeon-hcd.c struct octeon_hcd { · line 413

Quoted 1 line
drivers/staging/octeon-usb/octeon-hcd.c +0 -0
            while (1) {

     \
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

drivers/staging/octeon-usb/octeon-hcd.c struct octeon_hcd { · line 414

Quoted 1 line
drivers/staging/octeon-usb/octeon-hcd.c +0 -0
                    c.u32 = __cvmx_usb_read_csr32(usb, address);

    \
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

drivers/staging/octeon-usb/octeon-hcd.c struct octeon_hcd { · line 415

Quoted 1 line
drivers/staging/octeon-usb/octeon-hcd.c +0 -1
-                    if (c.s.field op (value)) {

     \
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

drivers/staging/octeon-usb/octeon-hcd.c struct octeon_hcd { · line 415

Quoted 1 line
drivers/staging/octeon-usb/octeon-hcd.c +1 -0
+                    if (c.s.field op(value)) {

      \
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines

have you read the code before modifying it?
this is not a function.
have you seen how CVMX_WAIT_FOR_FIELD32 is being called? on every call of CVMX_WAIT_FOR_FIELD32 op is the operator "==" so when called the macro will be c.s.field == (value). if your patch is applied then it will become c.s.field ==(value) ..

will that be correct ?

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

thanks
sudip

Quoted 1 line
drivers/staging/octeon-usb/octeon-hcd.c +0 -0
                            result = 0;

     \
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

drivers/staging/octeon-usb/octeon-hcd.c struct octeon_hcd { · line 417

Quoted 1 line
drivers/staging/octeon-usb/octeon-hcd.c +0 -0
                            break;

    \
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

drivers/staging/octeon-usb/octeon-hcd.c struct octeon_hcd { · line 418

Quoted 1 line
drivers/staging/octeon-usb/octeon-hcd.c +0 -0
                    } else if (cvmx_get_cycle() > done) {

     \
Quoted 23 lines
Quoted 20 lines
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
Quoted 8 lines

--
1.9.1

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies at kernelnewbies.org
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Quoted 5 lines
Quoted 5 lines
Quoted 4 lines

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies at kernelnewbies.org
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies at kernelnewbies.org
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Quoted 8 lines
Quoted 2 lines

--
Peter

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies at kernelnewbies.org
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20141011/0dccbd94/attachment.html

author
#8 lore raw

PATCH 1/1 staging: Fix spacing between function name and parentheses

Thank you for your help, I'll study the code and see what I can do about it. Do you have any suggestions of how to fix this checkpatch warning?
Nick

Quoted 130 lines · Kristofer Hallin

Even if you use checkpath you _should_ understand what you are changing. The output of checkpatch merely there to help.

In this case you can see that this is a macro just a few lines up in the code.

Quoted 122 lines · Sudip Mukherjee

I agree. But in my opinion checkpatch is here to help us fix style problems , but we should not blindly act on checkpatch warnings.

thanks
sudip

Quoted 6 lines · Peter Senna Tschudin

I think that, in this case, checkpatch.pl contributed:

$ ./scripts/checkpatch.pl -f drivers/staging/octeon-usb/octeon-hcd.c
WARNING: space prohibited between function name and open parenthesis '('
#415: FILE: drivers/staging/octeon-usb/octeon-hcd.c:415:
+                       if (c.s.field op (value)) {

\

Quoted 3 lines

On Sat, Oct 11, 2014 at 8:11 AM, Dave Tian <[email protected]>

Quoted 6 lines · wrote:
Quoted 6 lines

Agreed - that is why I mentioned the patch is neither right nor useful:)

-daveti

On Oct 11, 2014, at 2:08 PM, Sudip Mukherjee <

Quoted 9 lines · sudipm.mukherjee at gmail.com> wrote:
Quoted 9 lines
Quoted 9 lines

Hi Dave,
It will work. But my point of saying that was c.s.field ==(value) is again not according to the style.

thanks
sudip

On Sat, Oct 11, 2014 at 10:53 AM, Dave Tian <[email protected]>

Quoted 1 line · wrote:
Quoted 1 line
Quoted 1 line
Quoted 1 line

It also works as value is surrounded by (), though I do not think the

patch itself is right or useful.

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines

Dave Tian
dave.jing.tian at gmail.com

On Oct 11, 2014, at 12:58 PM, Sudip Mukherjee <

Quoted 5 lines · sudipm.mukherjee at gmail.com> wrote:
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines · Nicholas Krause
Quoted 3 lines

Fixes checkpatch coding style warning about unneeded space between function name an parentheses.

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines

Review

Quoted 7 lines
Signed-off-by Nicholas Krause <[email protected]>

---
Untested
drivers/staging/octeon-usb/octeon-hcd.c | 2 +-

1 file changed, 1 insertion(+), 1 deletion(-)
(quoted diff) +0 -0

b/drivers/staging/octeon-usb/octeon-hcd.c

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

drivers/staging/octeon-usb/octeon-hcd.c struct octeon_hcd { · line 412

Quoted 5 lines

index 5f9db4c..bbeb0cc 100644

drivers/staging/octeon-usb/octeon-hcd.c +0 -0
@@ -412,7 +412,7 @@ struct octeon_hcd {
            type c;

     \
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

drivers/staging/octeon-usb/octeon-hcd.c struct octeon_hcd { · line 413

Quoted 1 line
drivers/staging/octeon-usb/octeon-hcd.c +0 -0
            while (1) {

     \
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

drivers/staging/octeon-usb/octeon-hcd.c struct octeon_hcd { · line 414

Quoted 1 line
drivers/staging/octeon-usb/octeon-hcd.c +0 -0
                    c.u32 = __cvmx_usb_read_csr32(usb, address);

    \
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

drivers/staging/octeon-usb/octeon-hcd.c struct octeon_hcd { · line 415

Quoted 1 line
drivers/staging/octeon-usb/octeon-hcd.c +0 -1
-                    if (c.s.field op (value)) {

     \
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

drivers/staging/octeon-usb/octeon-hcd.c struct octeon_hcd { · line 415

Quoted 1 line
drivers/staging/octeon-usb/octeon-hcd.c +1 -0
+                    if (c.s.field op(value)) {

      \
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines

have you read the code before modifying it?
this is not a function.
have you seen how CVMX_WAIT_FOR_FIELD32 is being called? on every call of CVMX_WAIT_FOR_FIELD32 op is the operator "==" so when called the macro will be c.s.field == (value). if your patch is applied then it will become c.s.field ==(value) ..

will that be correct ?

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

thanks
sudip

Quoted 1 line
drivers/staging/octeon-usb/octeon-hcd.c +0 -0
                            result = 0;

     \
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

drivers/staging/octeon-usb/octeon-hcd.c struct octeon_hcd { · line 417

Quoted 1 line
drivers/staging/octeon-usb/octeon-hcd.c +0 -0
                            break;

    \
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

drivers/staging/octeon-usb/octeon-hcd.c struct octeon_hcd { · line 418

Quoted 1 line
drivers/staging/octeon-usb/octeon-hcd.c +0 -0
                    } else if (cvmx_get_cycle() > done) {

     \
Quoted 23 lines
Quoted 16 lines
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
Quoted 8 lines

--
1.9.1

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies at kernelnewbies.org
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Quoted 5 lines
Quoted 5 lines
Quoted 4 lines

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies at kernelnewbies.org
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Quoted 4 lines

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies at kernelnewbies.org
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Quoted 2 lines

--
Peter

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies at kernelnewbies.org
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

#10 lore raw

PATCH 1/1 staging: Fix spacing between function name and parentheses

In reply to nick
Quoted 3 lines · nick

Thank you for your help, I'll study the code and see what I can do about it. Do you have any suggestions of how to fix this checkpatch warning?

Ignore it. The checker has clearly triggered on a false positive --
this is not a function call, and should not be held to that standard. (Take a look at where the macro is actually used, to see what's going on here). Move on to find something more interesting to fix.

Hugo.

Quoted 101 lines

Nick

Quoted 98 lines · Kristofer Hallin

Even if you use checkpath you _should_ understand what you are changing. The output of checkpatch merely there to help.

In this case you can see that this is a macro just a few lines up in the code.

Quoted 91 lines · Sudip Mukherjee

I agree. But in my opinion checkpatch is here to help us fix style problems , but we should not blindly act on checkpatch warnings.

thanks
sudip

Quoted 6 lines · Peter Senna Tschudin

I think that, in this case, checkpatch.pl contributed:

$ ./scripts/checkpatch.pl -f drivers/staging/octeon-usb/octeon-hcd.c
WARNING: space prohibited between function name and open parenthesis '('
#415: FILE: drivers/staging/octeon-usb/octeon-hcd.c:415:
+                       if (c.s.field op (value)) {

\

Quoted 3 lines

On Sat, Oct 11, 2014 at 8:11 AM, Dave Tian <[email protected]>

Quoted 6 lines · wrote:
Quoted 6 lines

Agreed - that is why I mentioned the patch is neither right nor useful:)

-daveti

On Oct 11, 2014, at 2:08 PM, Sudip Mukherjee <

Quoted 9 lines · sudipm.mukherjee at gmail.com> wrote:
Quoted 9 lines
Quoted 9 lines

Hi Dave,
It will work. But my point of saying that was c.s.field ==(value) is again not according to the style.

thanks
sudip

On Sat, Oct 11, 2014 at 10:53 AM, Dave Tian <[email protected]>

Quoted 1 line · wrote:
Quoted 1 line
Quoted 1 line
Quoted 1 line

It also works as value is surrounded by (), though I do not think the

patch itself is right or useful.

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines

Dave Tian
dave.jing.tian at gmail.com

On Oct 11, 2014, at 12:58 PM, Sudip Mukherjee <

Quoted 5 lines · sudipm.mukherjee at gmail.com> wrote:
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines · Nicholas Krause
Quoted 3 lines

Fixes checkpatch coding style warning about unneeded space between function name an parentheses.

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines

Review

Quoted 7 lines
Signed-off-by Nicholas Krause <[email protected]>

---
Untested
drivers/staging/octeon-usb/octeon-hcd.c | 2 +-

1 file changed, 1 insertion(+), 1 deletion(-)
(quoted diff) +0 -0

b/drivers/staging/octeon-usb/octeon-hcd.c

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

drivers/staging/octeon-usb/octeon-hcd.c struct octeon_hcd { · line 412

Quoted 5 lines

index 5f9db4c..bbeb0cc 100644

drivers/staging/octeon-usb/octeon-hcd.c +0 -0
@@ -412,7 +412,7 @@ struct octeon_hcd {
            type c;

     \
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

drivers/staging/octeon-usb/octeon-hcd.c struct octeon_hcd { · line 413

Quoted 1 line
drivers/staging/octeon-usb/octeon-hcd.c +0 -0
            while (1) {

     \
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

drivers/staging/octeon-usb/octeon-hcd.c struct octeon_hcd { · line 414

Quoted 1 line
drivers/staging/octeon-usb/octeon-hcd.c +0 -0
                    c.u32 = __cvmx_usb_read_csr32(usb, address);

    \
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

drivers/staging/octeon-usb/octeon-hcd.c struct octeon_hcd { · line 415

Quoted 1 line
drivers/staging/octeon-usb/octeon-hcd.c +0 -1
-                    if (c.s.field op (value)) {

     \
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

drivers/staging/octeon-usb/octeon-hcd.c struct octeon_hcd { · line 415

Quoted 1 line
drivers/staging/octeon-usb/octeon-hcd.c +1 -0
+                    if (c.s.field op(value)) {

      \
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines

have you read the code before modifying it?
this is not a function.
have you seen how CVMX_WAIT_FOR_FIELD32 is being called? on every call of CVMX_WAIT_FOR_FIELD32 op is the operator "==" so when called the macro will be c.s.field == (value). if your patch is applied then it will become c.s.field ==(value) ..

will that be correct ?

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

thanks
sudip

Quoted 1 line
drivers/staging/octeon-usb/octeon-hcd.c +0 -0
                            result = 0;

     \
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

drivers/staging/octeon-usb/octeon-hcd.c struct octeon_hcd { · line 417

Quoted 1 line
drivers/staging/octeon-usb/octeon-hcd.c +0 -0
                            break;

    \
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

drivers/staging/octeon-usb/octeon-hcd.c struct octeon_hcd { · line 418

Quoted 1 line
drivers/staging/octeon-usb/octeon-hcd.c +0 -0
                    } else if (cvmx_get_cycle() > done) {

     \
Signature · 10 lines
=== Hugo Mills: hugo at ... carfax.org.uk | darksatanic.net | lug.org.uk ===
  PGP key: 65E74AC0 from wwwkeys.eu.pgp.net or http://www.carfax.org.uk
             --- Happiness is mandatory.  Are you happy? ---             
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: Digital signature
Url : http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20141011/a7281663/attachment-0001.bin
author
#11 lore raw

PATCH 1/1 staging: Fix spacing between function name and parentheses

Thanks Hugo,
Sorry about that. On the other hand was the patch good in terms of format? Cheers Nick

Quoted 114 lines · Hugo Mills
Quoted 3 lines · nick

Thank you for your help, I'll study the code and see what I can do about it. Do you have any suggestions of how to fix this checkpatch warning?

Ignore it. The checker has clearly triggered on a false positive --
this is not a function call, and should not be held to that standard. (Take a look at where the macro is actually used, to see what's going on here). Move on to find something more interesting to fix.

Hugo.

Quoted 101 lines

Nick

Quoted 98 lines · Kristofer Hallin

Even if you use checkpath you _should_ understand what you are changing. The output of checkpatch merely there to help.

In this case you can see that this is a macro just a few lines up in the code.

Quoted 91 lines · Sudip Mukherjee

I agree. But in my opinion checkpatch is here to help us fix style problems , but we should not blindly act on checkpatch warnings.

thanks
sudip

Quoted 6 lines · Peter Senna Tschudin

I think that, in this case, checkpatch.pl contributed:

$ ./scripts/checkpatch.pl -f drivers/staging/octeon-usb/octeon-hcd.c
WARNING: space prohibited between function name and open parenthesis '('
#415: FILE: drivers/staging/octeon-usb/octeon-hcd.c:415:
+                       if (c.s.field op (value)) {

\

Quoted 3 lines

On Sat, Oct 11, 2014 at 8:11 AM, Dave Tian <[email protected]>

Quoted 6 lines · wrote:
Quoted 6 lines

Agreed - that is why I mentioned the patch is neither right nor useful:)

-daveti

On Oct 11, 2014, at 2:08 PM, Sudip Mukherjee <

Quoted 9 lines · sudipm.mukherjee at gmail.com> wrote:
Quoted 9 lines
Quoted 9 lines

Hi Dave,
It will work. But my point of saying that was c.s.field ==(value) is again not according to the style.

thanks
sudip

On Sat, Oct 11, 2014 at 10:53 AM, Dave Tian <[email protected]>

Quoted 1 line · wrote:
Quoted 1 line
Quoted 1 line
Quoted 1 line

It also works as value is surrounded by (), though I do not think the

patch itself is right or useful.

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines

Dave Tian
dave.jing.tian at gmail.com

On Oct 11, 2014, at 12:58 PM, Sudip Mukherjee <

Quoted 5 lines · sudipm.mukherjee at gmail.com> wrote:
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines · Nicholas Krause
Quoted 3 lines

Fixes checkpatch coding style warning about unneeded space between function name an parentheses.

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines

Review

Quoted 7 lines
Signed-off-by Nicholas Krause <[email protected]>

---
Untested
drivers/staging/octeon-usb/octeon-hcd.c | 2 +-

1 file changed, 1 insertion(+), 1 deletion(-)
(quoted diff) +0 -0

b/drivers/staging/octeon-usb/octeon-hcd.c

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

drivers/staging/octeon-usb/octeon-hcd.c struct octeon_hcd { · line 412

Quoted 5 lines

index 5f9db4c..bbeb0cc 100644

drivers/staging/octeon-usb/octeon-hcd.c +0 -0
@@ -412,7 +412,7 @@ struct octeon_hcd {
            type c;

     \
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

drivers/staging/octeon-usb/octeon-hcd.c struct octeon_hcd { · line 413

Quoted 1 line
drivers/staging/octeon-usb/octeon-hcd.c +0 -0
            while (1) {

     \
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

drivers/staging/octeon-usb/octeon-hcd.c struct octeon_hcd { · line 414

Quoted 1 line
drivers/staging/octeon-usb/octeon-hcd.c +0 -0
                    c.u32 = __cvmx_usb_read_csr32(usb, address);

    \
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

drivers/staging/octeon-usb/octeon-hcd.c struct octeon_hcd { · line 415

Quoted 1 line
drivers/staging/octeon-usb/octeon-hcd.c +0 -1
-                    if (c.s.field op (value)) {

     \
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

drivers/staging/octeon-usb/octeon-hcd.c struct octeon_hcd { · line 415

Quoted 1 line
drivers/staging/octeon-usb/octeon-hcd.c +1 -0
+                    if (c.s.field op(value)) {

      \
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines

have you read the code before modifying it?
this is not a function.
have you seen how CVMX_WAIT_FOR_FIELD32 is being called? on every call of CVMX_WAIT_FOR_FIELD32 op is the operator "==" so when called the macro will be c.s.field == (value). if your patch is applied then it will become c.s.field ==(value) ..

will that be correct ?

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

thanks
sudip

Quoted 1 line
drivers/staging/octeon-usb/octeon-hcd.c +0 -0
                            result = 0;

     \
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

drivers/staging/octeon-usb/octeon-hcd.c struct octeon_hcd { · line 417

Quoted 1 line
drivers/staging/octeon-usb/octeon-hcd.c +0 -0
                            break;

    \
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

drivers/staging/octeon-usb/octeon-hcd.c struct octeon_hcd { · line 418

Quoted 1 line
drivers/staging/octeon-usb/octeon-hcd.c +0 -0
                    } else if (cvmx_get_cycle() > done) {

     \
#12 lore raw

PATCH 1/1 staging: Fix spacing between function name and parentheses

Hey Nick,
Nice try to fix a checkpatch warning. But do read what you're changing. Yes your format is right. If you haven't already, take a look at "git send-email" .
Have fun hacking :D
Regards,
Karthik Nayak

Quoted 124 lines · nick

Thanks Hugo,
Sorry about that. On the other hand was the patch good in terms of format? Cheers Nick

Quoted 5 lines · Hugo Mills
Quoted 3 lines · nick

Thank you for your help, I'll study the code and see what I can do about it. Do you have any suggestions of how to fix this checkpatch warning?

Quoted 109 lines

Ignore it. The checker has clearly triggered on a false positive --
this is not a function call, and should not be held to that standard. (Take a look at where the macro is actually used, to see what's going on here). Move on to find something more interesting to fix.

Hugo.

Quoted 101 lines

Nick

Quoted 98 lines · Kristofer Hallin

Even if you use checkpath you _should_ understand what you are changing. The output of checkpatch merely there to help.

In this case you can see that this is a macro just a few lines up in the code.

Quoted 91 lines · Sudip Mukherjee

I agree. But in my opinion checkpatch is here to help us fix style problems , but we should not blindly act on checkpatch warnings.

thanks
sudip

Quoted 6 lines · Peter Senna Tschudin

I think that, in this case, checkpatch.pl contributed:

$ ./scripts/checkpatch.pl -f drivers/staging/octeon-usb/octeon-hcd.c
WARNING: space prohibited between function name and open parenthesis '('
#415: FILE: drivers/staging/octeon-usb/octeon-hcd.c:415:
+                       if (c.s.field op (value)) {

\

Quoted 3 lines

On Sat, Oct 11, 2014 at 8:11 AM, Dave Tian <[email protected]>

Quoted 6 lines · wrote:
Quoted 6 lines

Agreed - that is why I mentioned the patch is neither right nor useful:)

-daveti

On Oct 11, 2014, at 2:08 PM, Sudip Mukherjee <

Quoted 9 lines · sudipm.mukherjee at gmail.com> wrote:
Quoted 9 lines
Quoted 9 lines

Hi Dave,
It will work. But my point of saying that was c.s.field ==(value) is again not according to the style.

thanks
sudip

On Sat, Oct 11, 2014 at 10:53 AM, Dave Tian <[email protected]>

Quoted 1 line · wrote:
Quoted 1 line
Quoted 1 line
Quoted 1 line

It also works as value is surrounded by (), though I do not think the

patch itself is right or useful.

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines

Dave Tian
dave.jing.tian at gmail.com

On Oct 11, 2014, at 12:58 PM, Sudip Mukherjee <

Quoted 5 lines · sudipm.mukherjee at gmail.com> wrote:
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines · Nicholas Krause
Quoted 3 lines

Fixes checkpatch coding style warning about unneeded space between function name an parentheses.

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines

Review

Quoted 7 lines
Signed-off-by Nicholas Krause <[email protected]>

---
Untested
drivers/staging/octeon-usb/octeon-hcd.c | 2 +-

1 file changed, 1 insertion(+), 1 deletion(-)
(quoted diff) +0 -0

b/drivers/staging/octeon-usb/octeon-hcd.c

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

drivers/staging/octeon-usb/octeon-hcd.c struct octeon_hcd { · line 412

Quoted 5 lines

index 5f9db4c..bbeb0cc 100644

drivers/staging/octeon-usb/octeon-hcd.c +0 -0
@@ -412,7 +412,7 @@ struct octeon_hcd {
            type c;

     \
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

drivers/staging/octeon-usb/octeon-hcd.c struct octeon_hcd { · line 413

Quoted 1 line
drivers/staging/octeon-usb/octeon-hcd.c +0 -0
            while (1) {

     \
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

drivers/staging/octeon-usb/octeon-hcd.c struct octeon_hcd { · line 414

Quoted 1 line
drivers/staging/octeon-usb/octeon-hcd.c +0 -0
                    c.u32 = __cvmx_usb_read_csr32(usb, address);

    \
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

drivers/staging/octeon-usb/octeon-hcd.c struct octeon_hcd { · line 415

Quoted 1 line
drivers/staging/octeon-usb/octeon-hcd.c +0 -1
-                    if (c.s.field op (value)) {

     \
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

drivers/staging/octeon-usb/octeon-hcd.c struct octeon_hcd { · line 415

Quoted 1 line
drivers/staging/octeon-usb/octeon-hcd.c +1 -0
+                    if (c.s.field op(value)) {

      \
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines

have you read the code before modifying it?
this is not a function.
have you seen how CVMX_WAIT_FOR_FIELD32 is being called? on every call of CVMX_WAIT_FOR_FIELD32 op is the operator "==" so when called the macro will be c.s.field == (value). if your patch is applied then it will become c.s.field ==(value) ..

will that be correct ?

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

thanks
sudip

Quoted 1 line
drivers/staging/octeon-usb/octeon-hcd.c +0 -0
                            result = 0;

     \
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

drivers/staging/octeon-usb/octeon-hcd.c struct octeon_hcd { · line 417

Quoted 1 line
drivers/staging/octeon-usb/octeon-hcd.c +0 -0
                            break;

    \
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

drivers/staging/octeon-usb/octeon-hcd.c struct octeon_hcd { · line 418

Quoted 1 line
drivers/staging/octeon-usb/octeon-hcd.c +0 -0
                    } else if (cvmx_get_cycle() > done) {

     \

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies at kernelnewbies.org
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

author
#13 lore raw

PATCH 1/1 staging: Fix spacing between function name and parentheses

Great to known. Sorry about not reading the code, will check more carefully before I fix it.
Regards Nick

Quoted 134 lines · karthik nayak

Hey Nick,
Nice try to fix a checkpatch warning. But do read what you're changing. Yes your format is right. If you haven't already, take a look at "git send-email" .
Have fun hacking :D
Regards,
Karthik Nayak

Quoted 124 lines · nick

Thanks Hugo,
Sorry about that. On the other hand was the patch good in terms of format? Cheers Nick

Quoted 5 lines · Hugo Mills
Quoted 3 lines · nick

Thank you for your help, I'll study the code and see what I can do about it. Do you have any suggestions of how to fix this checkpatch warning?

Quoted 109 lines

Ignore it. The checker has clearly triggered on a false positive --
this is not a function call, and should not be held to that standard. (Take a look at where the macro is actually used, to see what's going on here). Move on to find something more interesting to fix.

Hugo.

Quoted 101 lines

Nick

Quoted 98 lines · Kristofer Hallin

Even if you use checkpath you _should_ understand what you are changing. The output of checkpatch merely there to help.

In this case you can see that this is a macro just a few lines up in the code.

Quoted 91 lines · Sudip Mukherjee

I agree. But in my opinion checkpatch is here to help us fix style problems , but we should not blindly act on checkpatch warnings.

thanks
sudip

Quoted 6 lines · Peter Senna Tschudin

I think that, in this case, checkpatch.pl contributed:

$ ./scripts/checkpatch.pl -f drivers/staging/octeon-usb/octeon-hcd.c
WARNING: space prohibited between function name and open parenthesis '('
#415: FILE: drivers/staging/octeon-usb/octeon-hcd.c:415:
+                       if (c.s.field op (value)) {

\

Quoted 3 lines

On Sat, Oct 11, 2014 at 8:11 AM, Dave Tian <[email protected]>

Quoted 6 lines · wrote:
Quoted 6 lines

Agreed - that is why I mentioned the patch is neither right nor useful:)

-daveti

On Oct 11, 2014, at 2:08 PM, Sudip Mukherjee <

Quoted 9 lines · sudipm.mukherjee at gmail.com> wrote:
Quoted 9 lines
Quoted 9 lines

Hi Dave,
It will work. But my point of saying that was c.s.field ==(value) is again not according to the style.

thanks
sudip

On Sat, Oct 11, 2014 at 10:53 AM, Dave Tian <[email protected]>

Quoted 1 line · wrote:
Quoted 1 line
Quoted 1 line
Quoted 1 line

It also works as value is surrounded by (), though I do not think the

patch itself is right or useful.

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines

Dave Tian
dave.jing.tian at gmail.com

On Oct 11, 2014, at 12:58 PM, Sudip Mukherjee <

Quoted 5 lines · sudipm.mukherjee at gmail.com> wrote:
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines · Nicholas Krause
Quoted 3 lines

Fixes checkpatch coding style warning about unneeded space between function name an parentheses.

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines

Review

Quoted 7 lines
Signed-off-by Nicholas Krause <[email protected]>

---
Untested
drivers/staging/octeon-usb/octeon-hcd.c | 2 +-

1 file changed, 1 insertion(+), 1 deletion(-)
(quoted diff) +0 -0

b/drivers/staging/octeon-usb/octeon-hcd.c

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

drivers/staging/octeon-usb/octeon-hcd.c struct octeon_hcd { · line 412

Quoted 5 lines

index 5f9db4c..bbeb0cc 100644

drivers/staging/octeon-usb/octeon-hcd.c +0 -0
@@ -412,7 +412,7 @@ struct octeon_hcd {
            type c;

     \
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

drivers/staging/octeon-usb/octeon-hcd.c struct octeon_hcd { · line 413

Quoted 1 line
drivers/staging/octeon-usb/octeon-hcd.c +0 -0
            while (1) {

     \
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

drivers/staging/octeon-usb/octeon-hcd.c struct octeon_hcd { · line 414

Quoted 1 line
drivers/staging/octeon-usb/octeon-hcd.c +0 -0
                    c.u32 = __cvmx_usb_read_csr32(usb, address);

    \
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

drivers/staging/octeon-usb/octeon-hcd.c struct octeon_hcd { · line 415

Quoted 1 line
drivers/staging/octeon-usb/octeon-hcd.c +0 -1
-                    if (c.s.field op (value)) {

     \
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

drivers/staging/octeon-usb/octeon-hcd.c struct octeon_hcd { · line 415

Quoted 1 line
drivers/staging/octeon-usb/octeon-hcd.c +1 -0
+                    if (c.s.field op(value)) {

      \
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines

have you read the code before modifying it?
this is not a function.
have you seen how CVMX_WAIT_FOR_FIELD32 is being called? on every call of CVMX_WAIT_FOR_FIELD32 op is the operator "==" so when called the macro will be c.s.field == (value). if your patch is applied then it will become c.s.field ==(value) ..

will that be correct ?

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

thanks
sudip

Quoted 1 line
drivers/staging/octeon-usb/octeon-hcd.c +0 -0
                            result = 0;

     \
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

drivers/staging/octeon-usb/octeon-hcd.c struct octeon_hcd { · line 417

Quoted 1 line
drivers/staging/octeon-usb/octeon-hcd.c +0 -0
                            break;

    \
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

drivers/staging/octeon-usb/octeon-hcd.c struct octeon_hcd { · line 418

Quoted 1 line
drivers/staging/octeon-usb/octeon-hcd.c +0 -0
                    } else if (cvmx_get_cycle() > done) {

     \

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies at kernelnewbies.org
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

#14 lore raw

PATCH 1/1 staging: Fix spacing between function name and parentheses

In reply to Hugo Mills
Quoted 9 lines · Hugo Mills
Quoted 3 lines · nick

Thank you for your help, I'll study the code and see what I can do about it. Do you have any suggestions of how to fix this checkpatch warning?

Ignore it. The checker has clearly triggered on a false positive --
this is not a function call, and should not be held to that standard. (Take a look at where the macro is actually used, to see what's going on here). Move on to find something more interesting to fix.

Am I the only one suspicious of the fact that Nick found one of the
few false positives rather than one of the plentiful actual style
problems?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 848 bytes
Desc: not available
Url : http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20141012/eb18dacc/attachment.bin

author
#15 lore raw

PATCH 1/1 staging: Fix spacing between function name and parentheses

Valdis,
I do understand your concern with me finding one of the few false positives, due to my issues already with the community. I am trying my best now to improve and work toward good,solid work that is of actual use to the community and avoid shitty patches and not listening like I did for the last few months. Sorry,
Nick

Quoted 16 lines · Valdis.Kletnieks at vt.edu
Quoted 9 lines · Hugo Mills
Quoted 3 lines · nick

Thank you for your help, I'll study the code and see what I can do about it. Do you have any suggestions of how to fix this checkpatch warning?

Ignore it. The checker has clearly triggered on a false positive --
this is not a function call, and should not be held to that standard. (Take a look at where the macro is actually used, to see what's going on here). Move on to find something more interesting to fix.

Am I the only one suspicious of the fact that Nick found one of the few false positives rather than one of the plentiful actual style problems?

#16 lore raw

PATCH 1/1 staging: Fix spacing between function name and parentheses

Quoted 5 lines · nick

Valdis,
I do understand your concern with me finding one of the few false positives, due to my issues already with the community. I am trying my best now to improve and work toward good,solid work that is of actual use to the community and avoid shitty patches and not listening like I did for the last few months.

Forget it Nick. We've heard that too many times. And this time, you blew it by making your bad patch a bit *too* unbelievable. We're tired of your trolling.

Where's my drum of Troll-be-gone?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 848 bytes
Desc: not available
Url : http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20141012/28aeff75/attachment.bin

#17 lore raw

PATCH 1/1 staging: Fix spacing between function name and parentheses

In reply to Valdis.Kletnieks at vt.edu
Quoted 7 lines · On Mon, Oct 13, 2014 at 4:04 AM, <[email protected]> wrote:
Quoted 5 lines · Hugo Mills
Quoted 3 lines · nick

Thank you for your help, I'll study the code and see what I can do about it. Do you have any suggestions of how to fix this checkpatch warning?

Quoted 8 lines
Quoted 4 lines

Ignore it. The checker has clearly triggered on a false positive --
this is not a function call, and should not be held to that standard. (Take a look at where the macro is actually used, to see what's going on here). Move on to find something more interesting to fix.

Am I the only one suspicious of the fact that Nick found one of the few false positives rather than one of the plentiful actual style problems?

no , you are not the only one. That particular file has lots of checkpatch warning , and our Nick choose just the one which is a false positive.

#18 lore raw

PATCH 1/1 staging: Fix spacing between function name and parentheses

-----Original Message-----
From: [email protected] [mailto:kernelnewbies-bounces at kernelnewbies.org] On Behalf Of Sudip Mukherjee
Sent: Sunday, October 12, 2014 10:56 PM
To: Valdis Kletnieks
Cc: Dave Tian; nick; Kristofer Hallin; kernelnewbies; Peter Senna Tschudin; Hugo Mills
Subject: Re: [PATCH] staging: Fix spacing between function name and parentheses

Quoted 7 lines · On Mon, Oct 13, 2014 at 4:04 AM, <[email protected]> wrote:
Quoted 5 lines · Hugo Mills
Quoted 3 lines · nick

Thank you for your help, I'll study the code and see what I can do about it. Do you have any suggestions of how to fix this checkpatch warning?

Quoted 8 lines
Quoted 4 lines

Ignore it. The checker has clearly triggered on a false positive --
this is not a function call, and should not be held to that standard. (Take a look at where the macro is actually used, to see what's going on here). Move on to find something more interesting to fix.

Am I the only one suspicious of the fact that Nick found one of the few false positives rather than one of the plentiful actual style problems?

no , you are not the only one. That particular file has lots of checkpatch warning , and our Nick choose just the one which is a false positive.

What is wrong with you people - were you never on nntp and learned how to ignore trolls? I only read this list and Nicholas Krause is a pain - why where there ~11 responses to his stuff - he should be ignored - he is not banned to never respond to the idiot.

Nobody wants him, he only wants a byline of minor fame by says 'he I wrote some of linux' 3 months of this crap and you are still all entertaining someone with no code skill accept from adding/removing spaces - we all need house cleaners - but I would not trust him to clean my toilet correctly.

I raise the simple motion that when Nicholas Krause posts - NOBODY responds to him - EVER - under threat of torture or whatever - for whatever reason - my joy of lurking here has been ruined by a single IDIOT - if this was nntp he would have wandered off a long time ago with ignores - but I blame this list for constantly responding to the TROLL and FEEDING it. DONT DO IT

#19 lore raw

PATCH 1/1 staging: Fix spacing between function name and parentheses

Quoted 2 lines

I raise the simple motion that when Nicholas Krause posts - NOBODY responds to him - EVER - under threat of torture or whatever - for whatever reason - my joy of lurking here has been ruined by a single IDIOT - if this was nntp he would have wandered off a long time ago with ignores - but I blame this list for constantly responding to the TROLL and FEEDING it. DONT DO IT

Couldn't agree more. Just don't.

#20 lore raw

PATCH 1/1 staging: Fix spacing between function name and parentheses

In reply to Adrian Cornish
Quoted 1 line · Adrian Cornish

What is wrong with you people - were you never on nntp and learned how to ignore trolls?

This is what I have been thinking

#21 lore raw

PATCH 1/1 staging: Fix spacing between function name and parentheses

In reply to Adrian Cornish
Quoted 7 lines · Adrian Cornish <adrianc <at> cqg.com> writes:

I raise the simple motion that when Nicholas Krause posts - NOBODY responds to him - EVER - under threat of torture or whatever - for whatever reason -
my joy of lurking here has been ruined by a single IDIOT - if this was nntp he would have wandered off a long time ago with ignores - but I blame this list for constantly responding to the TROLL and FEEDING it. DONT DO IT

Unfortunately this isn't the time of NNTP, and since lkml (and newbies) always was a mailing list, the only hope is in respective peoples' killfiles...

We need to assume that the list is being watched not only by technically able people, but also by ones who have just a filter for [PATCH] and don't always get the difference between Reviewed-by and Acked-by... AND some that could assume that no-answer to a [PATCH] equals silent acceptance ... Or if he's con enough, he could say, oh see all the patches I submitted, no one ever said anything against them...

(and as Greg K-H once said, a few /accepted/ patches can make
someone employed...)

Maybe better to introduce a standard clear marker that able people just respond with, to alikes of nick:

REJECTED-by Name <[email protected]>

single line, maybe add 1 line of technical explanation
and DO NOT react* to any further emails demanding further explanation.

(* e.g. place on kill file in your private MUA, so the only way
they can ever get un-ignored is IFF they submit VALID patches to public list AND they get at least Acked.).

(Rationale:

REJECTED, because Nacked-by contains the 'acked' word... too easy to overlook.
CAPITALS because they do stand out.

REJECTED also is clear in intent in any english<->native dictionary,

REJECTED is (IMO) also a strong enough word to not need any further emotional emits...

It's NOT intended in the way the April Fools like/dislike patches on Facebook was suggested. Nothing to emotionally react to, just a signal that THIS PATCH IS NOT GOING TO BE APPLIED.

Also it's a serious word, contrary to Trash-detected-by or This-is-BS-by or Stay-away-from-my-list-by...

Only to be used as 'last resort' if someone repeatedly fails sanity test.

Because alternative is, that the mailing list(s) become invite-only, and that is not going to happen.
)

el_es

#22 lore raw

PATCH 1/1 staging: Fix spacing between function name and parentheses

Quoted 4 lines · el_es

Maybe better to introduce a standard clear marker that able people just respond with, to alikes of nick:

REJECTED-by Name <[email protected]>

We already do something like this.

You'll on occasion see 'Nacked-By: ...' go by when a kernel hacker wants to denote their displeasure with a given patch. It's up to the maintainer to decide how much credence to give to the Nack, based on the relative reputations of the person submitting the patch and the person nacking it, and any technical grounds given with the nack.

So for instance, if Al Viro sticks a Nacked-By: on a submission, it's going to be *really* hard to get a maintainer to accept the patch, because Al has a very long history of almost always being right about such things.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 848 bytes
Desc: not available
Url : http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20141101/53c6e3d0/attachment.bin

#23 lore raw

PATCH 1/1 staging: Fix spacing between function name and parentheses

Quoted 15 lines · Valdis.Kletnieks at vt.edu
Quoted 4 lines · el_es

Maybe better to introduce a standard clear marker that able people just respond with, to alikes of nick:

REJECTED-by Name <[email protected]>

We already do something like this.

You'll on occasion see 'Nacked-By: ...' go by when a kernel hacker wants to denote their displeasure with a given patch. It's up to the maintainer to decide how much credence to give to the Nack, based on the relative reputations of the person submitting the patch and the person nacking it, and any technical grounds given with the nack.

Quoted 3 lines

So for instance, if Al Viro sticks a Nacked-By: on a submission, it's going to be *really* hard to get a maintainer to accept the patch, because Al has a very long history of almost always being right about such things.

Yes, but the technical grounds are still the reason the patch is not accepted. Which is why a formalized nak is pointless. It has no value without a verbose explanation of the technical grounds behind it. If Al Viro, or anyone else, use a simple one-line reject message, then I am pretty sure that is because they have already explained their objections somewhere else. I don't think anyone can reject anything merely on their personal reputation. And there is nowhere to record naks, so a standard label just isn't needed.

Rejecting is completely different from e.g. Acked-by, which both is a complete explanation ("I am fine with this patch as it is") and is recorded for future reference in the commit message.

Bj?rn

author
#24 lore raw

PATCH 1/1 staging: Fix spacing between function name and parentheses

Greetings Bjorn and others,
I sent this patch a few weeks ago and am really trying to to improve my rep here by sending solid patches. In addition I understand the caution with my patches due to my terrible rep but it would be nice if someone sends or acknowledges the patch as other people on this list have stated it's correct and good in previous emails to me.
Nick

Quoted 41 lines · Bj?rn Mork
Quoted 15 lines · Valdis.Kletnieks at vt.edu
Quoted 4 lines · el_es

Maybe better to introduce a standard clear marker that able people just respond with, to alikes of nick:

REJECTED-by Name <[email protected]>

We already do something like this.

You'll on occasion see 'Nacked-By: ...' go by when a kernel hacker wants to denote their displeasure with a given patch. It's up to the maintainer to decide how much credence to give to the Nack, based on the relative reputations of the person submitting the patch and the person nacking it, and any technical grounds given with the nack.

Quoted 3 lines

So for instance, if Al Viro sticks a Nacked-By: on a submission, it's going to be *really* hard to get a maintainer to accept the patch, because Al has a very long history of almost always being right about such things.

Yes, but the technical grounds are still the reason the patch is not accepted. Which is why a formalized nak is pointless. It has no value without a verbose explanation of the technical grounds behind it. If Al Viro, or anyone else, use a simple one-line reject message, then I am pretty sure that is because they have already explained their objections somewhere else. I don't think anyone can reject anything merely on their personal reputation. And there is nowhere to record naks, so a standard label just isn't needed.

Rejecting is completely different from e.g. Acked-by, which both is a complete explanation ("I am fine with this patch as it is") and is recorded for future reference in the commit message.

Bj?rn

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies at kernelnewbies.org
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

#25 lore raw

PATCH 1/1 staging: Fix spacing between function name and parentheses

In reply to nick
Quoted 3 lines · nick

Thank you for your help, I'll study the code and see what I can do about it. Do you have any suggestions of how to fix this checkpatch warning?

there is absolutely nothing wrong with that code -- the problem is
your utter lack of understanding.

rday

Signature · 7 lines
========================================================================
Robert P. J. Day                                 Ottawa, Ontario, CANADA
                        http://crashcourse.ca

Twitter:                                       http://twitter.com/rpjday
LinkedIn:                               http://ca.linkedin.com/in/rpjday
========================================================================