linux-kernel - Re: [PATCH 7/8] asus-wireless: Export ids list

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] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 27 Jan 2017 17:36:15 +0200
From:   Andy Shevchenko <andy.shevchenko@...il.com>
To:     João Paulo Rechi Vita <jprvita@...il.com>
Cc:     Corentin Chary <corentin.chary@...il.com>,
        Darren Hart <dvhart@...radead.org>,
        Platform Driver <platform-driver-x86@...r.kernel.org>,
        acpi4asus-user@...ts.sourceforge.net,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        linux@...lessm.com,
        João Paulo Rechi Vita <jprvita@...lessm.com>
Subject: Re: [PATCH 7/8] asus-wireless: Export ids list

On Thu, Jan 26, 2017 at 5:30 PM, João Paulo Rechi Vita
<jprvita@...il.com> wrote:
> Signed-off-by: João Paulo Rechi Vita <jprvita@...lessm.com>
> ---
>  drivers/platform/x86/asus-wireless.c | 15 ++++++---------
>  drivers/platform/x86/asus-wireless.h | 10 ++++++++++
>  2 files changed, 16 insertions(+), 9 deletions(-)
>  create mode 100644 drivers/platform/x86/asus-wireless.h
>
> diff --git a/drivers/platform/x86/asus-wireless.c b/drivers/platform/x86/asus-wireless.c
> index 5a238fad35fb..fa28613688b4 100644
> --- a/drivers/platform/x86/asus-wireless.c
> +++ b/drivers/platform/x86/asus-wireless.c
> @@ -17,6 +17,8 @@
>  #include <linux/pci_ids.h>
>  #include <linux/leds.h>
>
> +#include "asus-wireless.h"
> +
>  #define ASUS_WIRELESS_LED_STATUS 0x2
>
>  struct hswc_params {
> @@ -40,12 +42,7 @@ static const struct hswc_params id_params[] = {
>         { 0x5, 0x4 },
>  };
>
> -static const struct acpi_device_id device_ids[] = {
> -       {"ATK4001", 0},
> -       {"ATK4002", 0},
> -       {"", 0},
> -};
> -MODULE_DEVICE_TABLE(acpi, device_ids);
> +MODULE_DEVICE_TABLE(acpi, asus_wireless_ids);
>

No, Don't do this.

>  static u64 asus_wireless_method(acpi_handle handle, const char *method,
>                                 int param)
> @@ -130,8 +127,8 @@ static int asus_wireless_add(struct acpi_device *adev)
>         adev->driver_data = data;
>
>         hid = acpi_device_hid(adev);
> -       for (i = 0; strcmp(device_ids[i].id, ""); i++) {

This is wrong.

> -               if (!strcmp(device_ids[i].id, hid)) {
> +       for (i = 0; strcmp(asus_wireless_ids[i].id, ""); i++) {

This is too.

Potential infinite loop.

On top of that seems you just introduced this by previous patches and
changing here.
Often it means you need to reconsider how you actually split the
series on logical pieces.

> +               if (!strcmp(asus_wireless_ids[i].id, hid)) {
>                         data->hswc_params = &id_params[i];
>                         break;
>                 }
> @@ -179,7 +176,7 @@ static int asus_wireless_remove(struct acpi_device *adev)
>  static struct acpi_driver asus_wireless_driver = {
>         .name = "Asus Wireless Radio Control Driver",
>         .class = "hotkey",
> -       .ids = device_ids,
> +       .ids = asus_wireless_ids,
>         .ops = {
>                 .add = asus_wireless_add,
>                 .remove = asus_wireless_remove,
> diff --git a/drivers/platform/x86/asus-wireless.h b/drivers/platform/x86/asus-wireless.h
> new file mode 100644
> index 000000000000..10a345863da6
> --- /dev/null
> +++ b/drivers/platform/x86/asus-wireless.h
> @@ -0,0 +1,10 @@
> +#ifndef _ASUS_WIRELESS_H_
> +#define _ASUS_WIRELESS_H_
> +
> +const struct acpi_device_id asus_wireless_ids[] = {
> +       {"ATK4001", 0},
> +       {"ATK4002", 0},
> +       {"", 0},
> +};
> +
> +#endif /* !_ASUS_WIRELESS_H_ */
> --
> 2.11.0
>



-- 
With Best Regards,
Andy Shevchenko