leds: add led_trigger_rename function
PATCH RESEND v2 1/1 leds: add led_trigger_rename function
Implements a "led_trigger_rename" function to rename a trigger with proper locking.
This assumes that led name was originally allocated in non-constant storage.
---
Hi Bryan,
I'm resending this one as it was probably lost it in the mail transition.
Fabio
drivers/leds/led-triggers.c | 13 +++++++++++++ include/linux/leds.h | 18 ++++++++++++++++++ 2 files changed, 31 insertions(+)
drivers/leds/led-triggers.c +13 -0
@@ -166,6 +166,19 @@ void led_trigger_set_default(struct led_classdev *led_cdev) }EXPORT_SYMBOL_GPL (led_trigger_set_default); +void led_trigger_rename_static (const char *name,struct led_trigger *trig) +{ +/* new name must be on a temporary string to prevent races */ +BUG_ON (name == trig->name); + +down_write (&triggers_list_lock); +/* this assumes that trig->name was originaly allocated to +* non constant storage */ +strcpy ((char *)trig->name, name); +up_write (&triggers_list_lock); +} +EXPORT_SYMBOL_GPL (led_trigger_rename_static); +/* LED Trigger Interface */ int led_trigger_register (struct led_trigger *trig)
include/linux/leds.h +18 -0
@@ -139,6 +139,24 @@ extern void led_blink_set_oneshot(struct led_classdev *led_cdev,extern void led_set_brightness (struct led_classdev *led_cdev,enum led_brightness brightness); +/** +* led_trigger_rename_static - rename a trigger +* @name: the new trigger name +* @trig: the LED trigger to rename +* +* Change a LED trigger name by copying the string passed in +* name into current trigger name, which MUST be large +* enough for the new string. +* +* Note that name must NOT point to the same string used +* during LED registration, as that could lead to races. +* +* This is meant to be used on triggers with statically +* allocated name. +*/ +extern void led_trigger_rename_static (const char *name, +struct led_trigger *trig); +/* * LED Triggers */
Signature · 1 line
1.7.12.1
Quoted 28 lines · Fabio Baltieri
Implements a "
led_trigger_rename" function to rename a trigger with proper locking.This assumes that led name was originally allocated in non-constant storage.
Signed-off-by Fabio Baltieri <[email protected]>Cc Kurt Van Dijck <[email protected]>Cc Bryan Wu <[email protected]>---
Hi Bryan,
I'm resending this one as it was probably lost it in the mail transition.
Fabio
drivers/leds/led-triggers.c | 13 +++++++++++++ include/linux/leds.h | 18 ++++++++++++++++++ 2 files changed, 31 insertions(+)drivers/leds/led-triggers.c +0
-0@@ -166,6 +166,19 @@ void led_trigger_set_default(struct led_classdev *led_cdev) }EXPORT_SYMBOL_GPL (led_trigger_set_default);
drivers/leds/led-triggers.c void led_trigger_set_default(struct led_classdev *led_cdev) · line 177
Quoted 10 lines
drivers/leds/led-triggers.c +9
-0+void led_trigger_rename_static (const char *name,struct led_trigger *trig) +{ +/* new name must be on a temporary string to prevent races */ +BUG_ON (name == trig->name); + +down_write (&triggers_list_lock); +/* this assumes that trig->name was originaly allocated to +* non constant storage */ +strcpy ((char *)trig->name, name);
Is this strcpy() safe here? Probably strncpy() or strlcpy() is safer.
Quoted 23 lines
drivers/leds/led-triggers.c +4
-0+up_write (&triggers_list_lock); +} +EXPORT_SYMBOL_GPL (led_trigger_rename_static); +/* LED Trigger Interface */ int led_trigger_register (struct led_trigger *trig)include/linux/leds.h +8
-0@@ -139,6 +139,24 @@ extern void led_blink_set_oneshot(struct led_classdev *led_cdev,extern void led_set_brightness (struct led_classdev *led_cdev,enum led_brightness brightness); +/** +* led_trigger_rename_static - rename a trigger +* @name: the new trigger name +* @trig: the LED trigger to rename +* +* Change a LED trigger name by copying the string passed in +* name into current trigger name, which MUST be large +* enough for the new string.
include/linux/leds.h extern void led_blink_set_oneshot(struct led_classdev *led_cdev, · line 159
Quoted 10 lines
include/linux/leds.h +10
-0+* +* Note that name must NOT point to the same string used +* during LED registration, as that could lead to races. +* +* This is meant to be used on triggers with statically +* allocated name. +*/ +extern void led_trigger_rename_static (const char *name, +struct led_trigger *trig); +
Any example how to use this new API?
Thanks,
-Bryan
Quoted 6 lines
include/linux/leds.h +0
-1/* * LED Triggers */ --1.7.12.1
Hi Bryan,
Quoted 11 lines · Bryan Wu
drivers/leds/led-triggers.c void led_trigger_set_default(struct led_classdev *led_cdev) · line 177
Quoted 9 lines
drivers/leds/led-triggers.c +9
-0+void led_trigger_rename_static (const char *name,struct led_trigger *trig) +{ +/* new name must be on a temporary string to prevent races */ +BUG_ON (name == trig->name); + +down_write (&triggers_list_lock); +/* this assumes that trig->name was originaly allocated to +* non constant storage */ +strcpy ((char *)trig->name, name);Is this
strcpy()safe here? Probablystrncpy()orstrlcpy()is safer.
Actually the LED subsystem is not aware of the string allocation size, so I guess that strcpy is the only option here.
On the other side, the caller, who originally allocated the string, should do the check properly, such as in:
snprintf(name, sizeof(name), "%s-tx", netdev->name); led_trigger_rename_static(name, priv->tx_led_trig);
Quoted 5 lines
include/linux/leds.h extern void led_blink_set_oneshot(struct led_classdev *led_cdev, · line 159
Quoted 3 lines
include/linux/leds.h +3
-0+extern void led_trigger_rename_static (const char *name, +struct led_trigger *trig); +Any example how to use this new API?
Sure! That was developed as part of CANBUS LED triggers to have trigger name follow can interfaces name changes.
Original patch using this function, including the whole discussion behind it, was posted here:
https://lkml.org/lkml/2012/9/10/544
or you can find the complete set on my github branch:
http://github.com/fabiobaltieri/linux.git can-leds-devel
Fabio
Signature · 1 line
Fabio Baltieri
OK, I will apply this patch to my tree.
Thanks,
-Bryan
Quoted 46 lines · Fabio Baltieri
Hi Bryan,
Quoted 11 lines · Bryan Wu
drivers/leds/led-triggers.c void led_trigger_set_default(struct led_classdev *led_cdev) · line 177
Quoted 9 lines
drivers/leds/led-triggers.c +9
-0+void led_trigger_rename_static (const char *name,struct led_trigger *trig) +{ +/* new name must be on a temporary string to prevent races */ +BUG_ON (name == trig->name); + +down_write (&triggers_list_lock); +/* this assumes that trig->name was originaly allocated to +* non constant storage */ +strcpy ((char *)trig->name, name);Is this
strcpy()safe here? Probablystrncpy()orstrlcpy()is safer.Actually the LED subsystem is not aware of the string allocation size, so I guess that strcpy is the only option here.
On the other side, the caller, who originally allocated the string, should do the check properly, such as in:
snprintf(name, sizeof(name), "%s-tx", netdev->name); led_trigger_rename_static(name, priv->tx_led_trig);Quoted 5 lines
include/linux/leds.h extern void led_blink_set_oneshot(struct led_classdev *led_cdev, · line 159
Quoted 3 lines
include/linux/leds.h +3
-0+extern void led_trigger_rename_static (const char *name, +struct led_trigger *trig); +Any example how to use this new API?
Sure! That was developed as part of CANBUS LED triggers to have trigger name follow can interfaces name changes.
Original patch using this function, including the whole discussion behind it, was posted here:
https://lkml.org/lkml/2012/9/10/544
or you can find the complete set on my github branch:
http://github.com/fabiobaltieri/linux.git can-leds-devel
Fabio
--
Fabio Baltieri