| Message ID | 20220525130123.767410-6-sakari.ailus@linux.intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ACPI: Buffer property and reference as string support | expand |
On Wed, May 25, 2022 at 04:01:20PM +0300, Sakari Ailus wrote: > __acpi_node_get_property_reference() uses a series of if () statements for > testing the same variable. There's soon going to be one more value to be > tested. > > Switch to use switch() instead. Looks like sanitization over the different parts of the code in this file. Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > --- > drivers/acpi/property.c | 41 ++++++++++++++++++++++------------------- > 1 file changed, 22 insertions(+), 19 deletions(-) > > diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c > index dd6cce955ee28..a8e8a214a524f 100644 > --- a/drivers/acpi/property.c > +++ b/drivers/acpi/property.c > @@ -780,11 +780,9 @@ int __acpi_node_get_property_reference(const struct fwnode_handle *fwnode, > if (ret) > return ret == -EINVAL ? -ENOENT : -EINVAL; > > - /* > - * The simplest case is when the value is a single reference. Just > - * return that reference then. > - */ > - if (obj->type == ACPI_TYPE_LOCAL_REFERENCE) { > + switch (obj->type) { > + case ACPI_TYPE_LOCAL_REFERENCE: > + /* Plain single reference without arguments. */ > if (index) > return -ENOENT; > > @@ -795,19 +793,21 @@ int __acpi_node_get_property_reference(const struct fwnode_handle *fwnode, > args->fwnode = acpi_fwnode_handle(device); > args->nargs = 0; > return 0; > + case ACPI_TYPE_PACKAGE: > + /* > + * If it is not a single reference, then it is a package of > + * references followed by number of ints as follows: > + * > + * Package () { REF, INT, REF, INT, INT } > + * > + * The index argument is then used to determine which reference > + * the caller wants (along with the arguments). > + */ > + break; > + default: > + return -EINVAL; > } > > - /* > - * If it is not a single reference, then it is a package of > - * references followed by number of ints as follows: > - * > - * Package () { REF, INT, REF, INT, INT } > - * > - * The index argument is then used to determine which reference > - * the caller wants (along with the arguments). > - */ > - if (obj->type != ACPI_TYPE_PACKAGE) > - return -EINVAL; > if (index >= obj->package.count) > return -ENOENT; > > @@ -815,7 +815,8 @@ int __acpi_node_get_property_reference(const struct fwnode_handle *fwnode, > end = element + obj->package.count; > > while (element < end) { > - if (element->type == ACPI_TYPE_LOCAL_REFERENCE) { > + switch (element->type) { > + case ACPI_TYPE_LOCAL_REFERENCE: > device = acpi_fetch_acpi_dev(element->reference.handle); > if (!device) > return -EINVAL; > @@ -831,11 +832,13 @@ int __acpi_node_get_property_reference(const struct fwnode_handle *fwnode, > if (idx == index) > return 0; > > - } else if (element->type == ACPI_TYPE_INTEGER) { > + break; > + case ACPI_TYPE_INTEGER: > if (idx == index) > return -ENOENT; > element++; > - } else { > + break; > + default: > return -EINVAL; > } > > -- > 2.30.2 >
diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c index dd6cce955ee28..a8e8a214a524f 100644 --- a/drivers/acpi/property.c +++ b/drivers/acpi/property.c @@ -780,11 +780,9 @@ int __acpi_node_get_property_reference(const struct fwnode_handle *fwnode, if (ret) return ret == -EINVAL ? -ENOENT : -EINVAL; - /* - * The simplest case is when the value is a single reference. Just - * return that reference then. - */ - if (obj->type == ACPI_TYPE_LOCAL_REFERENCE) { + switch (obj->type) { + case ACPI_TYPE_LOCAL_REFERENCE: + /* Plain single reference without arguments. */ if (index) return -ENOENT; @@ -795,19 +793,21 @@ int __acpi_node_get_property_reference(const struct fwnode_handle *fwnode, args->fwnode = acpi_fwnode_handle(device); args->nargs = 0; return 0; + case ACPI_TYPE_PACKAGE: + /* + * If it is not a single reference, then it is a package of + * references followed by number of ints as follows: + * + * Package () { REF, INT, REF, INT, INT } + * + * The index argument is then used to determine which reference + * the caller wants (along with the arguments). + */ + break; + default: + return -EINVAL; } - /* - * If it is not a single reference, then it is a package of - * references followed by number of ints as follows: - * - * Package () { REF, INT, REF, INT, INT } - * - * The index argument is then used to determine which reference - * the caller wants (along with the arguments). - */ - if (obj->type != ACPI_TYPE_PACKAGE) - return -EINVAL; if (index >= obj->package.count) return -ENOENT; @@ -815,7 +815,8 @@ int __acpi_node_get_property_reference(const struct fwnode_handle *fwnode, end = element + obj->package.count; while (element < end) { - if (element->type == ACPI_TYPE_LOCAL_REFERENCE) { + switch (element->type) { + case ACPI_TYPE_LOCAL_REFERENCE: device = acpi_fetch_acpi_dev(element->reference.handle); if (!device) return -EINVAL; @@ -831,11 +832,13 @@ int __acpi_node_get_property_reference(const struct fwnode_handle *fwnode, if (idx == index) return 0; - } else if (element->type == ACPI_TYPE_INTEGER) { + break; + case ACPI_TYPE_INTEGER: if (idx == index) return -ENOENT; element++; - } else { + break; + default: return -EINVAL; }
__acpi_node_get_property_reference() uses a series of if () statements for testing the same variable. There's soon going to be one more value to be tested. Switch to use switch() instead. Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> --- drivers/acpi/property.c | 41 ++++++++++++++++++++++------------------- 1 file changed, 22 insertions(+), 19 deletions(-)