Messages in this thread | | | Date | Wed, 23 Mar 2011 16:37:40 +0000 | From | Jonathan Cameron <> | Subject | Re: [PATCH 1/3] Add a kstrtobool function matching semantics of existing in kernel equivalents. |
| |
On 03/23/11 16:23, Greg KH wrote: > On Wed, Mar 23, 2011 at 04:15:51PM +0000, Jonathan Cameron wrote: >> On 03/23/11 16:01, Greg KH wrote: >>> On Wed, Mar 23, 2011 at 05:30:11PM +0200, Alexey Dobriyan wrote: >>>> On Wed, Mar 23, 2011 at 3:39 PM, Jonathan Cameron <jic23@cam.ac.uk> wrote: >>>>> +int kstrtobool(const char *s, bool *res) >>>>> +{ >>>>> + switch (s[0]) { >>>>> + case 'y': >>>>> + case 'Y': >>>>> + case '1': >>>>> + *res = true; >>>>> + case 'n': >>>>> + case 'N': >>>>> + case '0': >>>>> + *res = false; >>>>> + default: >>>>> + return -EINVAL; >>>>> + } >>>>> + return 0; >>>>> +} >>>> >>>> sigh... such simple thing and so many bugs >> Yeah, not by best work. >>>> >>>> The only values such function should accept is 0 and 1. >>> >>> Why? That's not the way the existing kernel functions that use this >>> work. >>> >>>> Have you read the rest of kstrto*() code? >>>> Where is newline check? >> There are plenty of nastier cases that get through than a newline >> in the middle of the string (ybobsyouruncle -> 1 nyes->0 :) >>>> >>>> Anyway, I think it's better do not exist. >>> >>> I think it is, as it's already duplicated in at least 2 different places >>> in the kernel, and probably more. Once we get this implementation >>> working correctly, we don't need to rewrite it again. >> Perhaps naming it like this is a bad idea. It manages to imply that it >> has the same level of strict checking which is seen in the other kstrto* >> functions - which is self evidently not true! > > Ok, perhaps the name might need to be changed a bit, but the idea is > still good to have. Please try again. > Any thoughts on what naming would work? Nothing immediately comes to mind which is why I ended up pinching the kstrto* naming...
reallysloppy_strtobool? guessintent_strtobool?
or the old classic underscore prefix to 'encourage' people to check what it does before using it
__strtobool?
| |