All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] firewire: mark expected switch fall-throughs
@ 2019-02-11 18:03 Gustavo A. R. Silva
  2019-03-14 10:00 ` Stefan Richter
  0 siblings, 1 reply; 3+ messages in thread
From: Gustavo A. R. Silva @ 2019-02-11 18:03 UTC (permalink / raw)
  To: Stefan Richter
  Cc: linux1394-devel, linux-kernel, Gustavo A. R. Silva, Kees Cook

In preparation to enabling -Wimplicit-fallthrough, mark switch
cases where we are expecting to fall through.

This patch fixes the following warnings:

drivers/firewire/core-device.c: In function ‘set_broadcast_channel’:
drivers/firewire/core-device.c:969:7: warning: this statement may fall through [-Wimplicit-fallthrough=]
    if (data & cpu_to_be32(1 << 31)) {
       ^
drivers/firewire/core-device.c:974:3: note: here
   case RCODE_ADDRESS_ERROR:
   ^~~~
drivers/firewire/core-iso.c: In function ‘manage_channel’:
drivers/firewire/core-iso.c:308:7: warning: this statement may fall through [-Wimplicit-fallthrough=]
    if ((data[0] & bit) == (data[1] & bit))
       ^
drivers/firewire/core-iso.c:312:3: note: here
   default:
   ^~~~~~~
drivers/firewire/core-topology.c: In function ‘count_ports’:
drivers/firewire/core-topology.c:69:23: warning: this statement may fall through [-Wimplicit-fallthrough=]
    (*child_port_count)++;
    ~~~~~~~~~~~~~~~~~~~^~
drivers/firewire/core-topology.c:70:3: note: here
   case SELFID_PORT_PARENT:
   ^~~~

Warning level 3 was used: -Wimplicit-fallthrough=3

Notice that in some cases, the code comment is modified in
accordance with what GCC is expecting to find.

This patch is part of the ongoing efforts to enable
-Wimplicit-fallthrough.

Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
 drivers/firewire/core-device.c   | 2 +-
 drivers/firewire/core-iso.c      | 2 +-
 drivers/firewire/core-topology.c | 1 +
 3 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/firewire/core-device.c b/drivers/firewire/core-device.c
index 7c2eed76011e..0c86548fa4a7 100644
--- a/drivers/firewire/core-device.c
+++ b/drivers/firewire/core-device.c
@@ -970,7 +970,7 @@ static void set_broadcast_channel(struct fw_device *device, int generation)
 				device->bc_implemented = BC_IMPLEMENTED;
 				break;
 			}
-			/* else fall through to case address error */
+			/* else, fall through - to case address error */
 		case RCODE_ADDRESS_ERROR:
 			device->bc_implemented = BC_UNIMPLEMENTED;
 		}
diff --git a/drivers/firewire/core-iso.c b/drivers/firewire/core-iso.c
index 35e784cffc23..7e5c98840b80 100644
--- a/drivers/firewire/core-iso.c
+++ b/drivers/firewire/core-iso.c
@@ -308,7 +308,7 @@ static int manage_channel(struct fw_card *card, int irm_id, int generation,
 			if ((data[0] & bit) == (data[1] & bit))
 				continue;
 
-			/* 1394-1995 IRM, fall through to retry. */
+			/* fall through - to retry for 1394-1995 IRM */
 		default:
 			if (retry) {
 				retry--;
diff --git a/drivers/firewire/core-topology.c b/drivers/firewire/core-topology.c
index 7db234d3fbdd..82c67e900aad 100644
--- a/drivers/firewire/core-topology.c
+++ b/drivers/firewire/core-topology.c
@@ -67,6 +67,7 @@ static u32 *count_ports(u32 *sid, int *total_port_count, int *child_port_count)
 		switch (port_type) {
 		case SELFID_PORT_CHILD:
 			(*child_port_count)++;
+			/* fall through */
 		case SELFID_PORT_PARENT:
 		case SELFID_PORT_NCONN:
 			(*total_port_count)++;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH] firewire: mark expected switch fall-throughs
  2019-02-11 18:03 [PATCH] firewire: mark expected switch fall-throughs Gustavo A. R. Silva
@ 2019-03-14 10:00 ` Stefan Richter
  2019-03-14 18:42   ` Gustavo A. R. Silva
  0 siblings, 1 reply; 3+ messages in thread
From: Stefan Richter @ 2019-03-14 10:00 UTC (permalink / raw)
  To: Gustavo A. R. Silva
  Cc: linux1394-devel, linux-kernel, Kees Cook, Mathieu Malaterre

(added Cc: Mathieu Malaterre, who sent a patch which is a subset of this)

On Feb 11 Gustavo A. R. Silva wrote:
> In preparation to enabling -Wimplicit-fallthrough, mark switch
> cases where we are expecting to fall through.
> 
> This patch fixes the following warnings:
> 
> drivers/firewire/core-device.c: In function ‘set_broadcast_channel’:
> drivers/firewire/core-device.c:969:7: warning: this statement may fall through [-Wimplicit-fallthrough=]
>     if (data & cpu_to_be32(1 << 31)) {
>        ^
> drivers/firewire/core-device.c:974:3: note: here
>    case RCODE_ADDRESS_ERROR:
>    ^~~~
> drivers/firewire/core-iso.c: In function ‘manage_channel’:
> drivers/firewire/core-iso.c:308:7: warning: this statement may fall through [-Wimplicit-fallthrough=]
>     if ((data[0] & bit) == (data[1] & bit))
>        ^
> drivers/firewire/core-iso.c:312:3: note: here
>    default:
>    ^~~~~~~
> drivers/firewire/core-topology.c: In function ‘count_ports’:
> drivers/firewire/core-topology.c:69:23: warning: this statement may fall through [-Wimplicit-fallthrough=]
>     (*child_port_count)++;
>     ~~~~~~~~~~~~~~~~~~~^~
> drivers/firewire/core-topology.c:70:3: note: here
>    case SELFID_PORT_PARENT:
>    ^~~~
> 
> Warning level 3 was used: -Wimplicit-fallthrough=3
> 
> Notice that in some cases, the code comment is modified in
> accordance with what GCC is expecting to find.

OK, I looked up
https://gcc.gnu.org/onlinedocs/gcc-8.3.0/gcc/Warning-Options.html#index-Wimplicit-fallthrough
now.  I am loudly sighing and rolling my eyes...  Anyway; the last regex
listed at the Wimplicit-fallthrough=3 bullet point is obviously the one
you are wanting to match.

> This patch is part of the ongoing efforts to enable
> -Wimplicit-fallthrough.
> 
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> ---
>  drivers/firewire/core-device.c   | 2 +-
>  drivers/firewire/core-iso.c      | 2 +-
>  drivers/firewire/core-topology.c | 1 +
>  3 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/firewire/core-device.c b/drivers/firewire/core-device.c
> index 7c2eed76011e..0c86548fa4a7 100644
> --- a/drivers/firewire/core-device.c
> +++ b/drivers/firewire/core-device.c
> @@ -970,7 +970,7 @@ static void set_broadcast_channel(struct fw_device *device, int generation)
>  				device->bc_implemented = BC_IMPLEMENTED;
>  				break;
>  			}
> -			/* else fall through to case address error */
> +			/* else, fall through - to case address error */
>  		case RCODE_ADDRESS_ERROR:
>  			device->bc_implemented = BC_UNIMPLEMENTED;
>  		}
> diff --git a/drivers/firewire/core-iso.c b/drivers/firewire/core-iso.c
> index 35e784cffc23..7e5c98840b80 100644
> --- a/drivers/firewire/core-iso.c
> +++ b/drivers/firewire/core-iso.c
> @@ -308,7 +308,7 @@ static int manage_channel(struct fw_card *card, int irm_id, int generation,
>  			if ((data[0] & bit) == (data[1] & bit))
>  				continue;
>  
> -			/* 1394-1995 IRM, fall through to retry. */
> +			/* fall through - to retry for 1394-1995 IRM */

If you don't mind, I will apply your patch with a different wording of this
comment:

			/* fall through - It's an 1394-1995 IRM, retry. */

I'll mark my modification to your patch in the signed-off-by-tag.

>  		default:
>  			if (retry) {
>  				retry--;
> diff --git a/drivers/firewire/core-topology.c b/drivers/firewire/core-topology.c
> index 7db234d3fbdd..82c67e900aad 100644
> --- a/drivers/firewire/core-topology.c
> +++ b/drivers/firewire/core-topology.c
> @@ -67,6 +67,7 @@ static u32 *count_ports(u32 *sid, int *total_port_count, int *child_port_count)
>  		switch (port_type) {
>  		case SELFID_PORT_CHILD:
>  			(*child_port_count)++;
> +			/* fall through */
>  		case SELFID_PORT_PARENT:
>  		case SELFID_PORT_NCONN:
>  			(*total_port_count)++;

-- 
Stefan Richter
-======---== --== -===-
http://arcgraph.de/sr/

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH] firewire: mark expected switch fall-throughs
  2019-03-14 10:00 ` Stefan Richter
@ 2019-03-14 18:42   ` Gustavo A. R. Silva
  0 siblings, 0 replies; 3+ messages in thread
From: Gustavo A. R. Silva @ 2019-03-14 18:42 UTC (permalink / raw)
  To: Stefan Richter
  Cc: linux1394-devel, linux-kernel, Kees Cook, Mathieu Malaterre



On 3/14/19 5:00 AM, Stefan Richter wrote:

[..]

>> diff --git a/drivers/firewire/core-iso.c b/drivers/firewire/core-iso.c
>> index 35e784cffc23..7e5c98840b80 100644
>> --- a/drivers/firewire/core-iso.c
>> +++ b/drivers/firewire/core-iso.c
>> @@ -308,7 +308,7 @@ static int manage_channel(struct fw_card *card, int irm_id, int generation,
>>  			if ((data[0] & bit) == (data[1] & bit))
>>  				continue;
>>  
>> -			/* 1394-1995 IRM, fall through to retry. */
>> +			/* fall through - to retry for 1394-1995 IRM */
> 
> If you don't mind, I will apply your patch with a different wording of this
> comment:
> 
> 			/* fall through - It's an 1394-1995 IRM, retry. */
> 
> I'll mark my modification to your patch in the signed-off-by-tag.
> 

Sure. That's fine with me.

Thank you.
--
Gustavo


^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-03-14 18:42 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-11 18:03 [PATCH] firewire: mark expected switch fall-throughs Gustavo A. R. Silva
2019-03-14 10:00 ` Stefan Richter
2019-03-14 18:42   ` Gustavo A. R. Silva

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.