On Fri, 01 Jul 2016, Markus Mayer <markus.mayer@xxxxxxxxxxxx> wrote:
> On 1 July 2016 at 03:52, Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> wrote:
>> On Fri, 01 Jul 2016, Markus Mayer <mmayer@xxxxxxxxxxxx> wrote:
>>> Add a function called strtolower() to convert strings to lower case
>>> in-place, overwriting the original string.
>>>
>>> This seems to be a recurring requirement in the kernel that is
>>> currently being solved by several duplicated implementations doing the
>>> same thing.
>>>
>>> Signed-off-by: Markus Mayer <mmayer@xxxxxxxxxxxx>
>>> ---
>>> include/linux/string.h | 1 +
>>> lib/string.c | 14 ++++++++++++++
>>> 2 files changed, 15 insertions(+)
>>>
>>> diff --git a/include/linux/string.h b/include/linux/string.h
>>> index 26b6f6a..aad605e 100644
>>> --- a/include/linux/string.h
>>> +++ b/include/linux/string.h
>>> @@ -116,6 +116,7 @@ extern void * memchr(const void *,int,__kernel_size_t);
>>> #endif
>>> void *memchr_inv(const void *s, int c, size_t n);
>>> char *strreplace(char *s, char old, char new);
>>> +char *strtolower(char *s);
>>>
>>> extern void kfree_const(const void *x);
>>>
>>> diff --git a/lib/string.c b/lib/string.c
>>> index ed83562..6e3b560 100644
>>> --- a/lib/string.c
>>> +++ b/lib/string.c
>>> @@ -952,3 +952,17 @@ char *strreplace(char *s, char old, char new)
>>> return s;
>>> }
>>> EXPORT_SYMBOL(strreplace);
>>> +
>>
>> This needs a kernel-doc comment right here.
>
> Will add it.
>
>>> +char *strtolower(char *s)
>>> +{
>>> + char *p;
>>> +
>>> + if (unlikely(!s))
>>> + return NULL;
>>
>> Using spaces for indentation? See scripts/checkpatch.pl.
>
> Not on purpose. Thanks for spotting it.
>
>>> +
>>> + for (p = s; *p; p++)
>>> + *p = tolower(*p);
>>> +
>>> + return s;
>>
>> Why does it return a value? Could be void?
>
> It could be void, but I thought that would make the function's use
> less flexible. As is, the return value is there if anybody wants it,
> but it can be ignored if it is not needed. Also, it seems customary
> for string functions to be returning the string that was passed in.
>
> I'll change it to void if there are strong opinions leaning that way.
> Personally, I like that it returns a char * better.
I don't have strong opinions on this. Just a general aversion to
returning something redundant. Avoids questions like, does it allocate a
new string, should I use the return value instead of the string I passed
in, should I check the return value or can I ignore it, should I check
both the string I pass in and the return value for != NULL, etc. But I
could be persuaded either way.
BR,
Jani.
>
>> BR,
>> Jani.
>>
>>> +}
>>> +EXPORT_SYMBOL(strtolower);
>>
>> --
>> Jani Nikula, Intel Open Source Technology Center
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
