| Message ID | 20160713225236.GA30571@lbrmn-mmayer.ric.broadcom.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
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 <cmetcalf@mellanox.com> 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 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. > * @len: Maximum string length. May be SIZE_MAX to set no limit. > + * > + * Returns the number of characters copied or -E2BIG if @dst wasn't big enough. > */ > -void strlcpytoupper(char *dst, const char *src, size_t len) > +int strlcpytoupper(char *dst, const char *src, size_t len) > { > size_t i; > > if (!len) > - return; > + return -E2BIG; > > for (i = 0; i < len && src[i]; ++i) > dst[i] = toupper(src[i]); > dst[i < len ? i : i - 1] = '\0'; > + > + return (i < len) ? i : -E2BIG; > } > EXPORT_SYMBOL(strlcpytoupper); > > @@ -977,16 +981,20 @@ EXPORT_SYMBOL(strlcpytoupper); > * @dst: The buffer to store the result. > * @src: The string to convert to lowercase. > * @len: Maximum string length. May be SIZE_MAX to set no limit. > + * > + * Returns the number of characters copied or -E2BIG if @dst wasn't big enough. > */ > -void strlcpytolower(char *dst, const char *src, size_t len) > +int strlcpytolower(char *dst, const char *src, size_t len) > { > size_t i; > > if (!len) > - return; > + return -E2BIG; > > for (i = 0; i < len && src[i]; ++i) > dst[i] = tolower(src[i]); > dst[i < len ? i : i - 1] = '\0'; > + > + return (i < len) ? i : -E2BIG; > } > EXPORT_SYMBOL(strlcpytolower); -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. * @len: Maximum string length. May be SIZE_MAX to set no limit. + * + * Returns the number of characters copied or -E2BIG if @dst wasn't big enough. */ -void strlcpytoupper(char *dst, const char *src, size_t len) +int strlcpytoupper(char *dst, const char *src, size_t len) { size_t i; if (!len) - return; + return -E2BIG; for (i = 0; i < len && src[i]; ++i) dst[i] = toupper(src[i]); dst[i < len ? i : i - 1] = '\0'; + + return (i < len) ? i : -E2BIG; } EXPORT_SYMBOL(strlcpytoupper); @@ -977,16 +981,20 @@ EXPORT_SYMBOL(strlcpytoupper); * @dst: The buffer to store the result. * @src: The string to convert to lowercase. * @len: Maximum string length. May be SIZE_MAX to set no limit. + * + * Returns the number of characters copied or -E2BIG if @dst wasn't big enough. */ -void strlcpytolower(char *dst, const char *src, size_t len) +int strlcpytolower(char *dst, const char *src, size_t len) { size_t i; if (!len) - return; + return -E2BIG; for (i = 0; i < len && src[i]; ++i) dst[i] = tolower(src[i]); dst[i < len ? i : i - 1] = '\0'; + + return (i < len) ? i : -E2BIG; } EXPORT_SYMBOL(strlcpytolower);