Pavel, > On Sep 3, 2019, at 9:50 PM, Pavel Machek <pavel@xxxxxx> wrote: > > 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? With current code if just one of them can sleep, can_sleep will be not zero (!= 0). And bad things will happen. > > >>>> @@ -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
