* [RFC PATCH] leds: core: Fix LED_COLOR_MAX_ID
@ 2019-10-02 16:34 Dan Murphy
2019-10-02 18:36 ` Pavel Machek
0 siblings, 1 reply; 3+ messages in thread
From: Dan Murphy @ 2019-10-02 16:34 UTC (permalink / raw)
To: jacek.anaszewski, pavel; +Cc: linux-leds, linux-kernel, Dan Murphy
The LED_COLOR_MAX_ID is incorrect. THe MAX_ID should
be the last COLOR_ID in the list. If an array was allocate
with MAX_ID the allocation would be correct but the meaning
is wrong.
So for array allocation the code should use LED_NUM_COLOR_IDS
which will allocate MAX_ID + 1. Whent the code needs to validate
that the color ID is not out of bounds then the code should use
LED_COLOR_MAX_ID.
Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
drivers/leds/led-core.c | 4 ++--
drivers/leds/leds.h | 2 +-
include/dt-bindings/leds/common.h | 3 ++-
3 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
index f1f718dbe0f8..4e57d47c97e0 100644
--- a/drivers/leds/led-core.c
+++ b/drivers/leds/led-core.c
@@ -25,7 +25,7 @@ EXPORT_SYMBOL_GPL(leds_list_lock);
LIST_HEAD(leds_list);
EXPORT_SYMBOL_GPL(leds_list);
-const char * const led_colors[LED_COLOR_ID_MAX] = {
+const char * const led_colors[LED_NUM_COLOR_IDS] = {
[LED_COLOR_ID_WHITE] = "white",
[LED_COLOR_ID_RED] = "red",
[LED_COLOR_ID_GREEN] = "green",
@@ -385,7 +385,7 @@ static void led_parse_fwnode_props(struct device *dev,
ret = fwnode_property_read_u32(fwnode, "color", &props->color);
if (ret)
dev_err(dev, "Error parsing 'color' property (%d)\n", ret);
- else if (props->color >= LED_COLOR_ID_MAX)
+ else if (props->color > LED_COLOR_ID_MAX)
dev_err(dev, "LED color identifier out of range\n");
else
props->color_present = true;
diff --git a/drivers/leds/leds.h b/drivers/leds/leds.h
index 2d9eb48bbed9..2c493efc8f55 100644
--- a/drivers/leds/leds.h
+++ b/drivers/leds/leds.h
@@ -33,6 +33,6 @@ ssize_t led_trigger_write(struct file *filp, struct kobject *kobj,
extern struct rw_semaphore leds_list_lock;
extern struct list_head leds_list;
extern struct list_head trigger_list;
-extern const char * const led_colors[LED_COLOR_ID_MAX];
+extern const char * const led_colors[LED_NUM_COLOR_IDS];
#endif /* __LEDS_H_INCLUDED */
diff --git a/include/dt-bindings/leds/common.h b/include/dt-bindings/leds/common.h
index 9e1256a7c1bf..ce82f0b5f6d6 100644
--- a/include/dt-bindings/leds/common.h
+++ b/include/dt-bindings/leds/common.h
@@ -29,7 +29,8 @@
#define LED_COLOR_ID_VIOLET 5
#define LED_COLOR_ID_YELLOW 6
#define LED_COLOR_ID_IR 7
-#define LED_COLOR_ID_MAX 8
+#define LED_COLOR_ID_MAX LED_COLOR_ID_IR
+#define LED_NUM_COLOR_IDS (LED_COLOR_ID_MAX + 1)
/* Standard LED functions */
#define LED_FUNCTION_ACTIVITY "activity"
--
2.22.0.214.g8dca754b1e
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [RFC PATCH] leds: core: Fix LED_COLOR_MAX_ID
2019-10-02 16:34 [RFC PATCH] leds: core: Fix LED_COLOR_MAX_ID Dan Murphy
@ 2019-10-02 18:36 ` Pavel Machek
2019-10-02 18:42 ` Dan Murphy
0 siblings, 1 reply; 3+ messages in thread
From: Pavel Machek @ 2019-10-02 18:36 UTC (permalink / raw)
To: Dan Murphy; +Cc: jacek.anaszewski, linux-leds, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 745 bytes --]
On Wed 2019-10-02 11:34:00, Dan Murphy wrote:
> The LED_COLOR_MAX_ID is incorrect. THe MAX_ID should
> be the last COLOR_ID in the list. If an array was allocate
> with MAX_ID the allocation would be correct but the meaning
> is wrong.
>
> So for array allocation the code should use LED_NUM_COLOR_IDS
> which will allocate MAX_ID + 1. Whent the code needs to validate
> that the color ID is not out of bounds then the code should use
> LED_COLOR_MAX_ID.
Renaming original define might have been okay. Having two defines is
not. I'd say we can keep it as is...
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [RFC PATCH] leds: core: Fix LED_COLOR_MAX_ID
2019-10-02 18:36 ` Pavel Machek
@ 2019-10-02 18:42 ` Dan Murphy
0 siblings, 0 replies; 3+ messages in thread
From: Dan Murphy @ 2019-10-02 18:42 UTC (permalink / raw)
To: Pavel Machek; +Cc: jacek.anaszewski, linux-leds, linux-kernel
Hello
On 10/2/19 1:36 PM, Pavel Machek wrote:
> On Wed 2019-10-02 11:34:00, Dan Murphy wrote:
>> The LED_COLOR_MAX_ID is incorrect. THe MAX_ID should
>> be the last COLOR_ID in the list. If an array was allocate
>> with MAX_ID the allocation would be correct but the meaning
>> is wrong.
>>
>> So for array allocation the code should use LED_NUM_COLOR_IDS
>> which will allocate MAX_ID + 1. Whent the code needs to validate
>> that the color ID is not out of bounds then the code should use
>> LED_COLOR_MAX_ID.
> Renaming original define might have been okay. Having two defines is
> not. I'd say we can keep it as is...
OK. It was just not logical that MAX_ID will always be 1 more then the
actual MAX_ID.
So every ID boundary check will need to be ">=" as opposed to ">" which
means we have to take care in reviews to make sure this is what is intended.
But it was just a RFC so I am not pushing this fix. It would mean I
would have to re-touch the MC framework patches.
Dan
>
> Pavel
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-10-02 18:41 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-02 16:34 [RFC PATCH] leds: core: Fix LED_COLOR_MAX_ID Dan Murphy
2019-10-02 18:36 ` Pavel Machek
2019-10-02 18:42 ` Dan Murphy
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).