Re: [PATCH v3 5/8] ACPI: property: Switch node property referencing from ifs to a switch

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

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
> 

-- 
With Best Regards,
Andy Shevchenko