Hi Bjorn, [...] > > A common use case for many PCI sysfs objects is to either enable some > > functionality or trigger an action following a write to a given > > attribute where the value is written would be simply either "1" or "0" > > synonymous with either disabling or enabling something. > > > > Parsing and validation of the input values are currently done using the > > kstrtoul() function to convert anything in the string buffer into an > > integer value - anything non-zero would be accepted as "enable" and zero > > simply as "disable". > > > > For a while now, the kernel offers another function called kstrtobool() > > which was created to parse common user inputs into a boolean value, so > > that a range of values such as "y", "n", "1", "0", "on" and "off" > > handled in a case-insensitive manner would yield a boolean true or false > > result accordingly after the input string has been parsed. > > > > Thus, move to kstrtobool() over kstrtoul() as it's a better fit for > > parsing user input, and it also implicitly offers a range check as only > > a finite amount of possible input values will be considered as valid. > > If I understand correctly, a user could enable things by writing "5" > today, and if we switch to kstrbool(), that will no longer work. Correct. After this change only the range values kstrtobool() allows would be permitted, thus the ability to enable something (or trigger an action) simply through the virtue of sending a non-zero value to a particular sysfs attribute would not longer work. > I'm sure everything is *documented* such that one should write "1" or > other sensible values. We document handling on non-zero values for the "remove" sysfs attribute, but the user might indeed make identical assumption for other attributes, aside of assuming that using 1 and 0 would always work, which also has become customary for /proc and /sys and is now part of the canon, so to speak. > But I'm not sure there's benefit in adding new restrictions. I personally would welcome API update and adding consistency that kstrtobool() offers, but we can keep existing behaviour so that there aren't any surprises in the userspace. I will drop this patch in the next version. Krzysztof
