Re: [PATCH] staging: greybus: fix get_channel_from_mode() failure path

[Alex Elder]

On 3/4/24 3:17 AM, Rui Miguel Silva wrote:
> Hi Dan,
> once again thanks for the patch.
>
> Dan Carpenter <dan.carpenter@xxxxxxxxxx> writes:
>
> > The get_channel_from_mode() function is supposed to return the channel
> > which matches the mode.  But it has a bug where if it doesn't find a
> > matching channel then it returns the last channel.  It should return
> > NULL instead.
> >
> > Also remove an unnecessary NULL check on "channel".
> >
> > Fixes: 2870b52bae4c ("greybus: lights: add lights implementation")
> > Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
>
> Reviewed-by: Rui Miguel Silva <rmfrfs@xxxxxxxxx>

Looks good.

Reviewed-by: Alex Elder <elder@xxxxxxxxxx>

> Cheers,
>    Rui
>
> > ---
> >   drivers/staging/greybus/light.c | 8 ++++----
> >   1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/staging/greybus/light.c b/drivers/staging/greybus/light.c
> > index d62f97249aca..a5c2fe963866 100644
> > --- a/drivers/staging/greybus/light.c
> > +++ b/drivers/staging/greybus/light.c
> > @@ -95,15 +95,15 @@ static struct led_classdev *get_channel_cdev(struct gb_channel *channel)
> >   static struct gb_channel *get_channel_from_mode(struct gb_light *light,
> >   						u32 mode)
> >   {
> > -	struct gb_channel *channel = NULL;
> > +	struct gb_channel *channel;
> >   	int i;
> >   
> >   	for (i = 0; i < light->channels_count; i++) {
> >   		channel = &light->channels[i];
> > -		if (channel && channel->mode == mode)
> > -			break;
> > +		if (channel->mode == mode)
> > +			return channel;
> >   	}
> > -	return channel;
> > +	return NULL;
> >   }
> >   
> >   static int __gb_lights_flash_intensity_set(struct gb_channel *channel,
> > --
> > 2.43.0