The "fallthrough" pseudo-keyword was added as a portable way to denote
intentional fallthrough. This code seemed to be using a mix of
fallthrough comments that GCC recognizes, and some kind of lint marker.
I'm guessing that linter hasn't been run in a while from the mixed use
of the marker vs comments.
Signed-off-by: Nick Desaulniers <[email protected]>
---
drivers/acpi/acpica/dscontrol.c | 3 +--
drivers/acpi/acpica/dswexec.c | 4 +---
drivers/acpi/acpica/dswload.c | 3 +--
drivers/acpi/acpica/dswload2.c | 3 +--
drivers/acpi/acpica/exfldio.c | 3 +--
drivers/acpi/acpica/exresop.c | 5 ++---
drivers/acpi/acpica/exstore.c | 6 ++----
drivers/acpi/acpica/hwgpe.c | 3 +--
drivers/acpi/acpica/utdelete.c | 3 +--
drivers/acpi/acpica/utprint.c | 2 +-
10 files changed, 12 insertions(+), 23 deletions(-)
diff --git a/drivers/acpi/acpica/dscontrol.c b/drivers/acpi/acpica/dscontrol.c
index 4b5b6e859f62..1e75e5fbfd19 100644
--- a/drivers/acpi/acpica/dscontrol.c
+++ b/drivers/acpi/acpica/dscontrol.c
@@ -61,8 +61,7 @@ acpi_ds_exec_begin_control_op(struct acpi_walk_state *walk_state,
break;
}
}
-
- /*lint -fallthrough */
+ fallthrough;
case AML_IF_OP:
/*
diff --git a/drivers/acpi/acpica/dswexec.c b/drivers/acpi/acpica/dswexec.c
index 1d4f8c81028c..e8c32d4fe55f 100644
--- a/drivers/acpi/acpica/dswexec.c
+++ b/drivers/acpi/acpica/dswexec.c
@@ -597,9 +597,7 @@ acpi_status acpi_ds_exec_end_op(struct acpi_walk_state *walk_state)
if (ACPI_FAILURE(status)) {
break;
}
-
- /* Fall through */
- /*lint -fallthrough */
+ fallthrough;
case AML_INT_EVAL_SUBTREE_OP:
diff --git a/drivers/acpi/acpica/dswload.c b/drivers/acpi/acpica/dswload.c
index 27069325b6de..afc663c3742d 100644
--- a/drivers/acpi/acpica/dswload.c
+++ b/drivers/acpi/acpica/dswload.c
@@ -223,8 +223,7 @@ acpi_ds_load1_begin_op(struct acpi_walk_state *walk_state,
parse_flags & ACPI_PARSE_MODULE_LEVEL)) {
break;
}
-
- /*lint -fallthrough */
+ fallthrough;
default:
diff --git a/drivers/acpi/acpica/dswload2.c b/drivers/acpi/acpica/dswload2.c
index edadbe146506..1b794b6ba072 100644
--- a/drivers/acpi/acpica/dswload2.c
+++ b/drivers/acpi/acpica/dswload2.c
@@ -213,8 +213,7 @@ acpi_ds_load2_begin_op(struct acpi_walk_state *walk_state,
parse_flags & ACPI_PARSE_MODULE_LEVEL)) {
break;
}
-
- /*lint -fallthrough */
+ fallthrough;
default:
diff --git a/drivers/acpi/acpica/exfldio.c b/drivers/acpi/acpica/exfldio.c
index ade35ff1c7ba..9d1cabe0fed9 100644
--- a/drivers/acpi/acpica/exfldio.c
+++ b/drivers/acpi/acpica/exfldio.c
@@ -433,8 +433,7 @@ acpi_ex_field_datum_io(union acpi_operand_object *obj_desc,
* Now that the Bank has been selected, fall through to the
* region_field case and write the datum to the Operation Region
*/
-
- /*lint -fallthrough */
+ fallthrough;
case ACPI_TYPE_LOCAL_REGION_FIELD:
/*
diff --git a/drivers/acpi/acpica/exresop.c b/drivers/acpi/acpica/exresop.c
index 4d1b22971d58..df48faa9a551 100644
--- a/drivers/acpi/acpica/exresop.c
+++ b/drivers/acpi/acpica/exresop.c
@@ -197,8 +197,7 @@ acpi_ex_resolve_operands(u16 opcode,
case ACPI_REFCLASS_DEBUG:
target_op = AML_DEBUG_OP;
-
- /*lint -fallthrough */
+ fallthrough;
case ACPI_REFCLASS_ARG:
case ACPI_REFCLASS_LOCAL:
@@ -264,7 +263,7 @@ acpi_ex_resolve_operands(u16 opcode,
* Else not a string - fall through to the normal Reference
* case below
*/
- /*lint -fallthrough */
+ 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..2067baa7c120 100644
--- a/drivers/acpi/acpica/exstore.c
+++ b/drivers/acpi/acpica/exstore.c
@@ -95,8 +95,7 @@ acpi_ex_store(union acpi_operand_object *source_desc,
if (dest_desc->common.flags & AOPOBJ_AML_CONSTANT) {
return_ACPI_STATUS(AE_OK);
}
-
- /*lint -fallthrough */
+ fallthrough;
default:
@@ -421,8 +420,7 @@ acpi_ex_store_object_to_node(union acpi_operand_object *source_desc,
}
break;
}
-
- /* Fallthrough */
+ 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..fbfad80c8a53 100644
--- a/drivers/acpi/acpica/hwgpe.c
+++ b/drivers/acpi/acpica/hwgpe.c
@@ -166,8 +166,7 @@ acpi_hw_low_set_gpe(struct acpi_gpe_event_info *gpe_event_info, u32 action)
if (!(register_bit & gpe_register_info->enable_mask)) {
return (AE_BAD_PARAMETER);
}
-
- /*lint -fallthrough */
+ fallthrough;
case ACPI_GPE_ENABLE:
diff --git a/drivers/acpi/acpica/utdelete.c b/drivers/acpi/acpica/utdelete.c
index 4c0d4e434196..8076e7947585 100644
--- a/drivers/acpi/acpica/utdelete.c
+++ b/drivers/acpi/acpica/utdelete.c
@@ -111,8 +111,7 @@ static void acpi_ut_delete_internal_obj(union acpi_operand_object *object)
(void)acpi_ev_delete_gpe_block(object->device.
gpe_block);
}
-
- /*lint -fallthrough */
+ fallthrough;
case ACPI_TYPE_PROCESSOR:
case ACPI_TYPE_THERMAL:
diff --git a/drivers/acpi/acpica/utprint.c b/drivers/acpi/acpica/utprint.c
index 681c11f4af4e..f7e43baf5ff2 100644
--- a/drivers/acpi/acpica/utprint.c
+++ b/drivers/acpi/acpica/utprint.c
@@ -475,7 +475,7 @@ int vsnprintf(char *string, acpi_size size, const char *format, va_list args)
case 'X':
type |= ACPI_FORMAT_UPPER;
- /* FALLTHROUGH */
+ fallthrough;
case 'x':
--
2.29.2.222.g5d2a92d10f8-goog
Yes, but: isn't the "fallthrough" keyword compiler-specific? That is the problem for us.
Bob
-----Original Message-----
From: ndesaulniers via sendgmr <[email protected]> On Behalf Of Nick Desaulniers
Sent: Tuesday, November 10, 2020 6:12 PM
To: Moore, Robert <[email protected]>; Kaneda, Erik <[email protected]>; Wysocki, Rafael J <[email protected]>; Gustavo A . R . Silva <[email protected]>
Cc: [email protected]; Nick Desaulniers <[email protected]>; Len Brown <[email protected]>; [email protected]; [email protected]; [email protected]
Subject: [PATCH] ACPICA: fix -Wfallthrough
The "fallthrough" pseudo-keyword was added as a portable way to denote intentional fallthrough. This code seemed to be using a mix of fallthrough comments that GCC recognizes, and some kind of lint marker.
I'm guessing that linter hasn't been run in a while from the mixed use of the marker vs comments.
Signed-off-by: Nick Desaulniers <[email protected]>
---
drivers/acpi/acpica/dscontrol.c | 3 +--
drivers/acpi/acpica/dswexec.c | 4 +---
drivers/acpi/acpica/dswload.c | 3 +--
drivers/acpi/acpica/dswload2.c | 3 +--
drivers/acpi/acpica/exfldio.c | 3 +--
drivers/acpi/acpica/exresop.c | 5 ++---
drivers/acpi/acpica/exstore.c | 6 ++----
drivers/acpi/acpica/hwgpe.c | 3 +--
drivers/acpi/acpica/utdelete.c | 3 +--
drivers/acpi/acpica/utprint.c | 2 +-
10 files changed, 12 insertions(+), 23 deletions(-)
diff --git a/drivers/acpi/acpica/dscontrol.c b/drivers/acpi/acpica/dscontrol.c index 4b5b6e859f62..1e75e5fbfd19 100644
--- a/drivers/acpi/acpica/dscontrol.c
+++ b/drivers/acpi/acpica/dscontrol.c
@@ -61,8 +61,7 @@ acpi_ds_exec_begin_control_op(struct acpi_walk_state *walk_state,
break;
}
}
-
- /*lint -fallthrough */
+ fallthrough;
case AML_IF_OP:
/*
diff --git a/drivers/acpi/acpica/dswexec.c b/drivers/acpi/acpica/dswexec.c index 1d4f8c81028c..e8c32d4fe55f 100644
--- a/drivers/acpi/acpica/dswexec.c
+++ b/drivers/acpi/acpica/dswexec.c
@@ -597,9 +597,7 @@ acpi_status acpi_ds_exec_end_op(struct acpi_walk_state *walk_state)
if (ACPI_FAILURE(status)) {
break;
}
-
- /* Fall through */
- /*lint -fallthrough */
+ fallthrough;
case AML_INT_EVAL_SUBTREE_OP:
diff --git a/drivers/acpi/acpica/dswload.c b/drivers/acpi/acpica/dswload.c index 27069325b6de..afc663c3742d 100644
--- a/drivers/acpi/acpica/dswload.c
+++ b/drivers/acpi/acpica/dswload.c
@@ -223,8 +223,7 @@ acpi_ds_load1_begin_op(struct acpi_walk_state *walk_state,
parse_flags & ACPI_PARSE_MODULE_LEVEL)) {
break;
}
-
- /*lint -fallthrough */
+ fallthrough;
default:
diff --git a/drivers/acpi/acpica/dswload2.c b/drivers/acpi/acpica/dswload2.c index edadbe146506..1b794b6ba072 100644
--- a/drivers/acpi/acpica/dswload2.c
+++ b/drivers/acpi/acpica/dswload2.c
@@ -213,8 +213,7 @@ acpi_ds_load2_begin_op(struct acpi_walk_state *walk_state,
parse_flags & ACPI_PARSE_MODULE_LEVEL)) {
break;
}
-
- /*lint -fallthrough */
+ fallthrough;
default:
diff --git a/drivers/acpi/acpica/exfldio.c b/drivers/acpi/acpica/exfldio.c index ade35ff1c7ba..9d1cabe0fed9 100644
--- a/drivers/acpi/acpica/exfldio.c
+++ b/drivers/acpi/acpica/exfldio.c
@@ -433,8 +433,7 @@ acpi_ex_field_datum_io(union acpi_operand_object *obj_desc,
* Now that the Bank has been selected, fall through to the
* region_field case and write the datum to the Operation Region
*/
-
- /*lint -fallthrough */
+ fallthrough;
case ACPI_TYPE_LOCAL_REGION_FIELD:
/*
diff --git a/drivers/acpi/acpica/exresop.c b/drivers/acpi/acpica/exresop.c index 4d1b22971d58..df48faa9a551 100644
--- a/drivers/acpi/acpica/exresop.c
+++ b/drivers/acpi/acpica/exresop.c
@@ -197,8 +197,7 @@ acpi_ex_resolve_operands(u16 opcode,
case ACPI_REFCLASS_DEBUG:
target_op = AML_DEBUG_OP;
-
- /*lint -fallthrough */
+ fallthrough;
case ACPI_REFCLASS_ARG:
case ACPI_REFCLASS_LOCAL:
@@ -264,7 +263,7 @@ acpi_ex_resolve_operands(u16 opcode,
* Else not a string - fall through to the normal Reference
* case below
*/
- /*lint -fallthrough */
+ 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..2067baa7c120 100644
--- a/drivers/acpi/acpica/exstore.c
+++ b/drivers/acpi/acpica/exstore.c
@@ -95,8 +95,7 @@ acpi_ex_store(union acpi_operand_object *source_desc,
if (dest_desc->common.flags & AOPOBJ_AML_CONSTANT) {
return_ACPI_STATUS(AE_OK);
}
-
- /*lint -fallthrough */
+ fallthrough;
default:
@@ -421,8 +420,7 @@ acpi_ex_store_object_to_node(union acpi_operand_object *source_desc,
}
break;
}
-
- /* Fallthrough */
+ 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..fbfad80c8a53 100644
--- a/drivers/acpi/acpica/hwgpe.c
+++ b/drivers/acpi/acpica/hwgpe.c
@@ -166,8 +166,7 @@ acpi_hw_low_set_gpe(struct acpi_gpe_event_info *gpe_event_info, u32 action)
if (!(register_bit & gpe_register_info->enable_mask)) {
return (AE_BAD_PARAMETER);
}
-
- /*lint -fallthrough */
+ fallthrough;
case ACPI_GPE_ENABLE:
diff --git a/drivers/acpi/acpica/utdelete.c b/drivers/acpi/acpica/utdelete.c index 4c0d4e434196..8076e7947585 100644
--- a/drivers/acpi/acpica/utdelete.c
+++ b/drivers/acpi/acpica/utdelete.c
@@ -111,8 +111,7 @@ static void acpi_ut_delete_internal_obj(union acpi_operand_object *object)
(void)acpi_ev_delete_gpe_block(object->device.
gpe_block);
}
-
- /*lint -fallthrough */
+ fallthrough;
case ACPI_TYPE_PROCESSOR:
case ACPI_TYPE_THERMAL:
diff --git a/drivers/acpi/acpica/utprint.c b/drivers/acpi/acpica/utprint.c index 681c11f4af4e..f7e43baf5ff2 100644
--- a/drivers/acpi/acpica/utprint.c
+++ b/drivers/acpi/acpica/utprint.c
@@ -475,7 +475,7 @@ int vsnprintf(char *string, acpi_size size, const char *format, va_list args)
case 'X':
type |= ACPI_FORMAT_UPPER;
- /* FALLTHROUGH */
+ fallthrough;
case 'x':
--
2.29.2.222.g5d2a92d10f8-goog
On Wed, Nov 11, 2020 at 7:15 AM Moore, Robert <[email protected]> wrote:
>
> Yes, but: isn't the "fallthrough" keyword compiler-specific? That is the problem for us.
It's not a keyword.
It's a preprocessor macro that expands to
__attribute__((__fallthrough__)) for compilers that support it. For
compilers that do not, it expands to nothing. Both GCC 7+ and Clang
support this attribute. Which other compilers that support
-Wimplicit-fallthrough do you care to support?
> Bob
>
>
> -----Original Message-----
> From: ndesaulniers via sendgmr <[email protected]> On Behalf Of Nick Desaulniers
> Sent: Tuesday, November 10, 2020 6:12 PM
> To: Moore, Robert <[email protected]>; Kaneda, Erik <[email protected]>; Wysocki, Rafael J <[email protected]>; Gustavo A . R . Silva <[email protected]>
> Cc: [email protected]; Nick Desaulniers <[email protected]>; Len Brown <[email protected]>; [email protected]; [email protected]; [email protected]
> Subject: [PATCH] ACPICA: fix -Wfallthrough
>
> The "fallthrough" pseudo-keyword was added as a portable way to denote intentional fallthrough. This code seemed to be using a mix of fallthrough comments that GCC recognizes, and some kind of lint marker.
> I'm guessing that linter hasn't been run in a while from the mixed use of the marker vs comments.
>
> Signed-off-by: Nick Desaulniers <[email protected]>
> ---
> drivers/acpi/acpica/dscontrol.c | 3 +--
> drivers/acpi/acpica/dswexec.c | 4 +---
> drivers/acpi/acpica/dswload.c | 3 +--
> drivers/acpi/acpica/dswload2.c | 3 +--
> drivers/acpi/acpica/exfldio.c | 3 +--
> drivers/acpi/acpica/exresop.c | 5 ++---
> drivers/acpi/acpica/exstore.c | 6 ++----
> drivers/acpi/acpica/hwgpe.c | 3 +--
> drivers/acpi/acpica/utdelete.c | 3 +--
> drivers/acpi/acpica/utprint.c | 2 +-
> 10 files changed, 12 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/acpi/acpica/dscontrol.c b/drivers/acpi/acpica/dscontrol.c index 4b5b6e859f62..1e75e5fbfd19 100644
> --- a/drivers/acpi/acpica/dscontrol.c
> +++ b/drivers/acpi/acpica/dscontrol.c
> @@ -61,8 +61,7 @@ acpi_ds_exec_begin_control_op(struct acpi_walk_state *walk_state,
> break;
> }
> }
> -
> - /*lint -fallthrough */
> + fallthrough;
>
> case AML_IF_OP:
> /*
> diff --git a/drivers/acpi/acpica/dswexec.c b/drivers/acpi/acpica/dswexec.c index 1d4f8c81028c..e8c32d4fe55f 100644
> --- a/drivers/acpi/acpica/dswexec.c
> +++ b/drivers/acpi/acpica/dswexec.c
> @@ -597,9 +597,7 @@ acpi_status acpi_ds_exec_end_op(struct acpi_walk_state *walk_state)
> if (ACPI_FAILURE(status)) {
> break;
> }
> -
> - /* Fall through */
> - /*lint -fallthrough */
> + fallthrough;
>
> case AML_INT_EVAL_SUBTREE_OP:
>
> diff --git a/drivers/acpi/acpica/dswload.c b/drivers/acpi/acpica/dswload.c index 27069325b6de..afc663c3742d 100644
> --- a/drivers/acpi/acpica/dswload.c
> +++ b/drivers/acpi/acpica/dswload.c
> @@ -223,8 +223,7 @@ acpi_ds_load1_begin_op(struct acpi_walk_state *walk_state,
> parse_flags & ACPI_PARSE_MODULE_LEVEL)) {
> break;
> }
> -
> - /*lint -fallthrough */
> + fallthrough;
>
> default:
>
> diff --git a/drivers/acpi/acpica/dswload2.c b/drivers/acpi/acpica/dswload2.c index edadbe146506..1b794b6ba072 100644
> --- a/drivers/acpi/acpica/dswload2.c
> +++ b/drivers/acpi/acpica/dswload2.c
> @@ -213,8 +213,7 @@ acpi_ds_load2_begin_op(struct acpi_walk_state *walk_state,
> parse_flags & ACPI_PARSE_MODULE_LEVEL)) {
> break;
> }
> -
> - /*lint -fallthrough */
> + fallthrough;
>
> default:
>
> diff --git a/drivers/acpi/acpica/exfldio.c b/drivers/acpi/acpica/exfldio.c index ade35ff1c7ba..9d1cabe0fed9 100644
> --- a/drivers/acpi/acpica/exfldio.c
> +++ b/drivers/acpi/acpica/exfldio.c
> @@ -433,8 +433,7 @@ acpi_ex_field_datum_io(union acpi_operand_object *obj_desc,
> * Now that the Bank has been selected, fall through to the
> * region_field case and write the datum to the Operation Region
> */
> -
> - /*lint -fallthrough */
> + fallthrough;
>
> case ACPI_TYPE_LOCAL_REGION_FIELD:
> /*
> diff --git a/drivers/acpi/acpica/exresop.c b/drivers/acpi/acpica/exresop.c index 4d1b22971d58..df48faa9a551 100644
> --- a/drivers/acpi/acpica/exresop.c
> +++ b/drivers/acpi/acpica/exresop.c
> @@ -197,8 +197,7 @@ acpi_ex_resolve_operands(u16 opcode,
> case ACPI_REFCLASS_DEBUG:
>
> target_op = AML_DEBUG_OP;
> -
> - /*lint -fallthrough */
> + fallthrough;
>
> case ACPI_REFCLASS_ARG:
> case ACPI_REFCLASS_LOCAL:
> @@ -264,7 +263,7 @@ acpi_ex_resolve_operands(u16 opcode,
> * Else not a string - fall through to the normal Reference
> * case below
> */
> - /*lint -fallthrough */
> + 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..2067baa7c120 100644
> --- a/drivers/acpi/acpica/exstore.c
> +++ b/drivers/acpi/acpica/exstore.c
> @@ -95,8 +95,7 @@ acpi_ex_store(union acpi_operand_object *source_desc,
> if (dest_desc->common.flags & AOPOBJ_AML_CONSTANT) {
> return_ACPI_STATUS(AE_OK);
> }
> -
> - /*lint -fallthrough */
> + fallthrough;
>
> default:
>
> @@ -421,8 +420,7 @@ acpi_ex_store_object_to_node(union acpi_operand_object *source_desc,
> }
> break;
> }
> -
> - /* Fallthrough */
> + 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..fbfad80c8a53 100644
> --- a/drivers/acpi/acpica/hwgpe.c
> +++ b/drivers/acpi/acpica/hwgpe.c
> @@ -166,8 +166,7 @@ acpi_hw_low_set_gpe(struct acpi_gpe_event_info *gpe_event_info, u32 action)
> if (!(register_bit & gpe_register_info->enable_mask)) {
> return (AE_BAD_PARAMETER);
> }
> -
> - /*lint -fallthrough */
> + fallthrough;
>
> case ACPI_GPE_ENABLE:
>
> diff --git a/drivers/acpi/acpica/utdelete.c b/drivers/acpi/acpica/utdelete.c index 4c0d4e434196..8076e7947585 100644
> --- a/drivers/acpi/acpica/utdelete.c
> +++ b/drivers/acpi/acpica/utdelete.c
> @@ -111,8 +111,7 @@ static void acpi_ut_delete_internal_obj(union acpi_operand_object *object)
> (void)acpi_ev_delete_gpe_block(object->device.
> gpe_block);
> }
> -
> - /*lint -fallthrough */
> + fallthrough;
>
> case ACPI_TYPE_PROCESSOR:
> case ACPI_TYPE_THERMAL:
> diff --git a/drivers/acpi/acpica/utprint.c b/drivers/acpi/acpica/utprint.c index 681c11f4af4e..f7e43baf5ff2 100644
> --- a/drivers/acpi/acpica/utprint.c
> +++ b/drivers/acpi/acpica/utprint.c
> @@ -475,7 +475,7 @@ int vsnprintf(char *string, acpi_size size, const char *format, va_list args)
> case 'X':
>
> type |= ACPI_FORMAT_UPPER;
> - /* FALLTHROUGH */
> + fallthrough;
>
> case 'x':
>
> --
> 2.29.2.222.g5d2a92d10f8-goog
>
--
Thanks,
~Nick Desaulniers
-----Original Message-----
From: Nick Desaulniers <[email protected]>
Sent: Wednesday, November 11, 2020 10:48 AM
To: Moore, Robert <[email protected]>
Cc: Kaneda, Erik <[email protected]>; Wysocki, Rafael J <[email protected]>; Gustavo A . R . Silva <[email protected]>; [email protected]; Len Brown <[email protected]>; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH] ACPICA: fix -Wfallthrough
On Wed, Nov 11, 2020 at 7:15 AM Moore, Robert <[email protected]> wrote:
>
> Yes, but: isn't the "fallthrough" keyword compiler-specific? That is the problem for us.
It's not a keyword.
It's a preprocessor macro that expands to
__attribute__((__fallthrough__)) for compilers that support it. For compilers that do not, it expands to nothing. Both GCC 7+ and Clang support this attribute. Which other compilers that support -Wimplicit-fallthrough do you care to support?
We need to support MSVC 2017 -- which apparently does not support this.
> Bob
>
>
> -----Original Message-----
> From: ndesaulniers via sendgmr
> <[email protected]> On Behalf Of Nick
> Desaulniers
> Sent: Tuesday, November 10, 2020 6:12 PM
> To: Moore, Robert <[email protected]>; Kaneda, Erik
> <[email protected]>; Wysocki, Rafael J
> <[email protected]>; Gustavo A . R . Silva
> <[email protected]>
> Cc: [email protected]; Nick Desaulniers
> <[email protected]>; Len Brown <[email protected]>;
> [email protected]; [email protected];
> [email protected]
> Subject: [PATCH] ACPICA: fix -Wfallthrough
>
> The "fallthrough" pseudo-keyword was added as a portable way to denote intentional fallthrough. This code seemed to be using a mix of fallthrough comments that GCC recognizes, and some kind of lint marker.
> I'm guessing that linter hasn't been run in a while from the mixed use of the marker vs comments.
>
> Signed-off-by: Nick Desaulniers <[email protected]>
> ---
> drivers/acpi/acpica/dscontrol.c | 3 +--
> drivers/acpi/acpica/dswexec.c | 4 +---
> drivers/acpi/acpica/dswload.c | 3 +--
> drivers/acpi/acpica/dswload2.c | 3 +--
> drivers/acpi/acpica/exfldio.c | 3 +--
> drivers/acpi/acpica/exresop.c | 5 ++---
> drivers/acpi/acpica/exstore.c | 6 ++----
> drivers/acpi/acpica/hwgpe.c | 3 +--
> drivers/acpi/acpica/utdelete.c | 3 +--
> drivers/acpi/acpica/utprint.c | 2 +-
> 10 files changed, 12 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/acpi/acpica/dscontrol.c
> b/drivers/acpi/acpica/dscontrol.c index 4b5b6e859f62..1e75e5fbfd19
> 100644
> --- a/drivers/acpi/acpica/dscontrol.c
> +++ b/drivers/acpi/acpica/dscontrol.c
> @@ -61,8 +61,7 @@ acpi_ds_exec_begin_control_op(struct acpi_walk_state *walk_state,
> break;
> }
> }
> -
> - /*lint -fallthrough */
> + fallthrough;
>
> case AML_IF_OP:
> /*
> diff --git a/drivers/acpi/acpica/dswexec.c
> b/drivers/acpi/acpica/dswexec.c index 1d4f8c81028c..e8c32d4fe55f
> 100644
> --- a/drivers/acpi/acpica/dswexec.c
> +++ b/drivers/acpi/acpica/dswexec.c
> @@ -597,9 +597,7 @@ acpi_status acpi_ds_exec_end_op(struct acpi_walk_state *walk_state)
> if (ACPI_FAILURE(status)) {
> break;
> }
> -
> - /* Fall through */
> - /*lint -fallthrough */
> + fallthrough;
>
> case AML_INT_EVAL_SUBTREE_OP:
>
> diff --git a/drivers/acpi/acpica/dswload.c
> b/drivers/acpi/acpica/dswload.c index 27069325b6de..afc663c3742d
> 100644
> --- a/drivers/acpi/acpica/dswload.c
> +++ b/drivers/acpi/acpica/dswload.c
> @@ -223,8 +223,7 @@ acpi_ds_load1_begin_op(struct acpi_walk_state *walk_state,
> parse_flags & ACPI_PARSE_MODULE_LEVEL)) {
> break;
> }
> -
> - /*lint -fallthrough */
> + fallthrough;
>
> default:
>
> diff --git a/drivers/acpi/acpica/dswload2.c
> b/drivers/acpi/acpica/dswload2.c index edadbe146506..1b794b6ba072
> 100644
> --- a/drivers/acpi/acpica/dswload2.c
> +++ b/drivers/acpi/acpica/dswload2.c
> @@ -213,8 +213,7 @@ acpi_ds_load2_begin_op(struct acpi_walk_state *walk_state,
> parse_flags & ACPI_PARSE_MODULE_LEVEL)) {
> break;
> }
> -
> - /*lint -fallthrough */
> + fallthrough;
>
> default:
>
> diff --git a/drivers/acpi/acpica/exfldio.c
> b/drivers/acpi/acpica/exfldio.c index ade35ff1c7ba..9d1cabe0fed9
> 100644
> --- a/drivers/acpi/acpica/exfldio.c
> +++ b/drivers/acpi/acpica/exfldio.c
> @@ -433,8 +433,7 @@ acpi_ex_field_datum_io(union acpi_operand_object *obj_desc,
> * Now that the Bank has been selected, fall through to the
> * region_field case and write the datum to the Operation Region
> */
> -
> - /*lint -fallthrough */
> + fallthrough;
>
> case ACPI_TYPE_LOCAL_REGION_FIELD:
> /*
> diff --git a/drivers/acpi/acpica/exresop.c
> b/drivers/acpi/acpica/exresop.c index 4d1b22971d58..df48faa9a551
> 100644
> --- a/drivers/acpi/acpica/exresop.c
> +++ b/drivers/acpi/acpica/exresop.c
> @@ -197,8 +197,7 @@ acpi_ex_resolve_operands(u16 opcode,
> case ACPI_REFCLASS_DEBUG:
>
> target_op = AML_DEBUG_OP;
> -
> - /*lint -fallthrough */
> + fallthrough;
>
> case ACPI_REFCLASS_ARG:
> case ACPI_REFCLASS_LOCAL:
> @@ -264,7 +263,7 @@ acpi_ex_resolve_operands(u16 opcode,
> * Else not a string - fall through to the normal Reference
> * case below
> */
> - /*lint -fallthrough */
> + 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..2067baa7c120
> 100644
> --- a/drivers/acpi/acpica/exstore.c
> +++ b/drivers/acpi/acpica/exstore.c
> @@ -95,8 +95,7 @@ acpi_ex_store(union acpi_operand_object *source_desc,
> if (dest_desc->common.flags & AOPOBJ_AML_CONSTANT) {
> return_ACPI_STATUS(AE_OK);
> }
> -
> - /*lint -fallthrough */
> + fallthrough;
>
> default:
>
> @@ -421,8 +420,7 @@ acpi_ex_store_object_to_node(union acpi_operand_object *source_desc,
> }
> break;
> }
> -
> - /* Fallthrough */
> + 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..fbfad80c8a53 100644
> --- a/drivers/acpi/acpica/hwgpe.c
> +++ b/drivers/acpi/acpica/hwgpe.c
> @@ -166,8 +166,7 @@ acpi_hw_low_set_gpe(struct acpi_gpe_event_info *gpe_event_info, u32 action)
> if (!(register_bit & gpe_register_info->enable_mask)) {
> return (AE_BAD_PARAMETER);
> }
> -
> - /*lint -fallthrough */
> + fallthrough;
>
> case ACPI_GPE_ENABLE:
>
> diff --git a/drivers/acpi/acpica/utdelete.c
> b/drivers/acpi/acpica/utdelete.c index 4c0d4e434196..8076e7947585
> 100644
> --- a/drivers/acpi/acpica/utdelete.c
> +++ b/drivers/acpi/acpica/utdelete.c
> @@ -111,8 +111,7 @@ static void acpi_ut_delete_internal_obj(union acpi_operand_object *object)
> (void)acpi_ev_delete_gpe_block(object->device.
> gpe_block);
> }
> -
> - /*lint -fallthrough */
> + fallthrough;
>
> case ACPI_TYPE_PROCESSOR:
> case ACPI_TYPE_THERMAL:
> diff --git a/drivers/acpi/acpica/utprint.c
> b/drivers/acpi/acpica/utprint.c index 681c11f4af4e..f7e43baf5ff2
> 100644
> --- a/drivers/acpi/acpica/utprint.c
> +++ b/drivers/acpi/acpica/utprint.c
> @@ -475,7 +475,7 @@ int vsnprintf(char *string, acpi_size size, const char *format, va_list args)
> case 'X':
>
> type |= ACPI_FORMAT_UPPER;
> - /* FALLTHROUGH */
> + fallthrough;
>
> case 'x':
>
> --
> 2.29.2.222.g5d2a92d10f8-goog
>
--
Thanks,
~Nick Desaulniers
On Thu, Nov 12, 2020 at 7:13 AM Moore, Robert <[email protected]> wrote:
>
>
>
> -----Original Message-----
> From: Nick Desaulniers <[email protected]>
> Sent: Wednesday, November 11, 2020 10:48 AM
> To: Moore, Robert <[email protected]>
> Cc: Kaneda, Erik <[email protected]>; Wysocki, Rafael J <[email protected]>; Gustavo A . R . Silva <[email protected]>; [email protected]; Len Brown <[email protected]>; [email protected]; [email protected]; [email protected]
> Subject: Re: [PATCH] ACPICA: fix -Wfallthrough
>
> On Wed, Nov 11, 2020 at 7:15 AM Moore, Robert <[email protected]> wrote:
> >
> > Yes, but: isn't the "fallthrough" keyword compiler-specific? That is the problem for us.
>
> It's not a keyword.
>
> It's a preprocessor macro that expands to
> __attribute__((__fallthrough__)) for compilers that support it. For compilers that do not, it expands to nothing. Both GCC 7+ and Clang support this attribute. Which other compilers that support -Wimplicit-fallthrough do you care to support?
>
> We need to support MSVC 2017 -- which apparently does not support this.
In which case, the macro is not expanded to a compiler attribute the
compiler doesn't support. Please see also its definition in
include/linux/compiler_attributes.h.
From what I can tell, MSVC does not warn on implicit fallthrough, so
there's no corresponding attribute (or comment) to disable the warning
in MSVC.
That doesn't mean this code is not portable to MSVC; a macro that
expands to nothing should not be a problem.
Based on
https://docs.microsoft.com/en-us/cpp/code-quality/c26819?view=msvc-160
https://developercommunity.visualstudio.com/idea/423975/issue-compiler-warning-when-using-implicit-fallthr.html
it sounds like MSVC 2019 will be able to warn, for C++ mode, which
will rely on the C++ style attribute to annotate intentional
fallthrough.
Can you confirm how this does not work for MSVC 2017?
> > Bob
> >
> >
> > -----Original Message-----
> > From: ndesaulniers via sendgmr
> > <[email protected]> On Behalf Of Nick
> > Desaulniers
> > Sent: Tuesday, November 10, 2020 6:12 PM
> > To: Moore, Robert <[email protected]>; Kaneda, Erik
> > <[email protected]>; Wysocki, Rafael J
> > <[email protected]>; Gustavo A . R . Silva
> > <[email protected]>
> > Cc: [email protected]; Nick Desaulniers
> > <[email protected]>; Len Brown <[email protected]>;
> > [email protected]; [email protected];
> > [email protected]
> > Subject: [PATCH] ACPICA: fix -Wfallthrough
> >
> > The "fallthrough" pseudo-keyword was added as a portable way to denote intentional fallthrough. This code seemed to be using a mix of fallthrough comments that GCC recognizes, and some kind of lint marker.
> > I'm guessing that linter hasn't been run in a while from the mixed use of the marker vs comments.
> >
> > Signed-off-by: Nick Desaulniers <[email protected]>
> > ---
> > drivers/acpi/acpica/dscontrol.c | 3 +--
> > drivers/acpi/acpica/dswexec.c | 4 +---
> > drivers/acpi/acpica/dswload.c | 3 +--
> > drivers/acpi/acpica/dswload2.c | 3 +--
> > drivers/acpi/acpica/exfldio.c | 3 +--
> > drivers/acpi/acpica/exresop.c | 5 ++---
> > drivers/acpi/acpica/exstore.c | 6 ++----
> > drivers/acpi/acpica/hwgpe.c | 3 +--
> > drivers/acpi/acpica/utdelete.c | 3 +--
> > drivers/acpi/acpica/utprint.c | 2 +-
> > 10 files changed, 12 insertions(+), 23 deletions(-)
> >
> > diff --git a/drivers/acpi/acpica/dscontrol.c
> > b/drivers/acpi/acpica/dscontrol.c index 4b5b6e859f62..1e75e5fbfd19
> > 100644
> > --- a/drivers/acpi/acpica/dscontrol.c
> > +++ b/drivers/acpi/acpica/dscontrol.c
> > @@ -61,8 +61,7 @@ acpi_ds_exec_begin_control_op(struct acpi_walk_state *walk_state,
> > break;
> > }
> > }
> > -
> > - /*lint -fallthrough */
> > + fallthrough;
> >
> > case AML_IF_OP:
> > /*
> > diff --git a/drivers/acpi/acpica/dswexec.c
> > b/drivers/acpi/acpica/dswexec.c index 1d4f8c81028c..e8c32d4fe55f
> > 100644
> > --- a/drivers/acpi/acpica/dswexec.c
> > +++ b/drivers/acpi/acpica/dswexec.c
> > @@ -597,9 +597,7 @@ acpi_status acpi_ds_exec_end_op(struct acpi_walk_state *walk_state)
> > if (ACPI_FAILURE(status)) {
> > break;
> > }
> > -
> > - /* Fall through */
> > - /*lint -fallthrough */
> > + fallthrough;
> >
> > case AML_INT_EVAL_SUBTREE_OP:
> >
> > diff --git a/drivers/acpi/acpica/dswload.c
> > b/drivers/acpi/acpica/dswload.c index 27069325b6de..afc663c3742d
> > 100644
> > --- a/drivers/acpi/acpica/dswload.c
> > +++ b/drivers/acpi/acpica/dswload.c
> > @@ -223,8 +223,7 @@ acpi_ds_load1_begin_op(struct acpi_walk_state *walk_state,
> > parse_flags & ACPI_PARSE_MODULE_LEVEL)) {
> > break;
> > }
> > -
> > - /*lint -fallthrough */
> > + fallthrough;
> >
> > default:
> >
> > diff --git a/drivers/acpi/acpica/dswload2.c
> > b/drivers/acpi/acpica/dswload2.c index edadbe146506..1b794b6ba072
> > 100644
> > --- a/drivers/acpi/acpica/dswload2.c
> > +++ b/drivers/acpi/acpica/dswload2.c
> > @@ -213,8 +213,7 @@ acpi_ds_load2_begin_op(struct acpi_walk_state *walk_state,
> > parse_flags & ACPI_PARSE_MODULE_LEVEL)) {
> > break;
> > }
> > -
> > - /*lint -fallthrough */
> > + fallthrough;
> >
> > default:
> >
> > diff --git a/drivers/acpi/acpica/exfldio.c
> > b/drivers/acpi/acpica/exfldio.c index ade35ff1c7ba..9d1cabe0fed9
> > 100644
> > --- a/drivers/acpi/acpica/exfldio.c
> > +++ b/drivers/acpi/acpica/exfldio.c
> > @@ -433,8 +433,7 @@ acpi_ex_field_datum_io(union acpi_operand_object *obj_desc,
> > * Now that the Bank has been selected, fall through to the
> > * region_field case and write the datum to the Operation Region
> > */
> > -
> > - /*lint -fallthrough */
> > + fallthrough;
> >
> > case ACPI_TYPE_LOCAL_REGION_FIELD:
> > /*
> > diff --git a/drivers/acpi/acpica/exresop.c
> > b/drivers/acpi/acpica/exresop.c index 4d1b22971d58..df48faa9a551
> > 100644
> > --- a/drivers/acpi/acpica/exresop.c
> > +++ b/drivers/acpi/acpica/exresop.c
> > @@ -197,8 +197,7 @@ acpi_ex_resolve_operands(u16 opcode,
> > case ACPI_REFCLASS_DEBUG:
> >
> > target_op = AML_DEBUG_OP;
> > -
> > - /*lint -fallthrough */
> > + fallthrough;
> >
> > case ACPI_REFCLASS_ARG:
> > case ACPI_REFCLASS_LOCAL:
> > @@ -264,7 +263,7 @@ acpi_ex_resolve_operands(u16 opcode,
> > * Else not a string - fall through to the normal Reference
> > * case below
> > */
> > - /*lint -fallthrough */
> > + 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..2067baa7c120
> > 100644
> > --- a/drivers/acpi/acpica/exstore.c
> > +++ b/drivers/acpi/acpica/exstore.c
> > @@ -95,8 +95,7 @@ acpi_ex_store(union acpi_operand_object *source_desc,
> > if (dest_desc->common.flags & AOPOBJ_AML_CONSTANT) {
> > return_ACPI_STATUS(AE_OK);
> > }
> > -
> > - /*lint -fallthrough */
> > + fallthrough;
> >
> > default:
> >
> > @@ -421,8 +420,7 @@ acpi_ex_store_object_to_node(union acpi_operand_object *source_desc,
> > }
> > break;
> > }
> > -
> > - /* Fallthrough */
> > + 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..fbfad80c8a53 100644
> > --- a/drivers/acpi/acpica/hwgpe.c
> > +++ b/drivers/acpi/acpica/hwgpe.c
> > @@ -166,8 +166,7 @@ acpi_hw_low_set_gpe(struct acpi_gpe_event_info *gpe_event_info, u32 action)
> > if (!(register_bit & gpe_register_info->enable_mask)) {
> > return (AE_BAD_PARAMETER);
> > }
> > -
> > - /*lint -fallthrough */
> > + fallthrough;
> >
> > case ACPI_GPE_ENABLE:
> >
> > diff --git a/drivers/acpi/acpica/utdelete.c
> > b/drivers/acpi/acpica/utdelete.c index 4c0d4e434196..8076e7947585
> > 100644
> > --- a/drivers/acpi/acpica/utdelete.c
> > +++ b/drivers/acpi/acpica/utdelete.c
> > @@ -111,8 +111,7 @@ static void acpi_ut_delete_internal_obj(union acpi_operand_object *object)
> > (void)acpi_ev_delete_gpe_block(object->device.
> > gpe_block);
> > }
> > -
> > - /*lint -fallthrough */
> > + fallthrough;
> >
> > case ACPI_TYPE_PROCESSOR:
> > case ACPI_TYPE_THERMAL:
> > diff --git a/drivers/acpi/acpica/utprint.c
> > b/drivers/acpi/acpica/utprint.c index 681c11f4af4e..f7e43baf5ff2
> > 100644
> > --- a/drivers/acpi/acpica/utprint.c
> > +++ b/drivers/acpi/acpica/utprint.c
> > @@ -475,7 +475,7 @@ int vsnprintf(char *string, acpi_size size, const char *format, va_list args)
> > case 'X':
> >
> > type |= ACPI_FORMAT_UPPER;
> > - /* FALLTHROUGH */
> > + fallthrough;
> >
> > case 'x':
> >
> > --
> > 2.29.2.222.g5d2a92d10f8-goog
> >
>
>
> --
> Thanks,
> ~Nick Desaulniers
--
Thanks,
~Nick Desaulniers
On Thu, 2020-11-12 at 11:30 -0800, Nick Desaulniers wrote:
> On Thu, Nov 12, 2020 at 7:13 AM Moore, Robert <[email protected]> wrote:
> > -----Original Message-----
> > From: Nick Desaulniers <[email protected]>
> > On Wed, Nov 11, 2020 at 7:15 AM Moore, Robert <[email protected]> wrote:
> > > Yes, but: isn't the "fallthrough" keyword compiler-specific? That is the problem for us.
> > It's not a keyword.
> >
> > It's a preprocessor macro that expands to
> > __attribute__((__fallthrough__)) for compilers that support it. For compilers that do not, it expands to nothing. Both GCC 7+ and Clang support this attribute. Which other compilers that support -Wimplicit-fallthrough do you care to support?
> >
> > We need to support MSVC 2017 -- which apparently does not support this.
>
> In which case, the macro is not expanded to a compiler attribute the
> compiler doesn't support. Please see also its definition in
> include/linux/compiler_attributes.h.
>
> From what I can tell, MSVC does not warn on implicit fallthrough, so
> there's no corresponding attribute (or comment) to disable the warning
> in MSVC.
>
> That doesn't mean this code is not portable to MSVC; a macro that
> expands to nothing should not be a problem.
acpica is a special case as all the code is in a separate
repository and converted via Lindent to resemble linux
standard styles.
Perhaps it'd easier to avoid modifying acpica and add something like:
---
diff --git a/drivers/acpi/acpica/Makefile b/drivers/acpi/acpica/Makefile
index 59700433a96e..469508a8d671 100644
--- a/drivers/acpi/acpica/Makefile
+++ b/drivers/acpi/acpica/Makefile
@@ -4,6 +4,7 @@
#
ccflags-y := -Os -D_LINUX -DBUILDING_ACPICA
+ccflags-y += -Wno-implicit-fallthrough
ccflags-$(CONFIG_ACPI_DEBUG) += -DACPI_DEBUG_OUTPUT
# use acpi.o to put all files here into acpi.o modparam namespace
-----Original Message-----
From: Nick Desaulniers <[email protected]>
Sent: Thursday, November 12, 2020 11:31 AM
To: Moore, Robert <[email protected]>
Cc: Kaneda, Erik <[email protected]>; Wysocki, Rafael J <[email protected]>; Gustavo A . R . Silva <[email protected]>; [email protected]; Len Brown <[email protected]>; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH] ACPICA: fix -Wfallthrough
On Thu, Nov 12, 2020 at 7:13 AM Moore, Robert <[email protected]> wrote:
>
>
>
> -----Original Message-----
> From: Nick Desaulniers <[email protected]>
> Sent: Wednesday, November 11, 2020 10:48 AM
> To: Moore, Robert <[email protected]>
> Cc: Kaneda, Erik <[email protected]>; Wysocki, Rafael J
> <[email protected]>; Gustavo A . R . Silva
> <[email protected]>; [email protected]; Len Brown
> <[email protected]>; [email protected]; [email protected];
> [email protected]
> Subject: Re: [PATCH] ACPICA: fix -Wfallthrough
>
> On Wed, Nov 11, 2020 at 7:15 AM Moore, Robert <[email protected]> wrote:
> >
> > Yes, but: isn't the "fallthrough" keyword compiler-specific? That is the problem for us.
>
> It's not a keyword.
>
> It's a preprocessor macro that expands to
> __attribute__((__fallthrough__)) for compilers that support it. For compilers that do not, it expands to nothing. Both GCC 7+ and Clang support this attribute. Which other compilers that support -Wimplicit-fallthrough do you care to support?
>
> We need to support MSVC 2017 -- which apparently does not support this.
In which case, the macro is not expanded to a compiler attribute the compiler doesn't support. Please see also its definition in include/linux/compiler_attributes.h.
From what I can tell, MSVC does not warn on implicit fallthrough, so there's no corresponding attribute (or comment) to disable the warning in MSVC.
That doesn't mean this code is not portable to MSVC; a macro that expands to nothing should not be a problem.
Based on
https://docs.microsoft.com/en-us/cpp/code-quality/c26819?view=msvc-160
https://developercommunity.visualstudio.com/idea/423975/issue-compiler-warning-when-using-implicit-fallthr.html
it sounds like MSVC 2019 will be able to warn, for C++ mode, which will rely on the C++ style attribute to annotate intentional fallthrough.
Can you confirm how this does not work for MSVC 2017?
1>c:\acpica\source\components\utilities\utdelete.c(270): warning C4013: '__attribute__' undefined; assuming extern returning int
1>c:\acpica\source\components\utilities\utdelete.c(270): error C2065: '__fallthrough__': undeclared identifier
1>c:\acpica\source\components\utilities\utdelete.c(272): error C2143: syntax error: missing ';' before 'case'
> > Bob
> >
> >
> > -----Original Message-----
> > From: ndesaulniers via sendgmr
> > <[email protected]> On Behalf Of Nick
> > Desaulniers
> > Sent: Tuesday, November 10, 2020 6:12 PM
> > To: Moore, Robert <[email protected]>; Kaneda, Erik
> > <[email protected]>; Wysocki, Rafael J
> > <[email protected]>; Gustavo A . R . Silva
> > <[email protected]>
> > Cc: [email protected]; Nick Desaulniers
> > <[email protected]>; Len Brown <[email protected]>;
> > [email protected]; [email protected];
> > [email protected]
> > Subject: [PATCH] ACPICA: fix -Wfallthrough
> >
> > The "fallthrough" pseudo-keyword was added as a portable way to denote intentional fallthrough. This code seemed to be using a mix of fallthrough comments that GCC recognizes, and some kind of lint marker.
> > I'm guessing that linter hasn't been run in a while from the mixed use of the marker vs comments.
> >
> > Signed-off-by: Nick Desaulniers <[email protected]>
> > ---
> > drivers/acpi/acpica/dscontrol.c | 3 +--
> > drivers/acpi/acpica/dswexec.c | 4 +---
> > drivers/acpi/acpica/dswload.c | 3 +--
> > drivers/acpi/acpica/dswload2.c | 3 +--
> > drivers/acpi/acpica/exfldio.c | 3 +--
> > drivers/acpi/acpica/exresop.c | 5 ++---
> > drivers/acpi/acpica/exstore.c | 6 ++----
> > drivers/acpi/acpica/hwgpe.c | 3 +--
> > drivers/acpi/acpica/utdelete.c | 3 +--
> > drivers/acpi/acpica/utprint.c | 2 +-
> > 10 files changed, 12 insertions(+), 23 deletions(-)
> >
> > diff --git a/drivers/acpi/acpica/dscontrol.c
> > b/drivers/acpi/acpica/dscontrol.c index 4b5b6e859f62..1e75e5fbfd19
> > 100644
> > --- a/drivers/acpi/acpica/dscontrol.c
> > +++ b/drivers/acpi/acpica/dscontrol.c
> > @@ -61,8 +61,7 @@ acpi_ds_exec_begin_control_op(struct acpi_walk_state *walk_state,
> > break;
> > }
> > }
> > -
> > - /*lint -fallthrough */
> > + fallthrough;
> >
> > case AML_IF_OP:
> > /*
> > diff --git a/drivers/acpi/acpica/dswexec.c
> > b/drivers/acpi/acpica/dswexec.c index 1d4f8c81028c..e8c32d4fe55f
> > 100644
> > --- a/drivers/acpi/acpica/dswexec.c
> > +++ b/drivers/acpi/acpica/dswexec.c
> > @@ -597,9 +597,7 @@ acpi_status acpi_ds_exec_end_op(struct acpi_walk_state *walk_state)
> > if (ACPI_FAILURE(status)) {
> > break;
> > }
> > -
> > - /* Fall through */
> > - /*lint -fallthrough */
> > + fallthrough;
> >
> > case AML_INT_EVAL_SUBTREE_OP:
> >
> > diff --git a/drivers/acpi/acpica/dswload.c
> > b/drivers/acpi/acpica/dswload.c index 27069325b6de..afc663c3742d
> > 100644
> > --- a/drivers/acpi/acpica/dswload.c
> > +++ b/drivers/acpi/acpica/dswload.c
> > @@ -223,8 +223,7 @@ acpi_ds_load1_begin_op(struct acpi_walk_state *walk_state,
> > parse_flags & ACPI_PARSE_MODULE_LEVEL)) {
> > break;
> > }
> > -
> > - /*lint -fallthrough */
> > + fallthrough;
> >
> > default:
> >
> > diff --git a/drivers/acpi/acpica/dswload2.c
> > b/drivers/acpi/acpica/dswload2.c index edadbe146506..1b794b6ba072
> > 100644
> > --- a/drivers/acpi/acpica/dswload2.c
> > +++ b/drivers/acpi/acpica/dswload2.c
> > @@ -213,8 +213,7 @@ acpi_ds_load2_begin_op(struct acpi_walk_state *walk_state,
> > parse_flags & ACPI_PARSE_MODULE_LEVEL)) {
> > break;
> > }
> > -
> > - /*lint -fallthrough */
> > + fallthrough;
> >
> > default:
> >
> > diff --git a/drivers/acpi/acpica/exfldio.c
> > b/drivers/acpi/acpica/exfldio.c index ade35ff1c7ba..9d1cabe0fed9
> > 100644
> > --- a/drivers/acpi/acpica/exfldio.c
> > +++ b/drivers/acpi/acpica/exfldio.c
> > @@ -433,8 +433,7 @@ acpi_ex_field_datum_io(union acpi_operand_object *obj_desc,
> > * Now that the Bank has been selected, fall through to the
> > * region_field case and write the datum to the Operation Region
> > */
> > -
> > - /*lint -fallthrough */
> > + fallthrough;
> >
> > case ACPI_TYPE_LOCAL_REGION_FIELD:
> > /*
> > diff --git a/drivers/acpi/acpica/exresop.c
> > b/drivers/acpi/acpica/exresop.c index 4d1b22971d58..df48faa9a551
> > 100644
> > --- a/drivers/acpi/acpica/exresop.c
> > +++ b/drivers/acpi/acpica/exresop.c
> > @@ -197,8 +197,7 @@ acpi_ex_resolve_operands(u16 opcode,
> > case ACPI_REFCLASS_DEBUG:
> >
> > target_op = AML_DEBUG_OP;
> > -
> > - /*lint -fallthrough */
> > + fallthrough;
> >
> > case ACPI_REFCLASS_ARG:
> > case ACPI_REFCLASS_LOCAL:
> > @@ -264,7 +263,7 @@ acpi_ex_resolve_operands(u16 opcode,
> > * Else not a string - fall through to the normal Reference
> > * case below
> > */
> > - /*lint -fallthrough */
> > + 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..2067baa7c120
> > 100644
> > --- a/drivers/acpi/acpica/exstore.c
> > +++ b/drivers/acpi/acpica/exstore.c
> > @@ -95,8 +95,7 @@ acpi_ex_store(union acpi_operand_object *source_desc,
> > if (dest_desc->common.flags & AOPOBJ_AML_CONSTANT) {
> > return_ACPI_STATUS(AE_OK);
> > }
> > -
> > - /*lint -fallthrough */
> > + fallthrough;
> >
> > default:
> >
> > @@ -421,8 +420,7 @@ acpi_ex_store_object_to_node(union acpi_operand_object *source_desc,
> > }
> > break;
> > }
> > -
> > - /* Fallthrough */
> > + 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..fbfad80c8a53
> > 100644
> > --- a/drivers/acpi/acpica/hwgpe.c
> > +++ b/drivers/acpi/acpica/hwgpe.c
> > @@ -166,8 +166,7 @@ acpi_hw_low_set_gpe(struct acpi_gpe_event_info *gpe_event_info, u32 action)
> > if (!(register_bit & gpe_register_info->enable_mask)) {
> > return (AE_BAD_PARAMETER);
> > }
> > -
> > - /*lint -fallthrough */
> > + fallthrough;
> >
> > case ACPI_GPE_ENABLE:
> >
> > diff --git a/drivers/acpi/acpica/utdelete.c
> > b/drivers/acpi/acpica/utdelete.c index 4c0d4e434196..8076e7947585
> > 100644
> > --- a/drivers/acpi/acpica/utdelete.c
> > +++ b/drivers/acpi/acpica/utdelete.c
> > @@ -111,8 +111,7 @@ static void acpi_ut_delete_internal_obj(union acpi_operand_object *object)
> > (void)acpi_ev_delete_gpe_block(object->device.
> > gpe_block);
> > }
> > -
> > - /*lint -fallthrough */
> > + fallthrough;
> >
> > case ACPI_TYPE_PROCESSOR:
> > case ACPI_TYPE_THERMAL:
> > diff --git a/drivers/acpi/acpica/utprint.c
> > b/drivers/acpi/acpica/utprint.c index 681c11f4af4e..f7e43baf5ff2
> > 100644
> > --- a/drivers/acpi/acpica/utprint.c
> > +++ b/drivers/acpi/acpica/utprint.c
> > @@ -475,7 +475,7 @@ int vsnprintf(char *string, acpi_size size, const char *format, va_list args)
> > case 'X':
> >
> > type |= ACPI_FORMAT_UPPER;
> > - /* FALLTHROUGH */
> > + fallthrough;
> >
> > case 'x':
> >
> > --
> > 2.29.2.222.g5d2a92d10f8-goog
> >
>
>
> --
> Thanks,
> ~Nick Desaulniers
--
Thanks,
~Nick Desaulniers
On Thu, Nov 12, 2020 at 1:48 PM Moore, Robert <[email protected]> wrote:
>
>
>
> -----Original Message-----
> From: Nick Desaulniers <[email protected]>
> Sent: Thursday, November 12, 2020 11:31 AM
> To: Moore, Robert <[email protected]>
> Cc: Kaneda, Erik <[email protected]>; Wysocki, Rafael J <[email protected]>; Gustavo A . R . Silva <[email protected]>; [email protected]; Len Brown <[email protected]>; [email protected]; [email protected]; [email protected]
> Subject: Re: [PATCH] ACPICA: fix -Wfallthrough
>
> On Thu, Nov 12, 2020 at 7:13 AM Moore, Robert <[email protected]> wrote:
> >
> >
> >
> > -----Original Message-----
> > From: Nick Desaulniers <[email protected]>
> > Sent: Wednesday, November 11, 2020 10:48 AM
> > To: Moore, Robert <[email protected]>
> > Cc: Kaneda, Erik <[email protected]>; Wysocki, Rafael J
> > <[email protected]>; Gustavo A . R . Silva
> > <[email protected]>; [email protected]; Len Brown
> > <[email protected]>; [email protected]; [email protected];
> > [email protected]
> > Subject: Re: [PATCH] ACPICA: fix -Wfallthrough
> >
> > On Wed, Nov 11, 2020 at 7:15 AM Moore, Robert <[email protected]> wrote:
> > >
> > > Yes, but: isn't the "fallthrough" keyword compiler-specific? That is the problem for us.
> >
> > It's not a keyword.
> >
> > It's a preprocessor macro that expands to
> > __attribute__((__fallthrough__)) for compilers that support it. For compilers that do not, it expands to nothing. Both GCC 7+ and Clang support this attribute. Which other compilers that support -Wimplicit-fallthrough do you care to support?
> >
> > We need to support MSVC 2017 -- which apparently does not support this.
>
> In which case, the macro is not expanded to a compiler attribute the compiler doesn't support. Please see also its definition in include/linux/compiler_attributes.h.
>
> From what I can tell, MSVC does not warn on implicit fallthrough, so there's no corresponding attribute (or comment) to disable the warning in MSVC.
>
> That doesn't mean this code is not portable to MSVC; a macro that expands to nothing should not be a problem.
>
> Based on
> https://docs.microsoft.com/en-us/cpp/code-quality/c26819?view=msvc-160
> https://developercommunity.visualstudio.com/idea/423975/issue-compiler-warning-when-using-implicit-fallthr.html
> it sounds like MSVC 2019 will be able to warn, for C++ mode, which will rely on the C++ style attribute to annotate intentional fallthrough.
>
> Can you confirm how this does not work for MSVC 2017?
>
> 1>c:\acpica\source\components\utilities\utdelete.c(270): warning C4013: '__attribute__' undefined; assuming extern returning int
> 1>c:\acpica\source\components\utilities\utdelete.c(270): error C2065: '__fallthrough__': undeclared identifier
> 1>c:\acpica\source\components\utilities\utdelete.c(272): error C2143: syntax error: missing ';' before 'case'
Thank you for the explicit diagnostics observed. Something fishy is
going on though, https://godbolt.org/z/Gbxbxa is how I expect MSVC to
handle include/linux/compiler_attributes.h.
The C preprocessor should make it such that MSVC never sees
`__attribute__` or `__fallthrough__`; that it does begs the question.
That would seem to imply that `#if __has_attribute(__fallthrough__)`
somehow evaluates to true on MSVC, but my godbolt link shows it does
not.
Could the upstream ACPICA project be #define'ing something that could
be altering this? (Or not #define'ing something?)
Worst case, we could do as Joe Perches suggested and disable
-Wfallthrough for drivers/acpi/acpica/.
>
> > > Bob
> > >
> > >
> > > -----Original Message-----
> > > From: ndesaulniers via sendgmr
> > > <[email protected]> On Behalf Of Nick
> > > Desaulniers
> > > Sent: Tuesday, November 10, 2020 6:12 PM
> > > To: Moore, Robert <[email protected]>; Kaneda, Erik
> > > <[email protected]>; Wysocki, Rafael J
> > > <[email protected]>; Gustavo A . R . Silva
> > > <[email protected]>
> > > Cc: [email protected]; Nick Desaulniers
> > > <[email protected]>; Len Brown <[email protected]>;
> > > [email protected]; [email protected];
> > > [email protected]
> > > Subject: [PATCH] ACPICA: fix -Wfallthrough
> > >
> > > The "fallthrough" pseudo-keyword was added as a portable way to denote intentional fallthrough. This code seemed to be using a mix of fallthrough comments that GCC recognizes, and some kind of lint marker.
> > > I'm guessing that linter hasn't been run in a while from the mixed use of the marker vs comments.
> > >
> > > Signed-off-by: Nick Desaulniers <[email protected]>
> > > ---
> > > drivers/acpi/acpica/dscontrol.c | 3 +--
> > > drivers/acpi/acpica/dswexec.c | 4 +---
> > > drivers/acpi/acpica/dswload.c | 3 +--
> > > drivers/acpi/acpica/dswload2.c | 3 +--
> > > drivers/acpi/acpica/exfldio.c | 3 +--
> > > drivers/acpi/acpica/exresop.c | 5 ++---
> > > drivers/acpi/acpica/exstore.c | 6 ++----
> > > drivers/acpi/acpica/hwgpe.c | 3 +--
> > > drivers/acpi/acpica/utdelete.c | 3 +--
> > > drivers/acpi/acpica/utprint.c | 2 +-
> > > 10 files changed, 12 insertions(+), 23 deletions(-)
> > >
> > > diff --git a/drivers/acpi/acpica/dscontrol.c
> > > b/drivers/acpi/acpica/dscontrol.c index 4b5b6e859f62..1e75e5fbfd19
> > > 100644
> > > --- a/drivers/acpi/acpica/dscontrol.c
> > > +++ b/drivers/acpi/acpica/dscontrol.c
> > > @@ -61,8 +61,7 @@ acpi_ds_exec_begin_control_op(struct acpi_walk_state *walk_state,
> > > break;
> > > }
> > > }
> > > -
> > > - /*lint -fallthrough */
> > > + fallthrough;
> > >
> > > case AML_IF_OP:
> > > /*
> > > diff --git a/drivers/acpi/acpica/dswexec.c
> > > b/drivers/acpi/acpica/dswexec.c index 1d4f8c81028c..e8c32d4fe55f
> > > 100644
> > > --- a/drivers/acpi/acpica/dswexec.c
> > > +++ b/drivers/acpi/acpica/dswexec.c
> > > @@ -597,9 +597,7 @@ acpi_status acpi_ds_exec_end_op(struct acpi_walk_state *walk_state)
> > > if (ACPI_FAILURE(status)) {
> > > break;
> > > }
> > > -
> > > - /* Fall through */
> > > - /*lint -fallthrough */
> > > + fallthrough;
> > >
> > > case AML_INT_EVAL_SUBTREE_OP:
> > >
> > > diff --git a/drivers/acpi/acpica/dswload.c
> > > b/drivers/acpi/acpica/dswload.c index 27069325b6de..afc663c3742d
> > > 100644
> > > --- a/drivers/acpi/acpica/dswload.c
> > > +++ b/drivers/acpi/acpica/dswload.c
> > > @@ -223,8 +223,7 @@ acpi_ds_load1_begin_op(struct acpi_walk_state *walk_state,
> > > parse_flags & ACPI_PARSE_MODULE_LEVEL)) {
> > > break;
> > > }
> > > -
> > > - /*lint -fallthrough */
> > > + fallthrough;
> > >
> > > default:
> > >
> > > diff --git a/drivers/acpi/acpica/dswload2.c
> > > b/drivers/acpi/acpica/dswload2.c index edadbe146506..1b794b6ba072
> > > 100644
> > > --- a/drivers/acpi/acpica/dswload2.c
> > > +++ b/drivers/acpi/acpica/dswload2.c
> > > @@ -213,8 +213,7 @@ acpi_ds_load2_begin_op(struct acpi_walk_state *walk_state,
> > > parse_flags & ACPI_PARSE_MODULE_LEVEL)) {
> > > break;
> > > }
> > > -
> > > - /*lint -fallthrough */
> > > + fallthrough;
> > >
> > > default:
> > >
> > > diff --git a/drivers/acpi/acpica/exfldio.c
> > > b/drivers/acpi/acpica/exfldio.c index ade35ff1c7ba..9d1cabe0fed9
> > > 100644
> > > --- a/drivers/acpi/acpica/exfldio.c
> > > +++ b/drivers/acpi/acpica/exfldio.c
> > > @@ -433,8 +433,7 @@ acpi_ex_field_datum_io(union acpi_operand_object *obj_desc,
> > > * Now that the Bank has been selected, fall through to the
> > > * region_field case and write the datum to the Operation Region
> > > */
> > > -
> > > - /*lint -fallthrough */
> > > + fallthrough;
> > >
> > > case ACPI_TYPE_LOCAL_REGION_FIELD:
> > > /*
> > > diff --git a/drivers/acpi/acpica/exresop.c
> > > b/drivers/acpi/acpica/exresop.c index 4d1b22971d58..df48faa9a551
> > > 100644
> > > --- a/drivers/acpi/acpica/exresop.c
> > > +++ b/drivers/acpi/acpica/exresop.c
> > > @@ -197,8 +197,7 @@ acpi_ex_resolve_operands(u16 opcode,
> > > case ACPI_REFCLASS_DEBUG:
> > >
> > > target_op = AML_DEBUG_OP;
> > > -
> > > - /*lint -fallthrough */
> > > + fallthrough;
> > >
> > > case ACPI_REFCLASS_ARG:
> > > case ACPI_REFCLASS_LOCAL:
> > > @@ -264,7 +263,7 @@ acpi_ex_resolve_operands(u16 opcode,
> > > * Else not a string - fall through to the normal Reference
> > > * case below
> > > */
> > > - /*lint -fallthrough */
> > > + 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..2067baa7c120
> > > 100644
> > > --- a/drivers/acpi/acpica/exstore.c
> > > +++ b/drivers/acpi/acpica/exstore.c
> > > @@ -95,8 +95,7 @@ acpi_ex_store(union acpi_operand_object *source_desc,
> > > if (dest_desc->common.flags & AOPOBJ_AML_CONSTANT) {
> > > return_ACPI_STATUS(AE_OK);
> > > }
> > > -
> > > - /*lint -fallthrough */
> > > + fallthrough;
> > >
> > > default:
> > >
> > > @@ -421,8 +420,7 @@ acpi_ex_store_object_to_node(union acpi_operand_object *source_desc,
> > > }
> > > break;
> > > }
> > > -
> > > - /* Fallthrough */
> > > + 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..fbfad80c8a53
> > > 100644
> > > --- a/drivers/acpi/acpica/hwgpe.c
> > > +++ b/drivers/acpi/acpica/hwgpe.c
> > > @@ -166,8 +166,7 @@ acpi_hw_low_set_gpe(struct acpi_gpe_event_info *gpe_event_info, u32 action)
> > > if (!(register_bit & gpe_register_info->enable_mask)) {
> > > return (AE_BAD_PARAMETER);
> > > }
> > > -
> > > - /*lint -fallthrough */
> > > + fallthrough;
> > >
> > > case ACPI_GPE_ENABLE:
> > >
> > > diff --git a/drivers/acpi/acpica/utdelete.c
> > > b/drivers/acpi/acpica/utdelete.c index 4c0d4e434196..8076e7947585
> > > 100644
> > > --- a/drivers/acpi/acpica/utdelete.c
> > > +++ b/drivers/acpi/acpica/utdelete.c
> > > @@ -111,8 +111,7 @@ static void acpi_ut_delete_internal_obj(union acpi_operand_object *object)
> > > (void)acpi_ev_delete_gpe_block(object->device.
> > > gpe_block);
> > > }
> > > -
> > > - /*lint -fallthrough */
> > > + fallthrough;
> > >
> > > case ACPI_TYPE_PROCESSOR:
> > > case ACPI_TYPE_THERMAL:
> > > diff --git a/drivers/acpi/acpica/utprint.c
> > > b/drivers/acpi/acpica/utprint.c index 681c11f4af4e..f7e43baf5ff2
> > > 100644
> > > --- a/drivers/acpi/acpica/utprint.c
> > > +++ b/drivers/acpi/acpica/utprint.c
> > > @@ -475,7 +475,7 @@ int vsnprintf(char *string, acpi_size size, const char *format, va_list args)
> > > case 'X':
> > >
> > > type |= ACPI_FORMAT_UPPER;
> > > - /* FALLTHROUGH */
> > > + fallthrough;
> > >
> > > case 'x':
> > >
> > > --
> > > 2.29.2.222.g5d2a92d10f8-goog
> > >
> >
> >
> > --
> > Thanks,
> > ~Nick Desaulniers
>
>
>
> --
> Thanks,
> ~Nick Desaulniers
--
Thanks,
~Nick Desaulniers
On Thu, Nov 12, 2020 at 10:49 PM Moore, Robert <[email protected]> wrote:
>
> 1>c:\acpica\source\components\utilities\utdelete.c(270): warning C4013: '__attribute__' undefined; assuming extern returning int
> 1>c:\acpica\source\components\utilities\utdelete.c(270): error C2065: '__fallthrough__': undeclared identifier
> 1>c:\acpica\source\components\utilities\utdelete.c(272): error C2143: syntax error: missing ';' before 'case'
Can you share a minimized sample with the `cl` version and command-line options?
Cheers,
Miguel
On Fri, Nov 13, 2020 at 1:09 AM Nick Desaulniers
<[email protected]> wrote:
>
> Thank you for the explicit diagnostics observed. Something fishy is
> going on though, https://godbolt.org/z/Gbxbxa is how I expect MSVC to
> handle include/linux/compiler_attributes.h.
>
> The C preprocessor should make it such that MSVC never sees
> `__attribute__` or `__fallthrough__`; that it does begs the question.
> That would seem to imply that `#if __has_attribute(__fallthrough__)`
> somehow evaluates to true on MSVC, but my godbolt link shows it does
> not.
>
> Could the upstream ACPICA project be #define'ing something that could
> be altering this? (Or not #define'ing something?)
>
> Worst case, we could do as Joe Perches suggested and disable
> -Wfallthrough for drivers/acpi/acpica/.
I agree, something is fishy. MSVC has several flags for conformance
and extensions support, including two full C preprocessors in newer
versions; which means we might be missing something, but I don't see
how the code in compiler_attributes.h could be confusing MSVC even in
older non-conforming versions.
Cheers,
Miguel
On Fri, 2020-11-13 at 09:14 +0100, Miguel Ojeda wrote:
> On Fri, Nov 13, 2020 at 1:09 AM Nick Desaulniers
> <[email protected]> wrote:
> >
> > Thank you for the explicit diagnostics observed. Something fishy is
> > going on though, https://godbolt.org/z/Gbxbxa is how I expect MSVC to
> > handle include/linux/compiler_attributes.h.
> >
> > The C preprocessor should make it such that MSVC never sees
> > `__attribute__` or `__fallthrough__`; that it does begs the question.
> > That would seem to imply that `#if __has_attribute(__fallthrough__)`
> > somehow evaluates to true on MSVC, but my godbolt link shows it does
> > not.
> >
> > Could the upstream ACPICA project be #define'ing something that could
> > be altering this? (Or not #define'ing something?)
> >
> > Worst case, we could do as Joe Perches suggested and disable
> > -Wfallthrough for drivers/acpi/acpica/.
>
> I agree, something is fishy. MSVC has several flags for conformance
> and extensions support, including two full C preprocessors in newer
> versions; which means we might be missing something, but I don't see
> how the code in compiler_attributes.h could be confusing MSVC even in
> older non-conforming versions.
I believe this has nothing to do with linux and only
to do with compiling acpica for other environments
like Windows.
From: https://acpica.org/
The ACPI Component Architecture (ACPICA) project provides an
operating system (OS)-independent reference implementation of the
Advanced Configuration and Power Interface Specification (ACPI).
It can be easily adapted to execute under any host OS.
On Fri, Nov 13, 2020 at 12:14 AM Miguel Ojeda
<[email protected]> wrote:
>
> On Fri, Nov 13, 2020 at 1:09 AM Nick Desaulniers
> <[email protected]> wrote:
> >
> > Thank you for the explicit diagnostics observed. Something fishy is
> > going on though, https://godbolt.org/z/Gbxbxa is how I expect MSVC to
> > handle include/linux/compiler_attributes.h.
> >
> > The C preprocessor should make it such that MSVC never sees
> > `__attribute__` or `__fallthrough__`; that it does begs the question.
> > That would seem to imply that `#if __has_attribute(__fallthrough__)`
> > somehow evaluates to true on MSVC, but my godbolt link shows it does
> > not.
> >
> > Could the upstream ACPICA project be #define'ing something that could
> > be altering this? (Or not #define'ing something?)
> >
> > Worst case, we could do as Joe Perches suggested and disable
> > -Wfallthrough for drivers/acpi/acpica/.
>
> I agree, something is fishy. MSVC has several flags for conformance
> and extensions support, including two full C preprocessors in newer
> versions; which means we might be missing something, but I don't see
> how the code in compiler_attributes.h could be confusing MSVC even in
> older non-conforming versions.
unless
```
# define fallthrough __attribute__((__fallthrough__))
```
was copy and pasted into the code, rather than #including the whole header?
--
Thanks,
~Nick Desaulniers
I can do it this way:
In the global header actypes.h:
#ifndef ACPI_FALLTHROUGH
#define ACPI_FALLTHROUGH
#endif
In the gcc-specific header (acgcc.h):
#define ACPI_FALLTHROUGH __attribute__((__fallthrough__))
This would not be #defined in the MSVC-specific header (acmsvc.h) -- thus using the default (null) in actypes.h (The per-environment headers are always included first).
(We do all macros in upper case, prefixed with "ACPI_")
If you can update your patch to use ACPI_FALLTHROUGH, I can do the rest (above).
Thanks,
Bob
-----Original Message-----
From: Joe Perches <[email protected]>
Sent: Friday, November 13, 2020 8:30 AM
To: Miguel Ojeda <[email protected]>; Nick Desaulniers <[email protected]>
Cc: Moore, Robert <[email protected]>; Kaneda, Erik <[email protected]>; Wysocki, Rafael J <[email protected]>; Gustavo A . R . Silva <[email protected]>; [email protected]; Len Brown <[email protected]>; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH] ACPICA: fix -Wfallthrough
On Fri, 2020-11-13 at 09:14 +0100, Miguel Ojeda wrote:
> On Fri, Nov 13, 2020 at 1:09 AM Nick Desaulniers
> <[email protected]> wrote:
> >
> > Thank you for the explicit diagnostics observed. Something fishy is
> > going on though, https://godbolt.org/z/Gbxbxa is how I expect MSVC
> > to handle include/linux/compiler_attributes.h.
> >
> > The C preprocessor should make it such that MSVC never sees
> > `__attribute__` or `__fallthrough__`; that it does begs the question.
> > That would seem to imply that `#if __has_attribute(__fallthrough__)`
> > somehow evaluates to true on MSVC, but my godbolt link shows it does
> > not.
> >
> > Could the upstream ACPICA project be #define'ing something that
> > could be altering this? (Or not #define'ing something?)
> >
> > Worst case, we could do as Joe Perches suggested and disable
> > -Wfallthrough for drivers/acpi/acpica/.
>
> I agree, something is fishy. MSVC has several flags for conformance
> and extensions support, including two full C preprocessors in newer
> versions; which means we might be missing something, but I don't see
> how the code in compiler_attributes.h could be confusing MSVC even in
> older non-conforming versions.
I believe this has nothing to do with linux and only to do with compiling acpica for other environments like Windows.
From: https://acpica.org/
The ACPI Component Architecture (ACPICA) project provides an operating system (OS)-independent reference implementation of the Advanced Configuration and Power Interface Specification (ACPI).
It can be easily adapted to execute under any host OS.
On Fri, Nov 13, 2020 at 1:01 PM Moore, Robert <[email protected]> wrote:
>
> I can do it this way:
>
> In the global header actypes.h:
>
> #ifndef ACPI_FALLTHROUGH
> #define ACPI_FALLTHROUGH
> #endif
>
> In the gcc-specific header (acgcc.h):
>
> #define ACPI_FALLTHROUGH __attribute__((__fallthrough__))
>
> This would not be #defined in the MSVC-specific header (acmsvc.h) -- thus using the default (null) in actypes.h (The per-environment headers are always included first).
>
> (We do all macros in upper case, prefixed with "ACPI_")
>
> If you can update your patch to use ACPI_FALLTHROUGH, I can do the rest (above).
Sure, I can do that. I'd need to wrap it in a little more logic for
__has_attribute to support old GCC versions, but that should be
doable.
--
Thanks,
~Nick Desaulniers
-----Original Message-----
From: ndesaulniers via sendgmr <[email protected]> On Behalf Of Nick Desaulniers
Sent: Tuesday, November 10, 2020 6:12 PM
To: Moore, Robert <[email protected]>; Kaneda, Erik <[email protected]>; Wysocki, Rafael J <[email protected]>; Gustavo A . R . Silva <[email protected]>
Cc: [email protected]; Nick Desaulniers <[email protected]>; Len Brown <[email protected]>; [email protected]; [email protected]; [email protected]
Subject: [PATCH] ACPICA: fix -Wfallthrough
The "fallthrough" pseudo-keyword was added as a portable way to denote intentional fallthrough. This code seemed to be using a mix of fallthrough comments that GCC recognizes, and some kind of lint marker.
I'm guessing that linter hasn't been run in a while from the mixed use of the marker vs comments.
/*lint -fallthrough */
This is the lint marker
BTW, what version of gcc added -Wfallthrough?
Signed-off-by: Nick Desaulniers <[email protected]>
---
drivers/acpi/acpica/dscontrol.c | 3 +--
drivers/acpi/acpica/dswexec.c | 4 +---
drivers/acpi/acpica/dswload.c | 3 +--
drivers/acpi/acpica/dswload2.c | 3 +--
drivers/acpi/acpica/exfldio.c | 3 +--
drivers/acpi/acpica/exresop.c | 5 ++---
drivers/acpi/acpica/exstore.c | 6 ++----
drivers/acpi/acpica/hwgpe.c | 3 +--
drivers/acpi/acpica/utdelete.c | 3 +--
drivers/acpi/acpica/utprint.c | 2 +-
10 files changed, 12 insertions(+), 23 deletions(-)
diff --git a/drivers/acpi/acpica/dscontrol.c b/drivers/acpi/acpica/dscontrol.c index 4b5b6e859f62..1e75e5fbfd19 100644
--- a/drivers/acpi/acpica/dscontrol.c
+++ b/drivers/acpi/acpica/dscontrol.c
@@ -61,8 +61,7 @@ acpi_ds_exec_begin_control_op(struct acpi_walk_state *walk_state,
break;
}
}
-
- /*lint -fallthrough */
+ fallthrough;
case AML_IF_OP:
/*
diff --git a/drivers/acpi/acpica/dswexec.c b/drivers/acpi/acpica/dswexec.c index 1d4f8c81028c..e8c32d4fe55f 100644
--- a/drivers/acpi/acpica/dswexec.c
+++ b/drivers/acpi/acpica/dswexec.c
@@ -597,9 +597,7 @@ acpi_status acpi_ds_exec_end_op(struct acpi_walk_state *walk_state)
if (ACPI_FAILURE(status)) {
break;
}
-
- /* Fall through */
- /*lint -fallthrough */
+ fallthrough;
case AML_INT_EVAL_SUBTREE_OP:
diff --git a/drivers/acpi/acpica/dswload.c b/drivers/acpi/acpica/dswload.c index 27069325b6de..afc663c3742d 100644
--- a/drivers/acpi/acpica/dswload.c
+++ b/drivers/acpi/acpica/dswload.c
@@ -223,8 +223,7 @@ acpi_ds_load1_begin_op(struct acpi_walk_state *walk_state,
parse_flags & ACPI_PARSE_MODULE_LEVEL)) {
break;
}
-
- /*lint -fallthrough */
+ fallthrough;
default:
diff --git a/drivers/acpi/acpica/dswload2.c b/drivers/acpi/acpica/dswload2.c index edadbe146506..1b794b6ba072 100644
--- a/drivers/acpi/acpica/dswload2.c
+++ b/drivers/acpi/acpica/dswload2.c
@@ -213,8 +213,7 @@ acpi_ds_load2_begin_op(struct acpi_walk_state *walk_state,
parse_flags & ACPI_PARSE_MODULE_LEVEL)) {
break;
}
-
- /*lint -fallthrough */
+ fallthrough;
default:
diff --git a/drivers/acpi/acpica/exfldio.c b/drivers/acpi/acpica/exfldio.c index ade35ff1c7ba..9d1cabe0fed9 100644
--- a/drivers/acpi/acpica/exfldio.c
+++ b/drivers/acpi/acpica/exfldio.c
@@ -433,8 +433,7 @@ acpi_ex_field_datum_io(union acpi_operand_object *obj_desc,
* Now that the Bank has been selected, fall through to the
* region_field case and write the datum to the Operation Region
*/
-
- /*lint -fallthrough */
+ fallthrough;
case ACPI_TYPE_LOCAL_REGION_FIELD:
/*
diff --git a/drivers/acpi/acpica/exresop.c b/drivers/acpi/acpica/exresop.c index 4d1b22971d58..df48faa9a551 100644
--- a/drivers/acpi/acpica/exresop.c
+++ b/drivers/acpi/acpica/exresop.c
@@ -197,8 +197,7 @@ acpi_ex_resolve_operands(u16 opcode,
case ACPI_REFCLASS_DEBUG:
target_op = AML_DEBUG_OP;
-
- /*lint -fallthrough */
+ fallthrough;
case ACPI_REFCLASS_ARG:
case ACPI_REFCLASS_LOCAL:
@@ -264,7 +263,7 @@ acpi_ex_resolve_operands(u16 opcode,
* Else not a string - fall through to the normal Reference
* case below
*/
- /*lint -fallthrough */
+ 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..2067baa7c120 100644
--- a/drivers/acpi/acpica/exstore.c
+++ b/drivers/acpi/acpica/exstore.c
@@ -95,8 +95,7 @@ acpi_ex_store(union acpi_operand_object *source_desc,
if (dest_desc->common.flags & AOPOBJ_AML_CONSTANT) {
return_ACPI_STATUS(AE_OK);
}
-
- /*lint -fallthrough */
+ fallthrough;
default:
@@ -421,8 +420,7 @@ acpi_ex_store_object_to_node(union acpi_operand_object *source_desc,
}
break;
}
-
- /* Fallthrough */
+ 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..fbfad80c8a53 100644
--- a/drivers/acpi/acpica/hwgpe.c
+++ b/drivers/acpi/acpica/hwgpe.c
@@ -166,8 +166,7 @@ acpi_hw_low_set_gpe(struct acpi_gpe_event_info *gpe_event_info, u32 action)
if (!(register_bit & gpe_register_info->enable_mask)) {
return (AE_BAD_PARAMETER);
}
-
- /*lint -fallthrough */
+ fallthrough;
case ACPI_GPE_ENABLE:
diff --git a/drivers/acpi/acpica/utdelete.c b/drivers/acpi/acpica/utdelete.c index 4c0d4e434196..8076e7947585 100644
--- a/drivers/acpi/acpica/utdelete.c
+++ b/drivers/acpi/acpica/utdelete.c
@@ -111,8 +111,7 @@ static void acpi_ut_delete_internal_obj(union acpi_operand_object *object)
(void)acpi_ev_delete_gpe_block(object->device.
gpe_block);
}
-
- /*lint -fallthrough */
+ fallthrough;
case ACPI_TYPE_PROCESSOR:
case ACPI_TYPE_THERMAL:
diff --git a/drivers/acpi/acpica/utprint.c b/drivers/acpi/acpica/utprint.c index 681c11f4af4e..f7e43baf5ff2 100644
--- a/drivers/acpi/acpica/utprint.c
+++ b/drivers/acpi/acpica/utprint.c
@@ -475,7 +475,7 @@ int vsnprintf(char *string, acpi_size size, const char *format, va_list args)
case 'X':
type |= ACPI_FORMAT_UPPER;
- /* FALLTHROUGH */
+ fallthrough;
case 'x':
--
2.29.2.222.g5d2a92d10f8-goog
On Fri, Nov 13, 2020 at 1:27 PM Moore, Robert <[email protected]> wrote:
>
>
>
> -----Original Message-----
> From: ndesaulniers via sendgmr <[email protected]> On Behalf Of Nick Desaulniers
> Sent: Tuesday, November 10, 2020 6:12 PM
> To: Moore, Robert <[email protected]>; Kaneda, Erik <[email protected]>; Wysocki, Rafael J <[email protected]>; Gustavo A . R . Silva <[email protected]>
> Cc: [email protected]; Nick Desaulniers <[email protected]>; Len Brown <[email protected]>; [email protected]; [email protected]; [email protected]
> Subject: [PATCH] ACPICA: fix -Wfallthrough
>
> The "fallthrough" pseudo-keyword was added as a portable way to denote intentional fallthrough. This code seemed to be using a mix of fallthrough comments that GCC recognizes, and some kind of lint marker.
> I'm guessing that linter hasn't been run in a while from the mixed use of the marker vs comments.
>
> /*lint -fallthrough */
>
> This is the lint marker
Yes; but from my patch, the hunk modifying
acpi_ex_store_object_to_node() and vsnprintf() seem to indicate that
maybe the linter hasn't been run in a while.
Which linter is that? I'm curious whether I should leave those be,
and whether we're going to have an issue between compilers and linters
as to which line/order these would need to appear on.
>
> BTW, what version of gcc added -Wfallthrough?
GCC 7.1 added -Wimplicit-fallthrough.
>
>
> Signed-off-by: Nick Desaulniers <[email protected]>
> ---
> drivers/acpi/acpica/dscontrol.c | 3 +--
> drivers/acpi/acpica/dswexec.c | 4 +---
> drivers/acpi/acpica/dswload.c | 3 +--
> drivers/acpi/acpica/dswload2.c | 3 +--
> drivers/acpi/acpica/exfldio.c | 3 +--
> drivers/acpi/acpica/exresop.c | 5 ++---
> drivers/acpi/acpica/exstore.c | 6 ++----
> drivers/acpi/acpica/hwgpe.c | 3 +--
> drivers/acpi/acpica/utdelete.c | 3 +--
> drivers/acpi/acpica/utprint.c | 2 +-
> 10 files changed, 12 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/acpi/acpica/dscontrol.c b/drivers/acpi/acpica/dscontrol.c index 4b5b6e859f62..1e75e5fbfd19 100644
> --- a/drivers/acpi/acpica/dscontrol.c
> +++ b/drivers/acpi/acpica/dscontrol.c
> @@ -61,8 +61,7 @@ acpi_ds_exec_begin_control_op(struct acpi_walk_state *walk_state,
> break;
> }
> }
> -
> - /*lint -fallthrough */
> + fallthrough;
>
> case AML_IF_OP:
> /*
> diff --git a/drivers/acpi/acpica/dswexec.c b/drivers/acpi/acpica/dswexec.c index 1d4f8c81028c..e8c32d4fe55f 100644
> --- a/drivers/acpi/acpica/dswexec.c
> +++ b/drivers/acpi/acpica/dswexec.c
> @@ -597,9 +597,7 @@ acpi_status acpi_ds_exec_end_op(struct acpi_walk_state *walk_state)
> if (ACPI_FAILURE(status)) {
> break;
> }
> -
> - /* Fall through */
> - /*lint -fallthrough */
> + fallthrough;
>
> case AML_INT_EVAL_SUBTREE_OP:
>
> diff --git a/drivers/acpi/acpica/dswload.c b/drivers/acpi/acpica/dswload.c index 27069325b6de..afc663c3742d 100644
> --- a/drivers/acpi/acpica/dswload.c
> +++ b/drivers/acpi/acpica/dswload.c
> @@ -223,8 +223,7 @@ acpi_ds_load1_begin_op(struct acpi_walk_state *walk_state,
> parse_flags & ACPI_PARSE_MODULE_LEVEL)) {
> break;
> }
> -
> - /*lint -fallthrough */
> + fallthrough;
>
> default:
>
> diff --git a/drivers/acpi/acpica/dswload2.c b/drivers/acpi/acpica/dswload2.c index edadbe146506..1b794b6ba072 100644
> --- a/drivers/acpi/acpica/dswload2.c
> +++ b/drivers/acpi/acpica/dswload2.c
> @@ -213,8 +213,7 @@ acpi_ds_load2_begin_op(struct acpi_walk_state *walk_state,
> parse_flags & ACPI_PARSE_MODULE_LEVEL)) {
> break;
> }
> -
> - /*lint -fallthrough */
> + fallthrough;
>
> default:
>
> diff --git a/drivers/acpi/acpica/exfldio.c b/drivers/acpi/acpica/exfldio.c index ade35ff1c7ba..9d1cabe0fed9 100644
> --- a/drivers/acpi/acpica/exfldio.c
> +++ b/drivers/acpi/acpica/exfldio.c
> @@ -433,8 +433,7 @@ acpi_ex_field_datum_io(union acpi_operand_object *obj_desc,
> * Now that the Bank has been selected, fall through to the
> * region_field case and write the datum to the Operation Region
> */
> -
> - /*lint -fallthrough */
> + fallthrough;
>
> case ACPI_TYPE_LOCAL_REGION_FIELD:
> /*
> diff --git a/drivers/acpi/acpica/exresop.c b/drivers/acpi/acpica/exresop.c index 4d1b22971d58..df48faa9a551 100644
> --- a/drivers/acpi/acpica/exresop.c
> +++ b/drivers/acpi/acpica/exresop.c
> @@ -197,8 +197,7 @@ acpi_ex_resolve_operands(u16 opcode,
> case ACPI_REFCLASS_DEBUG:
>
> target_op = AML_DEBUG_OP;
> -
> - /*lint -fallthrough */
> + fallthrough;
>
> case ACPI_REFCLASS_ARG:
> case ACPI_REFCLASS_LOCAL:
> @@ -264,7 +263,7 @@ acpi_ex_resolve_operands(u16 opcode,
> * Else not a string - fall through to the normal Reference
> * case below
> */
> - /*lint -fallthrough */
> + 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..2067baa7c120 100644
> --- a/drivers/acpi/acpica/exstore.c
> +++ b/drivers/acpi/acpica/exstore.c
> @@ -95,8 +95,7 @@ acpi_ex_store(union acpi_operand_object *source_desc,
> if (dest_desc->common.flags & AOPOBJ_AML_CONSTANT) {
> return_ACPI_STATUS(AE_OK);
> }
> -
> - /*lint -fallthrough */
> + fallthrough;
>
> default:
>
> @@ -421,8 +420,7 @@ acpi_ex_store_object_to_node(union acpi_operand_object *source_desc,
> }
> break;
> }
> -
> - /* Fallthrough */
> + 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..fbfad80c8a53 100644
> --- a/drivers/acpi/acpica/hwgpe.c
> +++ b/drivers/acpi/acpica/hwgpe.c
> @@ -166,8 +166,7 @@ acpi_hw_low_set_gpe(struct acpi_gpe_event_info *gpe_event_info, u32 action)
> if (!(register_bit & gpe_register_info->enable_mask)) {
> return (AE_BAD_PARAMETER);
> }
> -
> - /*lint -fallthrough */
> + fallthrough;
>
> case ACPI_GPE_ENABLE:
>
> diff --git a/drivers/acpi/acpica/utdelete.c b/drivers/acpi/acpica/utdelete.c index 4c0d4e434196..8076e7947585 100644
> --- a/drivers/acpi/acpica/utdelete.c
> +++ b/drivers/acpi/acpica/utdelete.c
> @@ -111,8 +111,7 @@ static void acpi_ut_delete_internal_obj(union acpi_operand_object *object)
> (void)acpi_ev_delete_gpe_block(object->device.
> gpe_block);
> }
> -
> - /*lint -fallthrough */
> + fallthrough;
>
> case ACPI_TYPE_PROCESSOR:
> case ACPI_TYPE_THERMAL:
> diff --git a/drivers/acpi/acpica/utprint.c b/drivers/acpi/acpica/utprint.c index 681c11f4af4e..f7e43baf5ff2 100644
> --- a/drivers/acpi/acpica/utprint.c
> +++ b/drivers/acpi/acpica/utprint.c
> @@ -475,7 +475,7 @@ int vsnprintf(char *string, acpi_size size, const char *format, va_list args)
> case 'X':
>
> type |= ACPI_FORMAT_UPPER;
> - /* FALLTHROUGH */
> + fallthrough;
>
> case 'x':
>
> --
> 2.29.2.222.g5d2a92d10f8-goog
>
--
Thanks,
~Nick Desaulniers
-----Original Message-----
From: Nick Desaulniers <[email protected]>
Sent: Friday, November 13, 2020 1:33 PM
To: Moore, Robert <[email protected]>
Cc: Kaneda, Erik <[email protected]>; Wysocki, Rafael J <[email protected]>; Gustavo A . R . Silva <[email protected]>; [email protected]; Len Brown <[email protected]>; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH] ACPICA: fix -Wfallthrough
On Fri, Nov 13, 2020 at 1:27 PM Moore, Robert <[email protected]> wrote:
>
>
>
> -----Original Message-----
> From: ndesaulniers via sendgmr
> <[email protected]> On Behalf Of Nick
> Desaulniers
> Sent: Tuesday, November 10, 2020 6:12 PM
> To: Moore, Robert <[email protected]>; Kaneda, Erik
> <[email protected]>; Wysocki, Rafael J
> <[email protected]>; Gustavo A . R . Silva
> <[email protected]>
> Cc: [email protected]; Nick Desaulniers
> <[email protected]>; Len Brown <[email protected]>;
> [email protected]; [email protected];
> [email protected]
> Subject: [PATCH] ACPICA: fix -Wfallthrough
>
> The "fallthrough" pseudo-keyword was added as a portable way to denote intentional fallthrough. This code seemed to be using a mix of fallthrough comments that GCC recognizes, and some kind of lint marker.
> I'm guessing that linter hasn't been run in a while from the mixed use of the marker vs comments.
>
> /*lint -fallthrough */
>
> This is the lint marker
Yes; but from my patch, the hunk modifying
acpi_ex_store_object_to_node() and vsnprintf() seem to indicate that maybe the linter hasn't been run in a while.
Which linter is that? I'm curious whether I should leave those be, and whether we're going to have an issue between compilers and linters as to which line/order these would need to appear on.
It's an old version of PC-Lint, which we don't use anymore.
>
> BTW, what version of gcc added -Wfallthrough?
GCC 7.1 added -Wimplicit-fallthrough.
>
>
> Signed-off-by: Nick Desaulniers <[email protected]>
> ---
> drivers/acpi/acpica/dscontrol.c | 3 +--
> drivers/acpi/acpica/dswexec.c | 4 +---
> drivers/acpi/acpica/dswload.c | 3 +--
> drivers/acpi/acpica/dswload2.c | 3 +--
> drivers/acpi/acpica/exfldio.c | 3 +--
> drivers/acpi/acpica/exresop.c | 5 ++---
> drivers/acpi/acpica/exstore.c | 6 ++----
> drivers/acpi/acpica/hwgpe.c | 3 +--
> drivers/acpi/acpica/utdelete.c | 3 +--
> drivers/acpi/acpica/utprint.c | 2 +-
> 10 files changed, 12 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/acpi/acpica/dscontrol.c
> b/drivers/acpi/acpica/dscontrol.c index 4b5b6e859f62..1e75e5fbfd19
> 100644
> --- a/drivers/acpi/acpica/dscontrol.c
> +++ b/drivers/acpi/acpica/dscontrol.c
> @@ -61,8 +61,7 @@ acpi_ds_exec_begin_control_op(struct acpi_walk_state *walk_state,
> break;
> }
> }
> -
> - /*lint -fallthrough */
> + fallthrough;
>
> case AML_IF_OP:
> /*
> diff --git a/drivers/acpi/acpica/dswexec.c
> b/drivers/acpi/acpica/dswexec.c index 1d4f8c81028c..e8c32d4fe55f
> 100644
> --- a/drivers/acpi/acpica/dswexec.c
> +++ b/drivers/acpi/acpica/dswexec.c
> @@ -597,9 +597,7 @@ acpi_status acpi_ds_exec_end_op(struct acpi_walk_state *walk_state)
> if (ACPI_FAILURE(status)) {
> break;
> }
> -
> - /* Fall through */
> - /*lint -fallthrough */
> + fallthrough;
>
> case AML_INT_EVAL_SUBTREE_OP:
>
> diff --git a/drivers/acpi/acpica/dswload.c
> b/drivers/acpi/acpica/dswload.c index 27069325b6de..afc663c3742d
> 100644
> --- a/drivers/acpi/acpica/dswload.c
> +++ b/drivers/acpi/acpica/dswload.c
> @@ -223,8 +223,7 @@ acpi_ds_load1_begin_op(struct acpi_walk_state *walk_state,
> parse_flags & ACPI_PARSE_MODULE_LEVEL)) {
> break;
> }
> -
> - /*lint -fallthrough */
> + fallthrough;
>
> default:
>
> diff --git a/drivers/acpi/acpica/dswload2.c
> b/drivers/acpi/acpica/dswload2.c index edadbe146506..1b794b6ba072
> 100644
> --- a/drivers/acpi/acpica/dswload2.c
> +++ b/drivers/acpi/acpica/dswload2.c
> @@ -213,8 +213,7 @@ acpi_ds_load2_begin_op(struct acpi_walk_state *walk_state,
> parse_flags & ACPI_PARSE_MODULE_LEVEL)) {
> break;
> }
> -
> - /*lint -fallthrough */
> + fallthrough;
>
> default:
>
> diff --git a/drivers/acpi/acpica/exfldio.c
> b/drivers/acpi/acpica/exfldio.c index ade35ff1c7ba..9d1cabe0fed9
> 100644
> --- a/drivers/acpi/acpica/exfldio.c
> +++ b/drivers/acpi/acpica/exfldio.c
> @@ -433,8 +433,7 @@ acpi_ex_field_datum_io(union acpi_operand_object *obj_desc,
> * Now that the Bank has been selected, fall through to the
> * region_field case and write the datum to the Operation Region
> */
> -
> - /*lint -fallthrough */
> + fallthrough;
>
> case ACPI_TYPE_LOCAL_REGION_FIELD:
> /*
> diff --git a/drivers/acpi/acpica/exresop.c
> b/drivers/acpi/acpica/exresop.c index 4d1b22971d58..df48faa9a551
> 100644
> --- a/drivers/acpi/acpica/exresop.c
> +++ b/drivers/acpi/acpica/exresop.c
> @@ -197,8 +197,7 @@ acpi_ex_resolve_operands(u16 opcode,
> case ACPI_REFCLASS_DEBUG:
>
> target_op = AML_DEBUG_OP;
> -
> - /*lint -fallthrough */
> + fallthrough;
>
> case ACPI_REFCLASS_ARG:
> case ACPI_REFCLASS_LOCAL:
> @@ -264,7 +263,7 @@ acpi_ex_resolve_operands(u16 opcode,
> * Else not a string - fall through to the normal Reference
> * case below
> */
> - /*lint -fallthrough */
> + 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..2067baa7c120
> 100644
> --- a/drivers/acpi/acpica/exstore.c
> +++ b/drivers/acpi/acpica/exstore.c
> @@ -95,8 +95,7 @@ acpi_ex_store(union acpi_operand_object *source_desc,
> if (dest_desc->common.flags & AOPOBJ_AML_CONSTANT) {
> return_ACPI_STATUS(AE_OK);
> }
> -
> - /*lint -fallthrough */
> + fallthrough;
>
> default:
>
> @@ -421,8 +420,7 @@ acpi_ex_store_object_to_node(union acpi_operand_object *source_desc,
> }
> break;
> }
> -
> - /* Fallthrough */
> + 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..fbfad80c8a53 100644
> --- a/drivers/acpi/acpica/hwgpe.c
> +++ b/drivers/acpi/acpica/hwgpe.c
> @@ -166,8 +166,7 @@ acpi_hw_low_set_gpe(struct acpi_gpe_event_info *gpe_event_info, u32 action)
> if (!(register_bit & gpe_register_info->enable_mask)) {
> return (AE_BAD_PARAMETER);
> }
> -
> - /*lint -fallthrough */
> + fallthrough;
>
> case ACPI_GPE_ENABLE:
>
> diff --git a/drivers/acpi/acpica/utdelete.c
> b/drivers/acpi/acpica/utdelete.c index 4c0d4e434196..8076e7947585
> 100644
> --- a/drivers/acpi/acpica/utdelete.c
> +++ b/drivers/acpi/acpica/utdelete.c
> @@ -111,8 +111,7 @@ static void acpi_ut_delete_internal_obj(union acpi_operand_object *object)
> (void)acpi_ev_delete_gpe_block(object->device.
> gpe_block);
> }
> -
> - /*lint -fallthrough */
> + fallthrough;
>
> case ACPI_TYPE_PROCESSOR:
> case ACPI_TYPE_THERMAL:
> diff --git a/drivers/acpi/acpica/utprint.c
> b/drivers/acpi/acpica/utprint.c index 681c11f4af4e..f7e43baf5ff2
> 100644
> --- a/drivers/acpi/acpica/utprint.c
> +++ b/drivers/acpi/acpica/utprint.c
> @@ -475,7 +475,7 @@ int vsnprintf(char *string, acpi_size size, const char *format, va_list args)
> case 'X':
>
> type |= ACPI_FORMAT_UPPER;
> - /* FALLTHROUGH */
> + fallthrough;
>
> case 'x':
>
> --
> 2.29.2.222.g5d2a92d10f8-goog
>
--
Thanks,
~Nick Desaulniers
On Fri, Nov 13, 2020 at 1:42 PM Moore, Robert <[email protected]> wrote:
>
>
>
> -----Original Message-----
> From: Nick Desaulniers <[email protected]>
> Sent: Friday, November 13, 2020 1:33 PM
> To: Moore, Robert <[email protected]>
> Cc: Kaneda, Erik <[email protected]>; Wysocki, Rafael J <[email protected]>; Gustavo A . R . Silva <[email protected]>; [email protected]; Len Brown <[email protected]>; [email protected]; [email protected]; [email protected]
> Subject: Re: [PATCH] ACPICA: fix -Wfallthrough
>
> On Fri, Nov 13, 2020 at 1:27 PM Moore, Robert <[email protected]> wrote:
> >
> >
> >
> > -----Original Message-----
> > From: ndesaulniers via sendgmr
> > <[email protected]> On Behalf Of Nick
> > Desaulniers
> > Sent: Tuesday, November 10, 2020 6:12 PM
> > To: Moore, Robert <[email protected]>; Kaneda, Erik
> > <[email protected]>; Wysocki, Rafael J
> > <[email protected]>; Gustavo A . R . Silva
> > <[email protected]>
> > Cc: [email protected]; Nick Desaulniers
> > <[email protected]>; Len Brown <[email protected]>;
> > [email protected]; [email protected];
> > [email protected]
> > Subject: [PATCH] ACPICA: fix -Wfallthrough
> >
> > The "fallthrough" pseudo-keyword was added as a portable way to denote intentional fallthrough. This code seemed to be using a mix of fallthrough comments that GCC recognizes, and some kind of lint marker.
> > I'm guessing that linter hasn't been run in a while from the mixed use of the marker vs comments.
> >
> > /*lint -fallthrough */
> >
> > This is the lint marker
>
> Yes; but from my patch, the hunk modifying
> acpi_ex_store_object_to_node() and vsnprintf() seem to indicate that maybe the linter hasn't been run in a while.
>
> Which linter is that? I'm curious whether I should leave those be, and whether we're going to have an issue between compilers and linters as to which line/order these would need to appear on.
>
> It's an old version of PC-Lint, which we don't use anymore.
Ah, ok, I'll remove them then.
+ ACPI_FALLTHROUGH;
/*lint -fallthrough */
should work to support both, but I'll just remove it. V2 inbound.
Thanks for the feedback!
--
Thanks,
~Nick Desaulniers
-----Original Message-----
From: Moore, Robert
Sent: Friday, November 13, 2020 1:42 PM
To: Nick Desaulniers <[email protected]>
Cc: Kaneda, Erik <[email protected]>; Wysocki, Rafael J <[email protected]>; Gustavo A . R . Silva <[email protected]>; [email protected]; Len Brown <[email protected]>; [email protected]; [email protected]; [email protected]
Subject: RE: [PATCH] ACPICA: fix -Wfallthrough
-----Original Message-----
From: Nick Desaulniers <[email protected]>
Sent: Friday, November 13, 2020 1:33 PM
To: Moore, Robert <[email protected]>
Cc: Kaneda, Erik <[email protected]>; Wysocki, Rafael J <[email protected]>; Gustavo A . R . Silva <[email protected]>; [email protected]; Len Brown <[email protected]>; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH] ACPICA: fix -Wfallthrough
On Fri, Nov 13, 2020 at 1:27 PM Moore, Robert <[email protected]> wrote:
>
>
>
> -----Original Message-----
> From: ndesaulniers via sendgmr
> <[email protected]> On Behalf Of Nick
> Desaulniers
> Sent: Tuesday, November 10, 2020 6:12 PM
> To: Moore, Robert <[email protected]>; Kaneda, Erik
> <[email protected]>; Wysocki, Rafael J
> <[email protected]>; Gustavo A . R . Silva
> <[email protected]>
> Cc: [email protected]; Nick Desaulniers
> <[email protected]>; Len Brown <[email protected]>;
> [email protected]; [email protected];
> [email protected]
> Subject: [PATCH] ACPICA: fix -Wfallthrough
>
> The "fallthrough" pseudo-keyword was added as a portable way to denote intentional fallthrough. This code seemed to be using a mix of fallthrough comments that GCC recognizes, and some kind of lint marker.
> I'm guessing that linter hasn't been run in a while from the mixed use of the marker vs comments.
>
> /*lint -fallthrough */
>
> This is the lint marker
Yes; but from my patch, the hunk modifying
acpi_ex_store_object_to_node() and vsnprintf() seem to indicate that maybe the linter hasn't been run in a while.
Which linter is that? I'm curious whether I should leave those be, and whether we're going to have an issue between compilers and linters as to which line/order these would need to appear on.
It's an old version of PC-Lint, which we don't use anymore.
So, you can get rid of the lint markers.
>
> BTW, what version of gcc added -Wfallthrough?
GCC 7.1 added -Wimplicit-fallthrough.
>
>
> Signed-off-by: Nick Desaulniers <[email protected]>
> ---
> drivers/acpi/acpica/dscontrol.c | 3 +--
> drivers/acpi/acpica/dswexec.c | 4 +---
> drivers/acpi/acpica/dswload.c | 3 +--
> drivers/acpi/acpica/dswload2.c | 3 +--
> drivers/acpi/acpica/exfldio.c | 3 +--
> drivers/acpi/acpica/exresop.c | 5 ++---
> drivers/acpi/acpica/exstore.c | 6 ++----
> drivers/acpi/acpica/hwgpe.c | 3 +--
> drivers/acpi/acpica/utdelete.c | 3 +--
> drivers/acpi/acpica/utprint.c | 2 +-
> 10 files changed, 12 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/acpi/acpica/dscontrol.c
> b/drivers/acpi/acpica/dscontrol.c index 4b5b6e859f62..1e75e5fbfd19
> 100644
> --- a/drivers/acpi/acpica/dscontrol.c
> +++ b/drivers/acpi/acpica/dscontrol.c
> @@ -61,8 +61,7 @@ acpi_ds_exec_begin_control_op(struct acpi_walk_state *walk_state,
> break;
> }
> }
> -
> - /*lint -fallthrough */
> + fallthrough;
>
> case AML_IF_OP:
> /*
> diff --git a/drivers/acpi/acpica/dswexec.c
> b/drivers/acpi/acpica/dswexec.c index 1d4f8c81028c..e8c32d4fe55f
> 100644
> --- a/drivers/acpi/acpica/dswexec.c
> +++ b/drivers/acpi/acpica/dswexec.c
> @@ -597,9 +597,7 @@ acpi_status acpi_ds_exec_end_op(struct acpi_walk_state *walk_state)
> if (ACPI_FAILURE(status)) {
> break;
> }
> -
> - /* Fall through */
> - /*lint -fallthrough */
> + fallthrough;
>
> case AML_INT_EVAL_SUBTREE_OP:
>
> diff --git a/drivers/acpi/acpica/dswload.c
> b/drivers/acpi/acpica/dswload.c index 27069325b6de..afc663c3742d
> 100644
> --- a/drivers/acpi/acpica/dswload.c
> +++ b/drivers/acpi/acpica/dswload.c
> @@ -223,8 +223,7 @@ acpi_ds_load1_begin_op(struct acpi_walk_state *walk_state,
> parse_flags & ACPI_PARSE_MODULE_LEVEL)) {
> break;
> }
> -
> - /*lint -fallthrough */
> + fallthrough;
>
> default:
>
> diff --git a/drivers/acpi/acpica/dswload2.c
> b/drivers/acpi/acpica/dswload2.c index edadbe146506..1b794b6ba072
> 100644
> --- a/drivers/acpi/acpica/dswload2.c
> +++ b/drivers/acpi/acpica/dswload2.c
> @@ -213,8 +213,7 @@ acpi_ds_load2_begin_op(struct acpi_walk_state *walk_state,
> parse_flags & ACPI_PARSE_MODULE_LEVEL)) {
> break;
> }
> -
> - /*lint -fallthrough */
> + fallthrough;
>
> default:
>
> diff --git a/drivers/acpi/acpica/exfldio.c
> b/drivers/acpi/acpica/exfldio.c index ade35ff1c7ba..9d1cabe0fed9
> 100644
> --- a/drivers/acpi/acpica/exfldio.c
> +++ b/drivers/acpi/acpica/exfldio.c
> @@ -433,8 +433,7 @@ acpi_ex_field_datum_io(union acpi_operand_object *obj_desc,
> * Now that the Bank has been selected, fall through to the
> * region_field case and write the datum to the Operation Region
> */
> -
> - /*lint -fallthrough */
> + fallthrough;
>
> case ACPI_TYPE_LOCAL_REGION_FIELD:
> /*
> diff --git a/drivers/acpi/acpica/exresop.c
> b/drivers/acpi/acpica/exresop.c index 4d1b22971d58..df48faa9a551
> 100644
> --- a/drivers/acpi/acpica/exresop.c
> +++ b/drivers/acpi/acpica/exresop.c
> @@ -197,8 +197,7 @@ acpi_ex_resolve_operands(u16 opcode,
> case ACPI_REFCLASS_DEBUG:
>
> target_op = AML_DEBUG_OP;
> -
> - /*lint -fallthrough */
> + fallthrough;
>
> case ACPI_REFCLASS_ARG:
> case ACPI_REFCLASS_LOCAL:
> @@ -264,7 +263,7 @@ acpi_ex_resolve_operands(u16 opcode,
> * Else not a string - fall through to the normal Reference
> * case below
> */
> - /*lint -fallthrough */
> + 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..2067baa7c120
> 100644
> --- a/drivers/acpi/acpica/exstore.c
> +++ b/drivers/acpi/acpica/exstore.c
> @@ -95,8 +95,7 @@ acpi_ex_store(union acpi_operand_object *source_desc,
> if (dest_desc->common.flags & AOPOBJ_AML_CONSTANT) {
> return_ACPI_STATUS(AE_OK);
> }
> -
> - /*lint -fallthrough */
> + fallthrough;
>
> default:
>
> @@ -421,8 +420,7 @@ acpi_ex_store_object_to_node(union acpi_operand_object *source_desc,
> }
> break;
> }
> -
> - /* Fallthrough */
> + 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..fbfad80c8a53 100644
> --- a/drivers/acpi/acpica/hwgpe.c
> +++ b/drivers/acpi/acpica/hwgpe.c
> @@ -166,8 +166,7 @@ acpi_hw_low_set_gpe(struct acpi_gpe_event_info *gpe_event_info, u32 action)
> if (!(register_bit & gpe_register_info->enable_mask)) {
> return (AE_BAD_PARAMETER);
> }
> -
> - /*lint -fallthrough */
> + fallthrough;
>
> case ACPI_GPE_ENABLE:
>
> diff --git a/drivers/acpi/acpica/utdelete.c
> b/drivers/acpi/acpica/utdelete.c index 4c0d4e434196..8076e7947585
> 100644
> --- a/drivers/acpi/acpica/utdelete.c
> +++ b/drivers/acpi/acpica/utdelete.c
> @@ -111,8 +111,7 @@ static void acpi_ut_delete_internal_obj(union acpi_operand_object *object)
> (void)acpi_ev_delete_gpe_block(object->device.
> gpe_block);
> }
> -
> - /*lint -fallthrough */
> + fallthrough;
>
> case ACPI_TYPE_PROCESSOR:
> case ACPI_TYPE_THERMAL:
> diff --git a/drivers/acpi/acpica/utprint.c
> b/drivers/acpi/acpica/utprint.c index 681c11f4af4e..f7e43baf5ff2
> 100644
> --- a/drivers/acpi/acpica/utprint.c
> +++ b/drivers/acpi/acpica/utprint.c
> @@ -475,7 +475,7 @@ int vsnprintf(char *string, acpi_size size, const char *format, va_list args)
> case 'X':
>
> type |= ACPI_FORMAT_UPPER;
> - /* FALLTHROUGH */
> + fallthrough;
>
> case 'x':
>
> --
> 2.29.2.222.g5d2a92d10f8-goog
>
--
Thanks,
~Nick Desaulniers
BTW, if you can make a pull request for the patch up on github, that would help.
-----Original Message-----
From: Moore, Robert
Sent: Friday, November 13, 2020 1:44 PM
To: Nick Desaulniers <[email protected]>
Cc: Kaneda, Erik <[email protected]>; Wysocki, Rafael J <[email protected]>; Gustavo A . R . Silva <[email protected]>; [email protected]; Len Brown <[email protected]>; [email protected]; [email protected]; [email protected]
Subject: RE: [PATCH] ACPICA: fix -Wfallthrough
-----Original Message-----
From: Moore, Robert
Sent: Friday, November 13, 2020 1:42 PM
To: Nick Desaulniers <[email protected]>
Cc: Kaneda, Erik <[email protected]>; Wysocki, Rafael J <[email protected]>; Gustavo A . R . Silva <[email protected]>; [email protected]; Len Brown <[email protected]>; [email protected]; [email protected]; [email protected]
Subject: RE: [PATCH] ACPICA: fix -Wfallthrough
-----Original Message-----
From: Nick Desaulniers <[email protected]>
Sent: Friday, November 13, 2020 1:33 PM
To: Moore, Robert <[email protected]>
Cc: Kaneda, Erik <[email protected]>; Wysocki, Rafael J <[email protected]>; Gustavo A . R . Silva <[email protected]>; [email protected]; Len Brown <[email protected]>; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH] ACPICA: fix -Wfallthrough
On Fri, Nov 13, 2020 at 1:27 PM Moore, Robert <[email protected]> wrote:
>
>
>
> -----Original Message-----
> From: ndesaulniers via sendgmr
> <[email protected]> On Behalf Of Nick
> Desaulniers
> Sent: Tuesday, November 10, 2020 6:12 PM
> To: Moore, Robert <[email protected]>; Kaneda, Erik
> <[email protected]>; Wysocki, Rafael J
> <[email protected]>; Gustavo A . R . Silva
> <[email protected]>
> Cc: [email protected]; Nick Desaulniers
> <[email protected]>; Len Brown <[email protected]>;
> [email protected]; [email protected];
> [email protected]
> Subject: [PATCH] ACPICA: fix -Wfallthrough
>
> The "fallthrough" pseudo-keyword was added as a portable way to denote intentional fallthrough. This code seemed to be using a mix of fallthrough comments that GCC recognizes, and some kind of lint marker.
> I'm guessing that linter hasn't been run in a while from the mixed use of the marker vs comments.
>
> /*lint -fallthrough */
>
> This is the lint marker
Yes; but from my patch, the hunk modifying
acpi_ex_store_object_to_node() and vsnprintf() seem to indicate that maybe the linter hasn't been run in a while.
Which linter is that? I'm curious whether I should leave those be, and whether we're going to have an issue between compilers and linters as to which line/order these would need to appear on.
It's an old version of PC-Lint, which we don't use anymore.
So, you can get rid of the lint markers.
>
> BTW, what version of gcc added -Wfallthrough?
GCC 7.1 added -Wimplicit-fallthrough.
>
>
> Signed-off-by: Nick Desaulniers <[email protected]>
> ---
> drivers/acpi/acpica/dscontrol.c | 3 +--
> drivers/acpi/acpica/dswexec.c | 4 +---
> drivers/acpi/acpica/dswload.c | 3 +--
> drivers/acpi/acpica/dswload2.c | 3 +--
> drivers/acpi/acpica/exfldio.c | 3 +--
> drivers/acpi/acpica/exresop.c | 5 ++---
> drivers/acpi/acpica/exstore.c | 6 ++----
> drivers/acpi/acpica/hwgpe.c | 3 +--
> drivers/acpi/acpica/utdelete.c | 3 +--
> drivers/acpi/acpica/utprint.c | 2 +-
> 10 files changed, 12 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/acpi/acpica/dscontrol.c
> b/drivers/acpi/acpica/dscontrol.c index 4b5b6e859f62..1e75e5fbfd19
> 100644
> --- a/drivers/acpi/acpica/dscontrol.c
> +++ b/drivers/acpi/acpica/dscontrol.c
> @@ -61,8 +61,7 @@ acpi_ds_exec_begin_control_op(struct acpi_walk_state *walk_state,
> break;
> }
> }
> -
> - /*lint -fallthrough */
> + fallthrough;
>
> case AML_IF_OP:
> /*
> diff --git a/drivers/acpi/acpica/dswexec.c
> b/drivers/acpi/acpica/dswexec.c index 1d4f8c81028c..e8c32d4fe55f
> 100644
> --- a/drivers/acpi/acpica/dswexec.c
> +++ b/drivers/acpi/acpica/dswexec.c
> @@ -597,9 +597,7 @@ acpi_status acpi_ds_exec_end_op(struct acpi_walk_state *walk_state)
> if (ACPI_FAILURE(status)) {
> break;
> }
> -
> - /* Fall through */
> - /*lint -fallthrough */
> + fallthrough;
>
> case AML_INT_EVAL_SUBTREE_OP:
>
> diff --git a/drivers/acpi/acpica/dswload.c
> b/drivers/acpi/acpica/dswload.c index 27069325b6de..afc663c3742d
> 100644
> --- a/drivers/acpi/acpica/dswload.c
> +++ b/drivers/acpi/acpica/dswload.c
> @@ -223,8 +223,7 @@ acpi_ds_load1_begin_op(struct acpi_walk_state *walk_state,
> parse_flags & ACPI_PARSE_MODULE_LEVEL)) {
> break;
> }
> -
> - /*lint -fallthrough */
> + fallthrough;
>
> default:
>
> diff --git a/drivers/acpi/acpica/dswload2.c
> b/drivers/acpi/acpica/dswload2.c index edadbe146506..1b794b6ba072
> 100644
> --- a/drivers/acpi/acpica/dswload2.c
> +++ b/drivers/acpi/acpica/dswload2.c
> @@ -213,8 +213,7 @@ acpi_ds_load2_begin_op(struct acpi_walk_state *walk_state,
> parse_flags & ACPI_PARSE_MODULE_LEVEL)) {
> break;
> }
> -
> - /*lint -fallthrough */
> + fallthrough;
>
> default:
>
> diff --git a/drivers/acpi/acpica/exfldio.c
> b/drivers/acpi/acpica/exfldio.c index ade35ff1c7ba..9d1cabe0fed9
> 100644
> --- a/drivers/acpi/acpica/exfldio.c
> +++ b/drivers/acpi/acpica/exfldio.c
> @@ -433,8 +433,7 @@ acpi_ex_field_datum_io(union acpi_operand_object *obj_desc,
> * Now that the Bank has been selected, fall through to the
> * region_field case and write the datum to the Operation Region
> */
> -
> - /*lint -fallthrough */
> + fallthrough;
>
> case ACPI_TYPE_LOCAL_REGION_FIELD:
> /*
> diff --git a/drivers/acpi/acpica/exresop.c
> b/drivers/acpi/acpica/exresop.c index 4d1b22971d58..df48faa9a551
> 100644
> --- a/drivers/acpi/acpica/exresop.c
> +++ b/drivers/acpi/acpica/exresop.c
> @@ -197,8 +197,7 @@ acpi_ex_resolve_operands(u16 opcode,
> case ACPI_REFCLASS_DEBUG:
>
> target_op = AML_DEBUG_OP;
> -
> - /*lint -fallthrough */
> + fallthrough;
>
> case ACPI_REFCLASS_ARG:
> case ACPI_REFCLASS_LOCAL:
> @@ -264,7 +263,7 @@ acpi_ex_resolve_operands(u16 opcode,
> * Else not a string - fall through to the normal Reference
> * case below
> */
> - /*lint -fallthrough */
> + 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..2067baa7c120
> 100644
> --- a/drivers/acpi/acpica/exstore.c
> +++ b/drivers/acpi/acpica/exstore.c
> @@ -95,8 +95,7 @@ acpi_ex_store(union acpi_operand_object *source_desc,
> if (dest_desc->common.flags & AOPOBJ_AML_CONSTANT) {
> return_ACPI_STATUS(AE_OK);
> }
> -
> - /*lint -fallthrough */
> + fallthrough;
>
> default:
>
> @@ -421,8 +420,7 @@ acpi_ex_store_object_to_node(union acpi_operand_object *source_desc,
> }
> break;
> }
> -
> - /* Fallthrough */
> + 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..fbfad80c8a53 100644
> --- a/drivers/acpi/acpica/hwgpe.c
> +++ b/drivers/acpi/acpica/hwgpe.c
> @@ -166,8 +166,7 @@ acpi_hw_low_set_gpe(struct acpi_gpe_event_info *gpe_event_info, u32 action)
> if (!(register_bit & gpe_register_info->enable_mask)) {
> return (AE_BAD_PARAMETER);
> }
> -
> - /*lint -fallthrough */
> + fallthrough;
>
> case ACPI_GPE_ENABLE:
>
> diff --git a/drivers/acpi/acpica/utdelete.c
> b/drivers/acpi/acpica/utdelete.c index 4c0d4e434196..8076e7947585
> 100644
> --- a/drivers/acpi/acpica/utdelete.c
> +++ b/drivers/acpi/acpica/utdelete.c
> @@ -111,8 +111,7 @@ static void acpi_ut_delete_internal_obj(union acpi_operand_object *object)
> (void)acpi_ev_delete_gpe_block(object->device.
> gpe_block);
> }
> -
> - /*lint -fallthrough */
> + fallthrough;
>
> case ACPI_TYPE_PROCESSOR:
> case ACPI_TYPE_THERMAL:
> diff --git a/drivers/acpi/acpica/utprint.c
> b/drivers/acpi/acpica/utprint.c index 681c11f4af4e..f7e43baf5ff2
> 100644
> --- a/drivers/acpi/acpica/utprint.c
> +++ b/drivers/acpi/acpica/utprint.c
> @@ -475,7 +475,7 @@ int vsnprintf(char *string, acpi_size size, const char *format, va_list args)
> case 'X':
>
> type |= ACPI_FORMAT_UPPER;
> - /* FALLTHROUGH */
> + fallthrough;
>
> case 'x':
>
> --
> 2.29.2.222.g5d2a92d10f8-goog
>
--
Thanks,
~Nick Desaulniers
On Fri, Nov 13, 2020 at 1:45 PM Moore, Robert <[email protected]> wrote:
>
> BTW, if you can make a pull request for the patch up on github, that would help.
https://github.com/acpica/acpica/pull/650
--
Thanks,
~Nick Desaulniers
-----Original Message-----
From: Nick Desaulniers <[email protected]>
Sent: Friday, November 13, 2020 2:09 PM
To: Moore, Robert <[email protected]>
Cc: Kaneda, Erik <[email protected]>; Wysocki, Rafael J <[email protected]>; Gustavo A . R . Silva <[email protected]>; [email protected]; Len Brown <[email protected]>; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH] ACPICA: fix -Wfallthrough
On Fri, Nov 13, 2020 at 1:45 PM Moore, Robert <[email protected]> wrote:
>
> BTW, if you can make a pull request for the patch up on github, that would help.
https://github.com/acpica/acpica/pull/650
Great, thanks. I'll look at/merge the request next week.
Bob
--
Thanks,
~Nick Desaulniers
On 11/11/2020 02:11, Nick Desaulniers wrote:
> The "fallthrough" pseudo-keyword was added as a portable way to denote
> intentional fallthrough. This code seemed to be using a mix of
> fallthrough comments that GCC recognizes, and some kind of lint marker.
> I'm guessing that linter hasn't been run in a while from the mixed use
> of the marker vs comments.
>
> Signed-off-by: Nick Desaulniers <[email protected]>
I know this is not the exact version that was merged, I can't find it on
the list, but looks like the version that was merged [0], is causing
build errors with older toolchains (GCC v6) ...
/dvs/git/dirty/git-master_l4t-upstream/kernel/drivers/acpi/acpica/dscontrol.c: In function ‘acpi_ds_exec_begin_control_op’:
/dvs/git/dirty/git-master_l4t-upstream/kernel/drivers/acpi/acpica/dscontrol.c:65:3: error: ‘ACPI_FALLTHROUGH’ undeclared (first use in this function)
ACPI_FALLTHROUGH;
^~~~~~~~~~~~~~~~
/dvs/git/dirty/git-master_l4t-upstream/kernel/drivers/acpi/acpica/dscontrol.c:65:3: note: each undeclared identifier is reported only once for each function it appears in
/dvs/git/dirty/git-master_l4t-upstream/kernel/scripts/Makefile.build:287: recipe for target 'drivers/acpi/acpica/dscontrol.o' failed
Cheers
Jon
[0] https://github.com/acpica/acpica/commit/4b9135f5
--
nvpublic
On Thu, Jan 21, 2021 at 11:08 AM Jon Hunter <[email protected]> wrote:
>
>
> On 11/11/2020 02:11, Nick Desaulniers wrote:
> > The "fallthrough" pseudo-keyword was added as a portable way to denote
> > intentional fallthrough. This code seemed to be using a mix of
> > fallthrough comments that GCC recognizes, and some kind of lint marker.
> > I'm guessing that linter hasn't been run in a while from the mixed use
> > of the marker vs comments.
> >
> > Signed-off-by: Nick Desaulniers <[email protected]>
>
>
> I know this is not the exact version that was merged, I can't find it on
> the list, but looks like the version that was merged [0],
It would be this patch:
https://patchwork.kernel.org/project/linux-acpi/patch/[email protected]/
Nick, Erik?
> is causing build errors with older toolchains (GCC v6) ...
>
> /dvs/git/dirty/git-master_l4t-upstream/kernel/drivers/acpi/acpica/dscontrol.c: In function ‘acpi_ds_exec_begin_control_op’:
> /dvs/git/dirty/git-master_l4t-upstream/kernel/drivers/acpi/acpica/dscontrol.c:65:3: error: ‘ACPI_FALLTHROUGH’ undeclared (first use in this function)
> ACPI_FALLTHROUGH;
> ^~~~~~~~~~~~~~~~
> /dvs/git/dirty/git-master_l4t-upstream/kernel/drivers/acpi/acpica/dscontrol.c:65:3: note: each undeclared identifier is reported only once for each function it appears in
> /dvs/git/dirty/git-master_l4t-upstream/kernel/scripts/Makefile.build:287: recipe for target 'drivers/acpi/acpica/dscontrol.o' failed
>
> Cheers
> Jon
>
> [0] https://github.com/acpica/acpica/commit/4b9135f5
>
> --
> nvpublic
On Thu, Jan 21, 2021 at 11:03 AM Rafael J. Wysocki <[email protected]> wrote:
>
> On Thu, Jan 21, 2021 at 11:08 AM Jon Hunter <[email protected]> wrote:
> >
> >
> > On 11/11/2020 02:11, Nick Desaulniers wrote:
> > > The "fallthrough" pseudo-keyword was added as a portable way to denote
> > > intentional fallthrough. This code seemed to be using a mix of
> > > fallthrough comments that GCC recognizes, and some kind of lint marker.
> > > I'm guessing that linter hasn't been run in a while from the mixed use
> > > of the marker vs comments.
> > >
> > > Signed-off-by: Nick Desaulniers <[email protected]>
> >
> >
> > I know this is not the exact version that was merged, I can't find it on
> > the list, but looks like the version that was merged [0],
>
> It would be this patch:
>
> https://patchwork.kernel.org/project/linux-acpi/patch/[email protected]/
>
> Nick, Erik?
oh, shit, looks like a line was dropped. Here's what I sent upstream:
https://github.com/acpica/acpica/pull/650/files#diff-cccd96e900e01f7224c81508cbddfb1af6fcfbff959d6bfb55123e1b9cad4e38R1543
Note in the patch Rafael links to that line is missing and there's
instead an #ifdef that's empty. Was this line accidentally dropped?
>
> > is causing build errors with older toolchains (GCC v6) ...
> >
> > /dvs/git/dirty/git-master_l4t-upstream/kernel/drivers/acpi/acpica/dscontrol.c: In function ‘acpi_ds_exec_begin_control_op’:
> > /dvs/git/dirty/git-master_l4t-upstream/kernel/drivers/acpi/acpica/dscontrol.c:65:3: error: ‘ACPI_FALLTHROUGH’ undeclared (first use in this function)
> > ACPI_FALLTHROUGH;
> > ^~~~~~~~~~~~~~~~
> > /dvs/git/dirty/git-master_l4t-upstream/kernel/drivers/acpi/acpica/dscontrol.c:65:3: note: each undeclared identifier is reported only once for each function it appears in
> > /dvs/git/dirty/git-master_l4t-upstream/kernel/scripts/Makefile.build:287: recipe for target 'drivers/acpi/acpica/dscontrol.o' failed
> >
> > Cheers
> > Jon
> >
> > [0] https://github.com/acpica/acpica/commit/4b9135f5
> >
> > --
> > nvpublic
--
Thanks,
~Nick Desaulniers
> -----Original Message-----
> From: Nick Desaulniers <[email protected]>
> Sent: Thursday, January 21, 2021 11:08 AM
> To: Rafael J. Wysocki <[email protected]>; Kaneda, Erik
> <[email protected]>
> Cc: Jon Hunter <[email protected]>; Moore, Robert
> <[email protected]>; Wysocki, Rafael J <[email protected]>;
> Gustavo A . R . Silva <[email protected]>; clang-built-linux <clang-built-
> [email protected]>; Len Brown <[email protected]>; ACPI Devel
> Maling List <[email protected]>; open list:ACPI COMPONENT
> ARCHITECTURE (ACPICA) <[email protected]>; Linux Kernel Mailing List
> <[email protected]>; linux-tegra <[email protected]>
> Subject: Re: [PATCH] ACPICA: fix -Wfallthrough
>
> On Thu, Jan 21, 2021 at 11:03 AM Rafael J. Wysocki <[email protected]>
> wrote:
> >
> > On Thu, Jan 21, 2021 at 11:08 AM Jon Hunter <[email protected]>
> wrote:
> > >
> > >
> > > On 11/11/2020 02:11, Nick Desaulniers wrote:
> > > > The "fallthrough" pseudo-keyword was added as a portable way to
> denote
> > > > intentional fallthrough. This code seemed to be using a mix of
> > > > fallthrough comments that GCC recognizes, and some kind of lint
> marker.
> > > > I'm guessing that linter hasn't been run in a while from the mixed use
> > > > of the marker vs comments.
> > > >
> > > > Signed-off-by: Nick Desaulniers <[email protected]>
> > >
> > >
> > > I know this is not the exact version that was merged, I can't find it on
> > > the list, but looks like the version that was merged [0],
> >
> > It would be this patch:
> >
> > https://patchwork.kernel.org/project/linux-
> acpi/patch/[email protected]/
> >
> > Nick, Erik?
>
> oh, shit, looks like a line was dropped. Here's what I sent upstream:
> https://github.com/acpica/acpica/pull/650/files#diff-
> cccd96e900e01f7224c81508cbddfb1af6fcfbff959d6bfb55123e1b9cad4e38R154
> 3
> Note in the patch Rafael links to that line is missing and there's
> instead an #ifdef that's empty. Was this line accidentally dropped?
Let me take a look...
>
> >
> > > is causing build errors with older toolchains (GCC v6) ...
> > >
> > > /dvs/git/dirty/git-master_l4t-
> upstream/kernel/drivers/acpi/acpica/dscontrol.c: In function
> ‘acpi_ds_exec_begin_control_op’:
> > > /dvs/git/dirty/git-master_l4t-
> upstream/kernel/drivers/acpi/acpica/dscontrol.c:65:3: error:
> ‘ACPI_FALLTHROUGH’ undeclared (first use in this function)
> > > ACPI_FALLTHROUGH;
> > > ^~~~~~~~~~~~~~~~
> > > /dvs/git/dirty/git-master_l4t-
> upstream/kernel/drivers/acpi/acpica/dscontrol.c:65:3: note: each
> undeclared identifier is reported only once for each function it appears in
> > > /dvs/git/dirty/git-master_l4t-
> upstream/kernel/scripts/Makefile.build:287: recipe for target
> 'drivers/acpi/acpica/dscontrol.o' failed
> > >
> > > Cheers
> > > Jon
> > >
> > > [0] https://github.com/acpica/acpica/commit/4b9135f5
> > >
> > > --
> > > nvpublic
>
>
>
> --
> Thanks,
> ~Nick Desaulniers
> -----Original Message-----
> From: Nick Desaulniers <[email protected]>
> Sent: Thursday, January 21, 2021 11:08 AM
> To: Rafael J. Wysocki <[email protected]>; Kaneda, Erik
> <[email protected]>
> Cc: Jon Hunter <[email protected]>; Moore, Robert
> <[email protected]>; Wysocki, Rafael J <[email protected]>;
> Gustavo A . R . Silva <[email protected]>; clang-built-linux <clang-built-
> [email protected]>; Len Brown <[email protected]>; ACPI Devel
> Maling List <[email protected]>; open list:ACPI COMPONENT
> ARCHITECTURE (ACPICA) <[email protected]>; Linux Kernel Mailing List
> <[email protected]>; linux-tegra <[email protected]>
> Subject: Re: [PATCH] ACPICA: fix -Wfallthrough
>
> On Thu, Jan 21, 2021 at 11:03 AM Rafael J. Wysocki <[email protected]>
> wrote:
> >
> > On Thu, Jan 21, 2021 at 11:08 AM Jon Hunter <[email protected]>
> wrote:
> > >
> > >
> > > On 11/11/2020 02:11, Nick Desaulniers wrote:
> > > > The "fallthrough" pseudo-keyword was added as a portable way to
> denote
> > > > intentional fallthrough. This code seemed to be using a mix of
> > > > fallthrough comments that GCC recognizes, and some kind of lint
> marker.
> > > > I'm guessing that linter hasn't been run in a while from the mixed use
> > > > of the marker vs comments.
> > > >
> > > > Signed-off-by: Nick Desaulniers <[email protected]>
> > >
> > >
> > > I know this is not the exact version that was merged, I can't find it on
> > > the list, but looks like the version that was merged [0],
> >
> > It would be this patch:
> >
> > https://patchwork.kernel.org/project/linux-
> acpi/patch/[email protected]/
> >
> > Nick, Erik?
>
> oh, shit, looks like a line was dropped. Here's what I sent upstream:
> https://github.com/acpica/acpica/pull/650/files#diff-
> cccd96e900e01f7224c81508cbddfb1af6fcfbff959d6bfb55123e1b9cad4e38R154
> 3
> Note in the patch Rafael links to that line is missing and there's
> instead an #ifdef that's empty. Was this line accidentally dropped?
Looks like this line was dropped by ACPICA's Linux-ize scripts. I'll re-add it and send again.
Rafael, do you want me to re-send the series or do you want me to resend the specific commit? I don't mind either way.
Thanks,
Erik
>
> >
> > > is causing build errors with older toolchains (GCC v6) ...
> > >
> > > /dvs/git/dirty/git-master_l4t-
> upstream/kernel/drivers/acpi/acpica/dscontrol.c: In function
> ‘acpi_ds_exec_begin_control_op’:
> > > /dvs/git/dirty/git-master_l4t-
> upstream/kernel/drivers/acpi/acpica/dscontrol.c:65:3: error:
> ‘ACPI_FALLTHROUGH’ undeclared (first use in this function)
> > > ACPI_FALLTHROUGH;
> > > ^~~~~~~~~~~~~~~~
> > > /dvs/git/dirty/git-master_l4t-
> upstream/kernel/drivers/acpi/acpica/dscontrol.c:65:3: note: each
> undeclared identifier is reported only once for each function it appears in
> > > /dvs/git/dirty/git-master_l4t-
> upstream/kernel/scripts/Makefile.build:287: recipe for target
> 'drivers/acpi/acpica/dscontrol.o' failed
> > >
> > > Cheers
> > > Jon
> > >
> > > [0] https://github.com/acpica/acpica/commit/4b9135f5
> > >
> > > --
> > > nvpublic
>
>
>
> --
> Thanks,
> ~Nick Desaulniers