Re: [Nouveau] [PATCH v2 0/7] lib: string: add functions to case-convert strings

[Alexandre Courbot]

On Wed, Jul 6, 2016 at 7:56 AM, Joe Perches <joe@xxxxxxxxxxx> wrote:
> On Tue, 2016-07-05 at 15:36 -0700, Markus Mayer wrote:
>> On 5 July 2016 at 15:14, Joe Perches <joe@xxxxxxxxxxx> wrote:
>> > On Tue, 2016-07-05 at 13:47 -0700, Markus Mayer wrote:
>> > > This series introduces a family of generic string case conversion
>> > > functions. This kind of functionality is needed in several places in
>> > > the kernel. Right now, everybody seems to be implementing their own
>> > > copy of this functionality.
>> > >
>> > > Based on the discussion of the previous version of this series[1] and
>> > > the use cases found in the kernel, it does look like having several
>> > > flavours of case conversion functions is beneficial. The use cases fall
>> > > into three categories:
>> > >     - copying a string and converting the case while specifying a
>> > >       maximum length to mimic strncpy()
>> > >     - copying a string and converting the case without specifying a
>> > >       length to mimic strcpy()
>> > >     - converting the case of a string in-place (i.e. modifying the
>> > >       string that was passed in)
>> > >
>> > > Consequently, I am proposing these new functions:
>> > >     char *strncpytoupper(char *dst, const char *src, size_t len);
>> > >     char *strncpytolower(char *dst, const char *src, size_t len);
>> > >     char *strcpytoupper(char *dst, const char *src);
>> > >     char *strcpytolower(char *dst, const char *src);
>> > >     char *strtoupper(char *s);
>> > >     char *strtolower(char *s);
>> > I think there isn't much value in anything other
>> > than strto.
>> >
>> > Using str[n]cpy followed by strto is
>> > pretty obvious and rarely used anyway.
>> First time around, folks were proposing the "copy" variants when I
>> submitted just strtolower() by itself[1]. They just asked for source
>> and destination parameters to strtolower(), but looking at the use
>> cases that wouldn't have worked so well. Hence it evolved into these 6
>> functions.
>>
>> Here's a breakdown of how the functions are being used (patches 2-7),
>> see also [2]:
>>
>> Patch 2: strncpytolower()
>> Patch 3: strtolower()
>> Patch 4: strncpytolower() and strtolower()
>> Patch 5: strtolower()
>> Patch 6: strcpytoupper()
>> Patch 7: strcpytoupper()
>>
>> So it does look like the copy + change case variant is more frequently
>> used than just strto.
>
> Are these functions useful?  <shrug> Not to me, not so much.
>
> None of the functions would have the strcpy performance of
> the arch / asm
> versions of strcpy and the savings in overall
> code isn't significant (or
> measured?).
>
> Of course none of the uses are runtime performance important.

I tend to agree. strcpy is better left to architecture-specific code
when it exists. Then doing a strcpy() followed by strtolower() is not
exactly unintuitive. An explosion of closely related function is
certainly more confusing to me.

I'd just keep strtolower()/strtoupper() because they are commonly done
operations and we can probably save some space by having a unique
implementation. But going beyond that is overthinking the problem
IMHO.