Re: [PATCH 14/21] leds: flash: Avoid divide-by-zero in led_clamp_align()

[Lee Jones <lee@kernel.org>]

On Thu, 22 Oct 2026, David Laight wrote:

> On Wed, 21 Oct 2026 12:32:23 +0000
> Lee Jones <lee@kernel.org> wrote:
> 
> > If a flash LED driver leaves a setting's step size as zero, writing to
> > the corresponding sysfs attribute invokes led_clamp_align() and triggers
> > an integer division by zero when aligning the offset to s->step.
> > 
> > Clamp s->val to [s->min, s->max] and return early when s->step is zero.
> > 
> > Fixes: 7aea8389a77a ("leds: Add LED Flash class extension to the LED subsystem")
> > Closes: https://sashiko.dev/#/bug/linux-9d06d7fe-310b-4aee-8b16-0fb49550c287
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Lee Jones <lee@kernel.org>
> > ---
> >  drivers/leds/led-class-flash.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/drivers/leds/led-class-flash.c b/drivers/leds/led-class-flash.c
> > index 181dfd49c6ad..16b3b6b9365b 100644
> > --- a/drivers/leds/led-class-flash.c
> > +++ b/drivers/leds/led-class-flash.c
> > @@ -375,6 +375,11 @@ static void led_clamp_align(struct led_flash_setting *s)
> >  {
> >  	u32 v, offset;
> >  
> > +	if (!s->step) {
> > +		s->val = clamp(s->val, s->min, s->max);
> > +		return;
> > +	}
> 
> Why not just:
> 	u32 step = s->step ?: 1;
> and then use step instead of s->step.

I can see where you're coming from, but I don't think this is better.

With `step = 1`, the reader has to mentally trace `1 / 2 == 0` and `1 *
(offset / 1) == offset` to see that it reduces to a plain clamp and the
CPU still executes a redundant integer division.

IMHO, simple, easy-to-read, maintainable code wins out over clever code.

> > +
> >  	v = s->val + s->step / 2;
> >  	v = clamp(v, s->min, s->max);
> >  	offset = v - s->min;
> 

-- 
Lee Jones