-----Original Message----- From: Moore, Robert Sent: Friday, November 13, 2020 1:42 PM To: Nick Desaulniers <ndesaulni...@google.com> Cc: Kaneda, Erik <erik.kan...@intel.com>; Wysocki, Rafael J <rafael.j.wyso...@intel.com>; Gustavo A . R . Silva <gustavo...@kernel.org>; clang-built-li...@googlegroups.com; Len Brown <l...@kernel.org>; linux-a...@vger.kernel.org; de...@acpica.org; linux-kernel@vger.kernel.org Subject: RE: [PATCH] ACPICA: fix -Wfallthrough
-----Original Message----- From: Nick Desaulniers <ndesaulni...@google.com> Sent: Friday, November 13, 2020 1:33 PM To: Moore, Robert <robert.mo...@intel.com> Cc: Kaneda, Erik <erik.kan...@intel.com>; Wysocki, Rafael J <rafael.j.wyso...@intel.com>; Gustavo A . R . Silva <gustavo...@kernel.org>; clang-built-li...@googlegroups.com; Len Brown <l...@kernel.org>; linux-a...@vger.kernel.org; de...@acpica.org; linux-kernel@vger.kernel.org Subject: Re: [PATCH] ACPICA: fix -Wfallthrough On Fri, Nov 13, 2020 at 1:27 PM Moore, Robert <robert.mo...@intel.com> wrote: > > > > -----Original Message----- > From: ndesaulniers via sendgmr > <ndesaulni...@ndesaulniers1.mtv.corp.google.com> On Behalf Of Nick > Desaulniers > Sent: Tuesday, November 10, 2020 6:12 PM > To: Moore, Robert <robert.mo...@intel.com>; Kaneda, Erik > <erik.kan...@intel.com>; Wysocki, Rafael J > <rafael.j.wyso...@intel.com>; Gustavo A . R . Silva > <gustavo...@kernel.org> > Cc: clang-built-li...@googlegroups.com; Nick Desaulniers > <ndesaulni...@google.com>; Len Brown <l...@kernel.org>; > linux-a...@vger.kernel.org; de...@acpica.org; > linux-kernel@vger.kernel.org > 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 <ndesaulni...@google.com> > --- > 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