diff mbox series

[v2,3/9] ACPICA: ACPICA: fix -Wfallthrough

Message ID 20210122002357.370836-4-erik.kaneda@intel.com
State New
Headers show
Series [v2,1/9] ACPICA: Fix exception code class checks | expand

Commit Message

Erik Kaneda Jan. 22, 2021, 12:23 a.m. UTC
From: Nick Desaulniers <ndesaulniers@google.com>


ACPICA commit 4b9135f5774caa796ddf826448811e8e7f08ef2f

GCC 7.1 gained -Wimplicit-fallthrough to warn on implicit fallthrough,
as well as __attribute__((__fallthrough__)) and comments to explicitly
denote that cases of fallthrough were intentional. Clang also supports
this warning and statement attribute, but not the comment form.

Robert Moore provides additional context about the lint comments being
removed. They were for "an old version of PC-Lint, which we don't use
anymore." Drop those.

This will help us enable -Wimplicit-fallthrough throughout the Linux
kernel.

Suggested-by: Robert Moore <robert.moore@intel.com>
Reported-by: Jon Hunter <jonathanh@nvidia.com>

Link: https://github.com/acpica/acpica/commit/4b9135f5
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

Signed-off-by: Bob Moore <robert.moore@intel.com>

Signed-off-by: Erik Kaneda <erik.kaneda@intel.com>

---
 drivers/acpi/acpica/dscontrol.c |  2 +-
 drivers/acpi/acpica/dswexec.c   |  3 +--
 drivers/acpi/acpica/dswload.c   |  2 +-
 drivers/acpi/acpica/dswload2.c  |  2 +-
 drivers/acpi/acpica/exfldio.c   |  2 +-
 drivers/acpi/acpica/exresop.c   |  4 ++--
 drivers/acpi/acpica/exstore.c   |  4 ++--
 drivers/acpi/acpica/hwgpe.c     |  2 +-
 drivers/acpi/acpica/utdelete.c  |  2 +-
 include/acpi/actypes.h          |  6 ++++++
 include/acpi/platform/acgcc.h   | 15 +++++++++++++++
 11 files changed, 32 insertions(+), 12 deletions(-)

-- 
2.29.2

Comments

Guenter Roeck Jan. 23, 2021, 3:21 p.m. UTC | #1
On Thu, Jan 21, 2021 at 04:23:51PM -0800, Erik Kaneda wrote:
> From: Nick Desaulniers <ndesaulniers@google.com>

> 

> ACPICA commit 4b9135f5774caa796ddf826448811e8e7f08ef2f

> 

> GCC 7.1 gained -Wimplicit-fallthrough to warn on implicit fallthrough,

> as well as __attribute__((__fallthrough__)) and comments to explicitly

> denote that cases of fallthrough were intentional. Clang also supports

> this warning and statement attribute, but not the comment form.

> 

> Robert Moore provides additional context about the lint comments being

> removed. They were for "an old version of PC-Lint, which we don't use

> anymore." Drop those.

> 

> This will help us enable -Wimplicit-fallthrough throughout the Linux

> kernel.

> 

> Suggested-by: Robert Moore <robert.moore@intel.com>

> Reported-by: Jon Hunter <jonathanh@nvidia.com>

> 

> Link: https://github.com/acpica/acpica/commit/4b9135f5

> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

> Signed-off-by: Bob Moore <robert.moore@intel.com>

> Signed-off-by: Erik Kaneda <erik.kaneda@intel.com>


With gcc 6.5 (and presumably each gcc version older than 7.1),
this patch results in:

drivers/acpi/acpica/dscontrol.c: In function ‘acpi_ds_exec_begin_control_op’:
drivers/acpi/acpica/dscontrol.c:65:3: error: ‘ACPI_FALLTHROUGH’ undeclared

and similar errors for other files.

Guenter

> ---

>  drivers/acpi/acpica/dscontrol.c |  2 +-

>  drivers/acpi/acpica/dswexec.c   |  3 +--

>  drivers/acpi/acpica/dswload.c   |  2 +-

>  drivers/acpi/acpica/dswload2.c  |  2 +-

>  drivers/acpi/acpica/exfldio.c   |  2 +-

>  drivers/acpi/acpica/exresop.c   |  4 ++--

>  drivers/acpi/acpica/exstore.c   |  4 ++--

>  drivers/acpi/acpica/hwgpe.c     |  2 +-

>  drivers/acpi/acpica/utdelete.c  |  2 +-

>  include/acpi/actypes.h          |  6 ++++++

>  include/acpi/platform/acgcc.h   | 15 +++++++++++++++

>  11 files changed, 32 insertions(+), 12 deletions(-)

> 

> -- 

> 2.29.2

> 

> diff --git a/drivers/acpi/acpica/dscontrol.c b/drivers/acpi/acpica/dscontrol.c

> index 4b5b6e859f62..b58ffc7acdb9 100644

> --- a/drivers/acpi/acpica/dscontrol.c

> +++ b/drivers/acpi/acpica/dscontrol.c

> @@ -62,7 +62,7 @@ acpi_ds_exec_begin_control_op(struct acpi_walk_state *walk_state,

>  			}

>  		}

>  

> -		/*lint -fallthrough */

> +		ACPI_FALLTHROUGH;

>  

>  	case AML_IF_OP:

>  		/*

> diff --git a/drivers/acpi/acpica/dswexec.c b/drivers/acpi/acpica/dswexec.c

> index 1d4f8c81028c..4a9799246fae 100644

> --- a/drivers/acpi/acpica/dswexec.c

> +++ b/drivers/acpi/acpica/dswexec.c

> @@ -598,8 +598,7 @@ acpi_status acpi_ds_exec_end_op(struct acpi_walk_state *walk_state)

>  					break;

>  				}

>  

> -				/* Fall through */

> -				/*lint -fallthrough */

> +				ACPI_FALLTHROUGH;

>  

>  			case AML_INT_EVAL_SUBTREE_OP:

>  

> diff --git a/drivers/acpi/acpica/dswload.c b/drivers/acpi/acpica/dswload.c

> index 27069325b6de..dd97c86f8e41 100644

> --- a/drivers/acpi/acpica/dswload.c

> +++ b/drivers/acpi/acpica/dswload.c

> @@ -224,7 +224,7 @@ acpi_ds_load1_begin_op(struct acpi_walk_state *walk_state,

>  				break;

>  			}

>  

> -			/*lint -fallthrough */

> +			ACPI_FALLTHROUGH;

>  

>  		default:

>  

> diff --git a/drivers/acpi/acpica/dswload2.c b/drivers/acpi/acpica/dswload2.c

> index edadbe146506..d9a3dfca7555 100644

> --- a/drivers/acpi/acpica/dswload2.c

> +++ b/drivers/acpi/acpica/dswload2.c

> @@ -214,7 +214,7 @@ acpi_ds_load2_begin_op(struct acpi_walk_state *walk_state,

>  				break;

>  			}

>  

> -			/*lint -fallthrough */

> +			ACPI_FALLTHROUGH;

>  

>  		default:

>  

> diff --git a/drivers/acpi/acpica/exfldio.c b/drivers/acpi/acpica/exfldio.c

> index ade35ff1c7ba..cde24e0fa6a8 100644

> --- a/drivers/acpi/acpica/exfldio.c

> +++ b/drivers/acpi/acpica/exfldio.c

> @@ -434,7 +434,7 @@ acpi_ex_field_datum_io(union acpi_operand_object *obj_desc,

>  		 * region_field case and write the datum to the Operation Region

>  		 */

>  

> -		/*lint -fallthrough */

> +		ACPI_FALLTHROUGH;

>  

>  	case ACPI_TYPE_LOCAL_REGION_FIELD:

>  		/*

> diff --git a/drivers/acpi/acpica/exresop.c b/drivers/acpi/acpica/exresop.c

> index 4d1b22971d58..4a0f8b8bfe62 100644

> --- a/drivers/acpi/acpica/exresop.c

> +++ b/drivers/acpi/acpica/exresop.c

> @@ -198,7 +198,7 @@ acpi_ex_resolve_operands(u16 opcode,

>  

>  					target_op = AML_DEBUG_OP;

>  

> -					/*lint -fallthrough */

> +					ACPI_FALLTHROUGH;

>  

>  				case ACPI_REFCLASS_ARG:

>  				case ACPI_REFCLASS_LOCAL:

> @@ -264,7 +264,7 @@ acpi_ex_resolve_operands(u16 opcode,

>  			 * Else not a string - fall through to the normal Reference

>  			 * case below

>  			 */

> -			/*lint -fallthrough */

> +			ACPI_FALLTHROUGH;

>  

>  		case ARGI_REFERENCE:	/* References: */

>  		case ARGI_INTEGER_REF:

> diff --git a/drivers/acpi/acpica/exstore.c b/drivers/acpi/acpica/exstore.c

> index 3adc0a29d890..8fe33051275d 100644

> --- a/drivers/acpi/acpica/exstore.c

> +++ b/drivers/acpi/acpica/exstore.c

> @@ -96,7 +96,7 @@ acpi_ex_store(union acpi_operand_object *source_desc,

>  			return_ACPI_STATUS(AE_OK);

>  		}

>  

> -		/*lint -fallthrough */

> +		ACPI_FALLTHROUGH;

>  

>  	default:

>  

> @@ -422,7 +422,7 @@ acpi_ex_store_object_to_node(union acpi_operand_object *source_desc,

>  				break;

>  			}

>  

> -			/* Fallthrough */

> +			ACPI_FALLTHROUGH;

>  

>  		case ACPI_TYPE_DEVICE:

>  		case ACPI_TYPE_EVENT:

> diff --git a/drivers/acpi/acpica/hwgpe.c b/drivers/acpi/acpica/hwgpe.c

> index b13a4ed5bc63..0c84300e915c 100644

> --- a/drivers/acpi/acpica/hwgpe.c

> +++ b/drivers/acpi/acpica/hwgpe.c

> @@ -167,7 +167,7 @@ acpi_hw_low_set_gpe(struct acpi_gpe_event_info *gpe_event_info, u32 action)

>  			return (AE_BAD_PARAMETER);

>  		}

>  

> -		/*lint -fallthrough */

> +		ACPI_FALLTHROUGH;

>  

>  	case ACPI_GPE_ENABLE:

>  

> diff --git a/drivers/acpi/acpica/utdelete.c b/drivers/acpi/acpica/utdelete.c

> index 4c0d4e434196..624a26794d55 100644

> --- a/drivers/acpi/acpica/utdelete.c

> +++ b/drivers/acpi/acpica/utdelete.c

> @@ -112,7 +112,7 @@ static void acpi_ut_delete_internal_obj(union acpi_operand_object *object)

>  						       gpe_block);

>  		}

>  

> -		/*lint -fallthrough */

> +		ACPI_FALLTHROUGH;

>  

>  	case ACPI_TYPE_PROCESSOR:

>  	case ACPI_TYPE_THERMAL:

> diff --git a/include/acpi/actypes.h b/include/acpi/actypes.h

> index 647cb11d0a0a..2a32593691bc 100644

> --- a/include/acpi/actypes.h

> +++ b/include/acpi/actypes.h

> @@ -1286,4 +1286,10 @@ typedef enum {

>  

>  #define ACPI_OPT_END                    -1

>  

> +/* Definitions for explicit fallthrough */

> +

> +#ifndef ACPI_FALLTHROUGH

> +#define ACPI_FALLTHROUGH do {} while(0)

> +#endif

> +

>  #endif				/* __ACTYPES_H__ */

> diff --git a/include/acpi/platform/acgcc.h b/include/acpi/platform/acgcc.h

> index 7d63d03cf507..91f7a02c798a 100644

> --- a/include/acpi/platform/acgcc.h

> +++ b/include/acpi/platform/acgcc.h

> @@ -54,4 +54,19 @@ typedef __builtin_va_list va_list;

>  

>  #define ACPI_USE_NATIVE_MATH64

>  

> +/* GCC did not support __has_attribute until 5.1. */

> +

> +#ifndef __has_attribute

> +#define __has_attribute(x) 0

> +#endif

> +

> +/*

> + * Explictly mark intentional explicit fallthrough to silence

> + * -Wimplicit-fallthrough in GCC 7.1+.

> + */

> +

> +#if __has_attribute(__fallthrough__)

> +#define ACPI_FALLTHROUGH __attribute__((__fallthrough__))

> +#endif

> +

>  #endif				/* __ACGCC_H__ */
Guenter Roeck Jan. 23, 2021, 3:25 p.m. UTC | #2
On Sat, Jan 23, 2021 at 07:21:04AM -0800, Guenter Roeck wrote:
> On Thu, Jan 21, 2021 at 04:23:51PM -0800, Erik Kaneda wrote:

> > From: Nick Desaulniers <ndesaulniers@google.com>

> > 

> > ACPICA commit 4b9135f5774caa796ddf826448811e8e7f08ef2f

> > 

> > GCC 7.1 gained -Wimplicit-fallthrough to warn on implicit fallthrough,

> > as well as __attribute__((__fallthrough__)) and comments to explicitly

> > denote that cases of fallthrough were intentional. Clang also supports

> > this warning and statement attribute, but not the comment form.

> > 

> > Robert Moore provides additional context about the lint comments being

> > removed. They were for "an old version of PC-Lint, which we don't use

> > anymore." Drop those.

> > 

> > This will help us enable -Wimplicit-fallthrough throughout the Linux

> > kernel.

> > 

> > Suggested-by: Robert Moore <robert.moore@intel.com>

> > Reported-by: Jon Hunter <jonathanh@nvidia.com>

> > 

> > Link: https://github.com/acpica/acpica/commit/4b9135f5

> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

> > Signed-off-by: Bob Moore <robert.moore@intel.com>

> > Signed-off-by: Erik Kaneda <erik.kaneda@intel.com>

> 

> With gcc 6.5 (and presumably each gcc version older than 7.1),

> this patch results in:

> 

> drivers/acpi/acpica/dscontrol.c: In function ‘acpi_ds_exec_begin_control_op’:

> drivers/acpi/acpica/dscontrol.c:65:3: error: ‘ACPI_FALLTHROUGH’ undeclared

> 

> and similar errors for other files.

> 


Wait, this differs from the patch in -next.

> > +#ifndef ACPI_FALLTHROUGH

> > +#define ACPI_FALLTHROUGH do {} while(0)


This line is missing in -next.

v2 doesn't say what changed in v2. Is that the change ?

Guenter
Rafael J. Wysocki Jan. 23, 2021, 8:11 p.m. UTC | #3
On Sat, Jan 23, 2021 at 4:25 PM Guenter Roeck <linux@roeck-us.net> wrote:
>

> On Sat, Jan 23, 2021 at 07:21:04AM -0800, Guenter Roeck wrote:

> > On Thu, Jan 21, 2021 at 04:23:51PM -0800, Erik Kaneda wrote:

> > > From: Nick Desaulniers <ndesaulniers@google.com>

> > >

> > > ACPICA commit 4b9135f5774caa796ddf826448811e8e7f08ef2f

> > >

> > > GCC 7.1 gained -Wimplicit-fallthrough to warn on implicit fallthrough,

> > > as well as __attribute__((__fallthrough__)) and comments to explicitly

> > > denote that cases of fallthrough were intentional. Clang also supports

> > > this warning and statement attribute, but not the comment form.

> > >

> > > Robert Moore provides additional context about the lint comments being

> > > removed. They were for "an old version of PC-Lint, which we don't use

> > > anymore." Drop those.

> > >

> > > This will help us enable -Wimplicit-fallthrough throughout the Linux

> > > kernel.

> > >

> > > Suggested-by: Robert Moore <robert.moore@intel.com>

> > > Reported-by: Jon Hunter <jonathanh@nvidia.com>

> > >

> > > Link: https://github.com/acpica/acpica/commit/4b9135f5

> > > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

> > > Signed-off-by: Bob Moore <robert.moore@intel.com>

> > > Signed-off-by: Erik Kaneda <erik.kaneda@intel.com>

> >

> > With gcc 6.5 (and presumably each gcc version older than 7.1),

> > this patch results in:

> >

> > drivers/acpi/acpica/dscontrol.c: In function ‘acpi_ds_exec_begin_control_op’:

> > drivers/acpi/acpica/dscontrol.c:65:3: error: ‘ACPI_FALLTHROUGH’ undeclared

> >

> > and similar errors for other files.

> >

>

> Wait, this differs from the patch in -next.

>

> > > +#ifndef ACPI_FALLTHROUGH

> > > +#define ACPI_FALLTHROUGH do {} while(0)

>

> This line is missing in -next.

>

> v2 doesn't say what changed in v2. Is that the change ?


Yes, it is.

It should appear in -next on Monday.
diff mbox series

Patch

diff --git a/drivers/acpi/acpica/dscontrol.c b/drivers/acpi/acpica/dscontrol.c
index 4b5b6e859f62..b58ffc7acdb9 100644
--- a/drivers/acpi/acpica/dscontrol.c
+++ b/drivers/acpi/acpica/dscontrol.c
@@ -62,7 +62,7 @@  acpi_ds_exec_begin_control_op(struct acpi_walk_state *walk_state,
 			}
 		}
 
-		/*lint -fallthrough */
+		ACPI_FALLTHROUGH;
 
 	case AML_IF_OP:
 		/*
diff --git a/drivers/acpi/acpica/dswexec.c b/drivers/acpi/acpica/dswexec.c
index 1d4f8c81028c..4a9799246fae 100644
--- a/drivers/acpi/acpica/dswexec.c
+++ b/drivers/acpi/acpica/dswexec.c
@@ -598,8 +598,7 @@  acpi_status acpi_ds_exec_end_op(struct acpi_walk_state *walk_state)
 					break;
 				}
 
-				/* Fall through */
-				/*lint -fallthrough */
+				ACPI_FALLTHROUGH;
 
 			case AML_INT_EVAL_SUBTREE_OP:
 
diff --git a/drivers/acpi/acpica/dswload.c b/drivers/acpi/acpica/dswload.c
index 27069325b6de..dd97c86f8e41 100644
--- a/drivers/acpi/acpica/dswload.c
+++ b/drivers/acpi/acpica/dswload.c
@@ -224,7 +224,7 @@  acpi_ds_load1_begin_op(struct acpi_walk_state *walk_state,
 				break;
 			}
 
-			/*lint -fallthrough */
+			ACPI_FALLTHROUGH;
 
 		default:
 
diff --git a/drivers/acpi/acpica/dswload2.c b/drivers/acpi/acpica/dswload2.c
index edadbe146506..d9a3dfca7555 100644
--- a/drivers/acpi/acpica/dswload2.c
+++ b/drivers/acpi/acpica/dswload2.c
@@ -214,7 +214,7 @@  acpi_ds_load2_begin_op(struct acpi_walk_state *walk_state,
 				break;
 			}
 
-			/*lint -fallthrough */
+			ACPI_FALLTHROUGH;
 
 		default:
 
diff --git a/drivers/acpi/acpica/exfldio.c b/drivers/acpi/acpica/exfldio.c
index ade35ff1c7ba..cde24e0fa6a8 100644
--- a/drivers/acpi/acpica/exfldio.c
+++ b/drivers/acpi/acpica/exfldio.c
@@ -434,7 +434,7 @@  acpi_ex_field_datum_io(union acpi_operand_object *obj_desc,
 		 * region_field case and write the datum to the Operation Region
 		 */
 
-		/*lint -fallthrough */
+		ACPI_FALLTHROUGH;
 
 	case ACPI_TYPE_LOCAL_REGION_FIELD:
 		/*
diff --git a/drivers/acpi/acpica/exresop.c b/drivers/acpi/acpica/exresop.c
index 4d1b22971d58..4a0f8b8bfe62 100644
--- a/drivers/acpi/acpica/exresop.c
+++ b/drivers/acpi/acpica/exresop.c
@@ -198,7 +198,7 @@  acpi_ex_resolve_operands(u16 opcode,
 
 					target_op = AML_DEBUG_OP;
 
-					/*lint -fallthrough */
+					ACPI_FALLTHROUGH;
 
 				case ACPI_REFCLASS_ARG:
 				case ACPI_REFCLASS_LOCAL:
@@ -264,7 +264,7 @@  acpi_ex_resolve_operands(u16 opcode,
 			 * Else not a string - fall through to the normal Reference
 			 * case below
 			 */
-			/*lint -fallthrough */
+			ACPI_FALLTHROUGH;
 
 		case ARGI_REFERENCE:	/* References: */
 		case ARGI_INTEGER_REF:
diff --git a/drivers/acpi/acpica/exstore.c b/drivers/acpi/acpica/exstore.c
index 3adc0a29d890..8fe33051275d 100644
--- a/drivers/acpi/acpica/exstore.c
+++ b/drivers/acpi/acpica/exstore.c
@@ -96,7 +96,7 @@  acpi_ex_store(union acpi_operand_object *source_desc,
 			return_ACPI_STATUS(AE_OK);
 		}
 
-		/*lint -fallthrough */
+		ACPI_FALLTHROUGH;
 
 	default:
 
@@ -422,7 +422,7 @@  acpi_ex_store_object_to_node(union acpi_operand_object *source_desc,
 				break;
 			}
 
-			/* Fallthrough */
+			ACPI_FALLTHROUGH;
 
 		case ACPI_TYPE_DEVICE:
 		case ACPI_TYPE_EVENT:
diff --git a/drivers/acpi/acpica/hwgpe.c b/drivers/acpi/acpica/hwgpe.c
index b13a4ed5bc63..0c84300e915c 100644
--- a/drivers/acpi/acpica/hwgpe.c
+++ b/drivers/acpi/acpica/hwgpe.c
@@ -167,7 +167,7 @@  acpi_hw_low_set_gpe(struct acpi_gpe_event_info *gpe_event_info, u32 action)
 			return (AE_BAD_PARAMETER);
 		}
 
-		/*lint -fallthrough */
+		ACPI_FALLTHROUGH;
 
 	case ACPI_GPE_ENABLE:
 
diff --git a/drivers/acpi/acpica/utdelete.c b/drivers/acpi/acpica/utdelete.c
index 4c0d4e434196..624a26794d55 100644
--- a/drivers/acpi/acpica/utdelete.c
+++ b/drivers/acpi/acpica/utdelete.c
@@ -112,7 +112,7 @@  static void acpi_ut_delete_internal_obj(union acpi_operand_object *object)
 						       gpe_block);
 		}
 
-		/*lint -fallthrough */
+		ACPI_FALLTHROUGH;
 
 	case ACPI_TYPE_PROCESSOR:
 	case ACPI_TYPE_THERMAL:
diff --git a/include/acpi/actypes.h b/include/acpi/actypes.h
index 647cb11d0a0a..2a32593691bc 100644
--- a/include/acpi/actypes.h
+++ b/include/acpi/actypes.h
@@ -1286,4 +1286,10 @@  typedef enum {
 
 #define ACPI_OPT_END                    -1
 
+/* Definitions for explicit fallthrough */
+
+#ifndef ACPI_FALLTHROUGH
+#define ACPI_FALLTHROUGH do {} while(0)
+#endif
+
 #endif				/* __ACTYPES_H__ */
diff --git a/include/acpi/platform/acgcc.h b/include/acpi/platform/acgcc.h
index 7d63d03cf507..91f7a02c798a 100644
--- a/include/acpi/platform/acgcc.h
+++ b/include/acpi/platform/acgcc.h
@@ -54,4 +54,19 @@  typedef __builtin_va_list va_list;
 
 #define ACPI_USE_NATIVE_MATH64
 
+/* GCC did not support __has_attribute until 5.1. */
+
+#ifndef __has_attribute
+#define __has_attribute(x) 0
+#endif
+
+/*
+ * Explictly mark intentional explicit fallthrough to silence
+ * -Wimplicit-fallthrough in GCC 7.1+.
+ */
+
+#if __has_attribute(__fallthrough__)
+#define ACPI_FALLTHROUGH __attribute__((__fallthrough__))
+#endif
+
 #endif				/* __ACGCC_H__ */