Re: [Nouveau] [PATCH v2 0/7] lib: string: add functions to case-convert strings
On Wed, Jul 6, 2016 at 7:56 AM, Joe Perches wrote: > On Tue, 2016-07-05 at 15:36 -0700, Markus Mayer wrote: >> On 5 July 2016 at 15:14, Joe Perches 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? 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. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/7] lib: string: add functions to case-convert strings
On 5 July 2016 at 15:56, Joe Perches wrote: > On Tue, 2016-07-05 at 15:36 -0700, Markus Mayer wrote: >> On 5 July 2016 at 15:14, Joe Perches 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? Not to me, not so much. The use cases do exist. I'll leave it up to the maintainers to decide whether duplicate implementations or potentially unused generic functions are to be preferred. What I do know is that I have a driver in the wings that also needs a strolower() implementation. If this ends up not being accepted, I'll have to add yet another driver-local strtolower() implementation to the kernel. But if that's the decision then so be it. > 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?). 100% agreed. These functions won't set any speed records. Keep in mind also that in 4 out of the 6 cases where I replaced local implementations of "strcpytoXXX", the code was doing essentially the same I am proposing here (no assembly code). Only 2 of the 6 called strncpy() before, benefiting from optimized assembly implementations. But they still had to walk the string explicitly afterwards to convert the case, which probably means the overall speed won't change that much using the functions proposed here. > Of course none of the uses are runtime performance important. That is also very true. > This patch also adds always compiled functions that aren't used > in many .configs. It adds 2 functions (strncpytolower() and strncpytoupper()). The other 4 are static inline one-liners and won't show up anywhere if not used (and if used the compiler will insert calls to strncpyto instead). Regards, -Markus -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/7] lib: string: add functions to case-convert strings
On Tue, 2016-07-05 at 15:36 -0700, Markus Mayer wrote: > On 5 July 2016 at 15:14, Joe Perches 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? 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. This patch also adds always compiled functions that aren't used in many .configs. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/7] lib: string: add functions to case-convert strings
On 5 July 2016 at 15:14, Joe Perches 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. Regards, -Markus [1] https://lkml.org/lkml/2016/7/1/652 [2] https://lkml.org/lkml/2016/7/5/542 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/7] lib: string: add functions to case-convert strings
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. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/7] lib: string: add functions to case-convert strings
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); They all return a pointer to the terminating '\0' in the destination string (for strtoupper() and strtolower() that is "s"). Several drivers are being modified to make use of the functions above. Another driver that also makes use of this functionality will be submitted upstream shortly, which prompted this whole exercise. The changes made here have been compile-tested, but not tried out, due to lack of required hardware. Changes since v1: - expanded strtolower() into a family of functions that cover use cases when a length argument is or isn't required and that support copying the string into a new buffer or changing it in-place - changed the function semantics to return a pointer to the terminating '\0' character of the modified string - added strtoupper() functionality mirroring the above - dropped the ACPICA patch, since that code is OS independent and can't rely on a Linux library function (see [2]) - Added two new patches replacing strtoupper() implementations [1] https://lkml.org/lkml/2016/6/30/727 [2] https://lkml.org/lkml/2016/7/1/9 Markus Mayer (7): lib: string: add functions str[n]cpytolower()/str[n]cpytoupper() drm/nouveau/core: make use of new strncpytolower() function ACPI / device_sysfs: make use of new strtolower() function staging: speakup: replace spk_strlwr() with strncpytolower() iscsi-target: replace iscsi_initiatorname_tolower() with strcpytolower() drm/nouveau/fifo/gk104: make use of new strcpytoupper() function power_supply: make use of new strcpytoupper() function drivers/acpi/device_sysfs.c | 4 +- drivers/gpu/drm/nouveau/nvkm/core/firmware.c | 9 + drivers/gpu/drm/nouveau/nvkm/engine/fifo/gk104.c | 5 +-- drivers/power/power_supply_sysfs.c | 13 +++ drivers/staging/speakup/kobjects.c | 3 +- drivers/staging/speakup/main.c | 3 +- drivers/staging/speakup/speakup.h| 1 - drivers/staging/speakup/varhandlers.c| 12 -- drivers/target/iscsi/iscsi_target_nego.c | 17 + include/linux/string.h | 48 lib/string.c | 40 11 files changed, 100 insertions(+), 55 deletions(-) -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
