linux-kernel - Re: [PATCH v1 1/1] ACPI: property: Get rid of redundant 'else'

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Fri, 18 Feb 2022 20:51:06 +0100
From:   "Rafael J. Wysocki" <rafael@...nel.org>
To:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc:     "Rafael J. Wysocki" <rafael.j.wysocki@...el.com>,
        ACPI Devel Maling List <linux-acpi@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        Len Brown <lenb@...nel.org>
Subject: Re: [PATCH v1 1/1] ACPI: property: Get rid of redundant 'else'

On Wed, Feb 9, 2022 at 3:53 PM Andy Shevchenko
<andriy.shevchenko@...ux.intel.com> wrote:
>
> In the snippets like the following
>
>         if (...)
>                 return / goto / break / continue ...;
>         else
>                 ...
>
> the 'else' is redundant. Get rid of it.
>
> While at it, replace conditional '<= 0' for unsigned type by '== 0'
> in acpi_data_prop_read(); update comment in the
> __acpi_node_get_property_reference() on how we parse the reference.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
> ---
>  drivers/acpi/property.c | 27 +++++++++++++++------------
>  1 file changed, 15 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
> index 4744c191acee..928aa4f86344 100644
> --- a/drivers/acpi/property.c
> +++ b/drivers/acpi/property.c
> @@ -541,7 +541,8 @@ acpi_device_data_of_node(const struct fwnode_handle *fwnode)
>         if (is_acpi_device_node(fwnode)) {
>                 const struct acpi_device *adev = to_acpi_device_node(fwnode);
>                 return &adev->data;
> -       } else if (is_acpi_data_node(fwnode)) {
> +       }
> +       if (is_acpi_data_node(fwnode)) {
>                 const struct acpi_data_node *dn = to_acpi_data_node(fwnode);
>                 return &dn->data;
>         }
> @@ -739,14 +740,19 @@ int __acpi_node_get_property_reference(const struct fwnode_handle *fwnode,
>                                         return -EINVAL;
>                         }
>
> -                       /* assume following integer elements are all args */
> +                       /*
> +                        * Assume the following integer elements are all args.
> +                        * Stop counting on the first reference or end of the
> +                        * package arguments. In case of neither reference,
> +                        * nor integer, return an error, we can't parse it.
> +                        */
>                         for (i = 0; element + i < end && i < num_args; i++) {
>                                 int type = element[i].type;
>
> +                               if (type == ACPI_TYPE_LOCAL_REFERENCE)
> +                                       break;
>                                 if (type == ACPI_TYPE_INTEGER)
>                                         nargs++;
> -                               else if (type == ACPI_TYPE_LOCAL_REFERENCE)
> -                                       break;
>                                 else
>                                         return -EINVAL;
>                         }
> @@ -938,7 +944,7 @@ static int acpi_data_prop_read(const struct acpi_device_data *data,
>
>         if (proptype != DEV_PROP_STRING && nval > obj->package.count)
>                 return -EOVERFLOW;
> -       else if (nval <= 0)
> +       if (nval == 0)
>                 return -EINVAL;
>
>         items = obj->package.elements;
> @@ -1000,14 +1006,10 @@ struct fwnode_handle *acpi_get_next_subnode(const struct fwnode_handle *fwnode,
>         const struct list_head *head;
>         struct list_head *next;
>
> -       if (!child || is_acpi_device_node(child)) {
> +       if ((!child || is_acpi_device_node(child)) && adev) {
>                 struct acpi_device *child_adev;
>
> -               if (adev)
> -                       head = &adev->children;
> -               else
> -                       goto nondev;
> -
> +               head = &adev->children;
>                 if (list_empty(head))
>                         goto nondev;
>
> @@ -1077,7 +1079,8 @@ acpi_node_get_parent(const struct fwnode_handle *fwnode)
>         if (is_acpi_data_node(fwnode)) {
>                 /* All data nodes have parent pointer so just return that */
>                 return to_acpi_data_node(fwnode)->parent;
> -       } else if (is_acpi_device_node(fwnode)) {
> +       }
> +       if (is_acpi_device_node(fwnode)) {
>                 struct device *dev = to_acpi_device_node(fwnode)->dev.parent;
>
>                 if (dev)
> --

Applied as 5.18 material, thanks!