Hi 2021. január 16., szombat 20:52 keltezéssel, Andy Shevchenko írta: > On Wed, Jan 13, 2021 at 8:23 PM Barnabás Pőcze wrote: > > > > Do not handle zero length buffer separately. Use kstrtouint() instead > > of sscanf(). > > ... > > > - int ret, state; > > + int ret; > > struct ideapad_private *priv = dev_get_drvdata(dev); > > + unsigned int state; > > Reversed xmas tree order? > I'll change the order. > ... > > > - if (sscanf(buf, "%i", &state) != 1) > > - return -EINVAL; > > + ret = kstrtouint(buf, 0, &state); > > + if (ret) > > + return ret; > > This seems to me a relaxing case, and should be 10 instead of 0. Am I > right about %i? > If it's true it's probably minor, but still an ABI breakage. > According to the latest C99 draft[1] (7.19.6.2): [The 'i' format specifier] Matches an optionally signed integer, whose format is the same as expected for the subject sequence of the strtol function with the value 0 for the base argument. The corresponding argument shall be a pointer to signed integer. Skimming over `vsscanf()`, I'm fairly sure it implements the same behaviour. So '0' as the base is correct, I believe. But technically it's still an ABI breakage since negative numbers are no longer accepted. In the case of `store_ideapad_fan()` it changes nothing since there was bounds checking in place. In the case of `store_ideapad_cam()` negative numbers are now rejected. I'm not sure if this change should be of great concern, since the the documentation only mentions '0' and '1', and I would be surprised if anyone used this interface to send negative numbers to the embedded controller. [1]: https://wg14.link/c99 Regards, Barnabás Pőcze
