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(+)
diff --git a/drivers/leds/led-triggers.c b/drivers/leds/led-triggers.c
index 262eb41..a4fa4bf 100644
--- a/drivers/leds/led-triggers.c
+++ b/drivers/leds/led-triggers.c
@@ -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)
diff --git a/include/linux/leds.h b/include/linux/leds.h
index 6e53bb3..8107592 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -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
*/
--
1.7.12.1
On Sun, Nov 4, 2012 at 1:54 AM, Fabio Baltieri <[email protected]> wrote:
> 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(+)
>
> diff --git a/drivers/leds/led-triggers.c b/drivers/leds/led-triggers.c
> index 262eb41..a4fa4bf 100644
> --- a/drivers/leds/led-triggers.c
> +++ b/drivers/leds/led-triggers.c
> @@ -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);
Is this strcpy() safe here? Probably strncpy() or strlcpy() is safer.
> + up_write(&triggers_list_lock);
> +}
> +EXPORT_SYMBOL_GPL(led_trigger_rename_static);
> +
> /* LED Trigger Interface */
>
> int led_trigger_register(struct led_trigger *trig)
> diff --git a/include/linux/leds.h b/include/linux/leds.h
> index 6e53bb3..8107592 100644
> --- a/include/linux/leds.h
> +++ b/include/linux/leds.h
> @@ -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);
> +
Any example how to use this new API?
Thanks,
-Bryan
> /*
> * LED Triggers
> */
> --
> 1.7.12.1
>
Hi Bryan,
On Tue, Nov 13, 2012 at 04:33:14PM -0800, Bryan Wu wrote:
> > +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.
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);
> > +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
OK, I will apply this patch to my tree.
Thanks,
-Bryan
On Wed, Nov 14, 2012 at 12:30 AM, Fabio Baltieri
<[email protected]> wrote:
> Hi Bryan,
>
> On Tue, Nov 13, 2012 at 04:33:14PM -0800, Bryan Wu wrote:
>> > +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.
>
> 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);
>
>> > +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