Re: [PATCH 2/2] leds: ns2: Fix wrong boolean expression

[Pavel Machek <pavel@ucw.cz>]

[-- Attachment #1: Type: text/plain, Size: 1523 bytes --]

On Tue 2019-09-03 19:00:21, Oleh Kravchenko wrote:
> Hello Pavel,
> 
> 03.09.19 17:12, Pavel Machek пише:
> > On Tue 2019-09-03 15:50:20, Oleh Kravchenko wrote:
> >> Error was detected by PVS-Studio:
> >> V792 The '__gpio_cansleep' function located to the right of
> >> the operator '|' will be called regardless of the value of
> >> the left operand. Perhaps, it is better to use '||'.
> > 1st: original code is not wrong
> 
> '|' is bitwise operation, if it really means *OR* then should be '||' -
> led_dat->can_sleep is bool.

I see that || would be more natural. But | also works.

> > 2nd: you are introducing a bug
> 
> No, because if GPIOs *slow* can sleep and GPIO *cmd* can't sleep
> it will call gpio_set_value_cansleep() for both.

If just one of them can sleep, can_sleep will be 0, and bad things
will happen, right?


> >> @@ -205,7 +205,7 @@ create_ns2_led(struct platform_device *pdev, struct ns2_led_data *led_dat,
> >>  	led_dat->cdev.groups = ns2_led_groups;
> >>  	led_dat->cmd = template->cmd;
> >>  	led_dat->slow = template->slow;
> >> -	led_dat->can_sleep = gpio_cansleep(led_dat->cmd) |
> >> +	led_dat->can_sleep = gpio_cansleep(led_dat->cmd) &&
> >>  				gpio_cansleep(led_dat->slow);
> >>  	if (led_dat->can_sleep)
> >>  		led_dat->cdev.brightness_set_blocking = ns2_led_set_blocking;

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]