netdev - Re: [PATCH 26/31] net: bluetooth: changing LED_* from enum led

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]

Message-Id: <AB5BDE93-D298-4EB8-91FA-634910C86308@holtmann.org>
Date:   Fri, 21 Jan 2022 21:19:45 +0100
From:   Marcel Holtmann <marcel@...tmann.org>
To:     Luiz Sampaio <sampaio.ime@...il.com>
Cc:     Johan Hedberg <johan.hedberg@...il.com>,
        Luiz Augusto von Dentz <luiz.dentz@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>, linux-kernel@...r.kernel.org,
        linux-bluetooth@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: [PATCH 26/31] net: bluetooth: changing LED_* from enum
 led_brightness to actual value

Hi Luiz,

> The enum led_brightness, which contains the declaration of LED_OFF,
> LED_ON, LED_HALF and LED_FULL is obsolete, as the led class now supports
> max_brightness.
> ---
> net/bluetooth/leds.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/net/bluetooth/leds.c b/net/bluetooth/leds.c
> index f46847632ffa..7f7e3eed9407 100644
> --- a/net/bluetooth/leds.c
> +++ b/net/bluetooth/leds.c
> @@ -22,7 +22,7 @@ void hci_leds_update_powered(struct hci_dev *hdev, bool enabled)
> {
> 	if (hdev->power_led)
> 		led_trigger_event(hdev->power_led,
> -				  enabled ? LED_FULL : LED_OFF);
> +				  enabled ? 255 : 0);
> 
> 	if (!enabled) {
> 		struct hci_dev *d;
> @@ -37,7 +37,7 @@ void hci_leds_update_powered(struct hci_dev *hdev, bool enabled)
> 		read_unlock(&hci_dev_list_lock);
> 	}
> 
> -	led_trigger_event(bt_power_led_trigger, enabled ? LED_FULL : LED_OFF);
> +	led_trigger_event(bt_power_led_trigger, enabled ? 255 : 0);
> }
> 
> static int power_activate(struct led_classdev *led_cdev)
> @@ -48,7 +48,7 @@ static int power_activate(struct led_classdev *led_cdev)
> 	htrig = to_hci_basic_led_trigger(led_cdev->trigger);
> 	powered = test_bit(HCI_UP, &htrig->hdev->flags);
> 
> -	led_trigger_event(led_cdev->trigger, powered ? LED_FULL : LED_OFF);
> +	led_trigger_event(led_cdev->trigger, powered ? 255 : 0);

how is this leading to more readable code?

What is wrong with providing proper constants to use instead of funky numbers now. I
think that changing every user of LED_FULL etc. and replacing it with 255 is stupid.

Seriously, add a #define LED_FULL 255 and leave the users alone.

Regards

Marcel