On Sat, Jan 16, 2021 at 11:54 PM Barnabás Pőcze <pobrn@xxxxxxxxxxxxxx> wrote: > 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: ... > > > - 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. Ah, okay, good to know. I assumed that %i is decimal only. Thanks! > 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. If it's only 0 / 1 perhaps you may go further and make it bool (kstrtobool() API)? > [1]: https://wg14.link/c99 -- With Best Regards, Andy Shevchenko