Re: [PATCH v3 0/7] lib: string: add functions to case-convert strings
On Wed, Jul 13, 2016 at 03:52:38PM -0700, Markus Mayer wrote:
> On Sat, Jul 09, 2016 at 09:11:05PM -0700, Markus Mayer wrote:
>> On 9 July 2016 at 20:13, Chris Metcalf wrote:
>>> On 7/8/2016 6:43 PM, 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 strlcpy()
- 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:
void strlcpytoupper(char *dst, const char *src, size_t len);
void strlcpytolower(char *dst, const char *src, size_t len);
void strcpytoupper(char *dst, const char *src);
void strcpytolower(char *dst, const char *src);
void strtoupper(char *s);
void strtolower(char *s);
>>>
>>>
>>> You may want to read the article here:
>>>
>>> https://lwn.net/Articles/659214/
>>
>> I'll read that. Thanks.
>
> It doesn't look like there is going to be the danger of "mass changes".
> So far, I have two ACKs (one where the semantics doesn't change,
> because it's using strtolower()) and the other in a driver in staging.
>
> But I understand the concern and will keep an eye out if there are
> other ACKs.
>
>>> and follow up some of the discussion threads on LKML about the best
>>> semantics to advertise for the strlcpy/strscpy variants. It might
>>> be helpful to return some kind of overflow/truncation error from
>>> your copy functions so people can error-check the result.
>>
>> I am inclined to agree. However, everybody has been telling me that
>> these functions should be void. Originally they weren't.
>
> What about something like this? It might also work to keep the four
> static inline functions as "void" (since they won't ever return E2BIG
> anyway) and just have strlcpyto* return an integer (since that's where
> the buffer could be too small).
>
> Rasmus, what's your take?
Ping. Any thoughts on this proposal? Does it make sense for me to sent out a
new revision of the patch set incorporating these changes -- at least for
the strlcpyto* functions?
Thanks,
-Markus
> diff --git a/include/linux/string.h b/include/linux/string.h
> index ae82d13..6cc85dc 100644
> --- a/include/linux/string.h
> +++ b/include/linux/string.h
> @@ -116,8 +116,8 @@ 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);
> -extern void strlcpytoupper(char *dst, const char *src, size_t len);
> -extern void strlcpytolower(char *dst, const char *src, size_t len);
> +extern int strlcpytoupper(char *dst, const char *src, size_t len);
> +extern int strlcpytolower(char *dst, const char *src, size_t len);
>
> extern void kfree_const(const void *x);
>
> @@ -175,38 +175,46 @@ static inline const char *kbasename(const char *path)
> * strcpytoupper - Copy string and convert to uppercase.
> * @dst: The buffer to store the result.
> * @src: The string to convert to uppercase.
> + *
> + * Returns the number of characters copied.
> */
> -static inline void strcpytoupper(char *dst, const char *src)
> +static inline int strcpytoupper(char *dst, const char *src)
> {
> - strlcpytoupper(dst, src, ~(size_t)0);
> + return strlcpytoupper(dst, src, ~(size_t)0);
> }
>
> /**
> * strcpytolower - Copy string and convert to lowercase.
> * @dst: The buffer to store the result.
> * @src: The string to convert to lowercase.
> + *
> + * Returns the number of characters copied.
> */
> -static inline void strcpytolower(char *dst, const char *src)
> +static inline int strcpytolower(char *dst, const char *src)
> {
> - strlcpytolower(dst, src, ~(size_t)0);
> + return strlcpytolower(dst, src, ~(size_t)0);
> }
>
> /**
> * strtoupper - Convert string to uppercase.
> * @s: The string to operate on.
> + *
> + * Returns the number of characters copied.
> */
> -static inline void strtoupper(char *s)
> +static inline int strtoupper(char *s)
> {
> - strlcpytoupper(s, s, ~(size_t)0);
> + return strlcpytoupper(s, s, ~(size_t)0);
> }
>
> /**
> * strtolower - Convert string to lowercase.
> * @s: The string to operate on.
> + *
> + * Returns the number of characters copied.
> */
> -static inline void strtolower(char *s)
> +static
Re: [PATCH v3 0/7] lib: string: add functions to case-convert strings
On Sat, Jul 09, 2016 at 09:11:05PM -0700, Markus Mayer wrote:
> On 9 July 2016 at 20:13, Chris Metcalf wrote:
> > On 7/8/2016 6:43 PM, 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 strlcpy()
> >> - 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:
> >> void strlcpytoupper(char *dst, const char *src, size_t len);
> >> void strlcpytolower(char *dst, const char *src, size_t len);
> >> void strcpytoupper(char *dst, const char *src);
> >> void strcpytolower(char *dst, const char *src);
> >> void strtoupper(char *s);
> >> void strtolower(char *s);
> >
> >
> > You may want to read the article here:
> >
> > https://lwn.net/Articles/659214/
>
> I'll read that. Thanks.
It doesn't look like there is going to be the danger of "mass changes".
So far, I have two ACKs (one where the semantics doesn't change,
because it's using strtolower()) and the other in a driver in staging.
But I understand the concern and will keep an eye out if there are
other ACKs.
> > and follow up some of the discussion threads on LKML about the best
> > semantics to advertise for the strlcpy/strscpy variants. It might
> > be helpful to return some kind of overflow/truncation error from
> > your copy functions so people can error-check the result.
>
> I am inclined to agree. However, everybody has been telling me that
> these functions should be void. Originally they weren't.
What about something like this? It might also work to keep the four
static inline functions as "void" (since they won't ever return E2BIG
anyway) and just have strlcpyto* return an integer (since that's where
the buffer could be too small).
Rasmus, what's your take?
diff --git a/include/linux/string.h b/include/linux/string.h
index ae82d13..6cc85dc 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -116,8 +116,8 @@ 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);
-extern void strlcpytoupper(char *dst, const char *src, size_t len);
-extern void strlcpytolower(char *dst, const char *src, size_t len);
+extern int strlcpytoupper(char *dst, const char *src, size_t len);
+extern int strlcpytolower(char *dst, const char *src, size_t len);
extern void kfree_const(const void *x);
@@ -175,38 +175,46 @@ static inline const char *kbasename(const char *path)
* strcpytoupper - Copy string and convert to uppercase.
* @dst: The buffer to store the result.
* @src: The string to convert to uppercase.
+ *
+ * Returns the number of characters copied.
*/
-static inline void strcpytoupper(char *dst, const char *src)
+static inline int strcpytoupper(char *dst, const char *src)
{
- strlcpytoupper(dst, src, ~(size_t)0);
+ return strlcpytoupper(dst, src, ~(size_t)0);
}
/**
* strcpytolower - Copy string and convert to lowercase.
* @dst: The buffer to store the result.
* @src: The string to convert to lowercase.
+ *
+ * Returns the number of characters copied.
*/
-static inline void strcpytolower(char *dst, const char *src)
+static inline int strcpytolower(char *dst, const char *src)
{
- strlcpytolower(dst, src, ~(size_t)0);
+ return strlcpytolower(dst, src, ~(size_t)0);
}
/**
* strtoupper - Convert string to uppercase.
* @s: The string to operate on.
+ *
+ * Returns the number of characters copied.
*/
-static inline void strtoupper(char *s)
+static inline int strtoupper(char *s)
{
- strlcpytoupper(s, s, ~(size_t)0);
+ return strlcpytoupper(s, s, ~(size_t)0);
}
/**
* strtolower - Convert string to lowercase.
* @s: The string to operate on.
+ *
+ * Returns the number of characters copied.
*/
-static inline void strtolower(char *s)
+static inline int strtolower(char *s)
{
- strlcpytolower(s, s, ~(size_t)0);
+ return strlcpytolower(s, s, ~(size_t)0);
}
#endif /* _LINUX_STRING_H_ */
diff --git a/lib/string.c b/lib/string.c
index 7903e10..d36d5fb2 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -958,17 +958,21 @@ EXPORT_SYMBOL(strreplace);
* @dst: The buffer to store the result.
* @src: The string to convert to uppercase.
*
Re: [PATCH v3 0/7] lib: string: add functions to case-convert strings
On 7/8/2016 6:43 PM, 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 strlcpy() - 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: void strlcpytoupper(char *dst, const char *src, size_t len); void strlcpytolower(char *dst, const char *src, size_t len); void strcpytoupper(char *dst, const char *src); void strcpytolower(char *dst, const char *src); void strtoupper(char *s); void strtolower(char *s); You may want to read the article here: https://lwn.net/Articles/659214/ and follow up some of the discussion threads on LKML about the best semantics to advertise for the strlcpy/strscpy variants. It might be helpful to return some kind of overflow/truncation error from your copy functions so people can error-check the result. -- Chris Metcalf, Mellanox Technologies http://www.mellanox.com -- 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 v3 0/7] lib: string: add functions to case-convert strings
On 9 July 2016 at 20:13, Chris Metcalf wrote: > On 7/8/2016 6:43 PM, 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 strlcpy() >> - 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: >> void strlcpytoupper(char *dst, const char *src, size_t len); >> void strlcpytolower(char *dst, const char *src, size_t len); >> void strcpytoupper(char *dst, const char *src); >> void strcpytolower(char *dst, const char *src); >> void strtoupper(char *s); >> void strtolower(char *s); > > > You may want to read the article here: > > https://lwn.net/Articles/659214/ I'll read that. Thanks. > and follow up some of the discussion threads on LKML about the best > semantics to advertise for the strlcpy/strscpy variants. It might be > helpful to return some kind of overflow/truncation error from your > copy functions so people can error-check the result. I am inclined to agree. However, everybody has been telling me that these functions should be void. Originally they weren't. 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
[PATCH v3 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 strlcpy() - 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: void strlcpytoupper(char *dst, const char *src, size_t len); void strlcpytolower(char *dst, const char *src, size_t len); void strcpytoupper(char *dst, const char *src); void strcpytolower(char *dst, const char *src); void strtoupper(char *s); void strtolower(char *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 v2: - use strlcpy() semantics not strncpy() semantics, i.e. guarantee NULL termination - as a result strncpyto are now called strlcpyto - make functions void - use len == -1 (SIZE_MAX) as no-limit indicator rather then len == 0 - change PATCH 2/7 to match strlcpy() semantics - change PATCH 4/7 to match strlcpy() semantics 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 to case-convert strings drm/nouveau/core: make use of new strlcpytolower() function ACPI / device_sysfs: make use of new strtolower() function staging: speakup: replace spk_strlwr() with strlcpytolower() iscsi-target: replace iscsi_initiatorname_tolower() with strtolower() 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 | 40 lib/string.c | 38 ++ 11 files changed, 90 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
