On Mon, May 14, 2018 at 12:15:02PM +0200, Uwe Kleine-König wrote:
> As triggers are identified by name, two triggers must not share the same
> name. This is enforced at registration but must be done for renames,
> too.
>
> Fixes: a8df7b1ab70b ("leds: add led_trigger_rename function")
Not sure a fixes tag is warranted for the current single user which
calls this functions with what (I assume is) a unique netdev name as
base. So this is currently a non-issue.
And while not really your problem, having a fixes tag here may also cause
some bots to pick this up for stable. And especially as the impact of
this change is not made clear in the commit message.
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx>
> ---
> drivers/leds/led-triggers.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/drivers/leds/led-triggers.c b/drivers/leds/led-triggers.c
> index 431123b048a2..28e78697c63e 100644
> --- a/drivers/leds/led-triggers.c
> +++ b/drivers/leds/led-triggers.c
> @@ -177,10 +177,21 @@ EXPORT_SYMBOL_GPL(led_trigger_set_default);
>
> void led_trigger_rename_static(const char *name, struct led_trigger *trig)
> {
> + 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);
> +
> + list_for_each_entry(_trig, &trigger_list, next_trig) {
> + if (!strcmp(_trig->name, name)) {
> + up_write(&triggers_list_lock);
> + pr_warning("Cannot rename trigger, new name already exists\n");
> + return;
Changing the API here makes some sense (i.e. to have core rather than
the caller guarantee uniqueness), but then I think you should return an
errno as well to allow the caller to detect and bail out on errors.
Thanks,
Johan
