| Message ID | 1391116318-17253-2-git-send-email-sebastian.capella@linaro.org |
|---|---|
| State | New |
| Headers | show |
Quoting Andrew Morton (2014-01-30 13:22:51) > On Thu, 30 Jan 2014 13:11:57 -0800 Sebastian Capella <sebastian.capella@linaro.org> wrote: > > +char *kstrimdup(const char *s, gfp_t gfp) > > +{ > > + char *buf; > > + char *begin = skip_spaces(s); > > + size_t len = strlen(begin); > > + > > + while (len > 1 && isspace(begin[len - 1])) > > + len--; > > That's off-by-one isn't it? kstrimdup(" ") should return "", not " ". > > > + buf = kmalloc_track_caller(len + 1, gfp); > > + if (!buf) > > + return NULL; > > + > > + memcpy(buf, begin, len); > > + buf[len] = '\0'; > > + > > + return buf; > > +} Hi Andrew, I think this is a little tricky. For an empty string, the function relies on skip_spaces to point begin at the \0'. Alternately, if we don't have an empty string, we know we have at least 1 non-space, non-null character at begin[0], and there's no need to check it, so the loop stops at [1]. If there's a space at 1, we just put the '\0' there. We could check at [0], but I think its already been checked by skip_spaces. I'll add a comment above the while for that Thanks, Sebastian
I tested it and saw it working for those cases. In my comments earlier, I thought Joe was underrunning the buffer, but that wasn't the case. I like Joe's version better as there's no trick there and it is clearer. Sebastian
diff --git a/include/linux/string.h b/include/linux/string.h index ac889c5..f29f9a0 100644 --- a/include/linux/string.h +++ b/include/linux/string.h @@ -114,6 +114,7 @@ void *memchr_inv(const void *s, int c, size_t n); extern char *kstrdup(const char *s, gfp_t gfp); extern char *kstrndup(const char *s, size_t len, gfp_t gfp); +extern char *kstrimdup(const char *s, gfp_t gfp); extern void *kmemdup(const void *src, size_t len, gfp_t gfp); extern char **argv_split(gfp_t gfp, const char *str, int *argcp); diff --git a/mm/util.c b/mm/util.c index 808f375..2066b33 100644 --- a/mm/util.c +++ b/mm/util.c @@ -1,6 +1,7 @@ #include <linux/mm.h> #include <linux/slab.h> #include <linux/string.h> +#include <linux/ctype.h> #include <linux/export.h> #include <linux/err.h> #include <linux/sched.h> @@ -63,6 +64,35 @@ char *kstrndup(const char *s, size_t max, gfp_t gfp) EXPORT_SYMBOL(kstrndup); /** + * kstrimdup - Trim and copy a %NUL terminated string. + * @s: the string to trim and duplicate + * @gfp: the GFP mask used in the kmalloc() call when allocating memory + * + * Returns an address, which the caller must kfree, containing + * a duplicate of the passed string with leading and/or trailing + * whitespace (as defined by isspace) removed. + */ +char *kstrimdup(const char *s, gfp_t gfp) +{ + char *buf; + char *begin = skip_spaces(s); + size_t len = strlen(begin); + + while (len > 1 && isspace(begin[len - 1])) + len--; + + buf = kmalloc_track_caller(len + 1, gfp); + if (!buf) + return NULL; + + memcpy(buf, begin, len); + buf[len] = '\0'; + + return buf; +} +EXPORT_SYMBOL(kstrimdup); + +/** * kmemdup - duplicate region of memory * * @src: memory region to duplicate
kstrimdup will duplicate and trim spaces from the passed in null terminated string. This is useful for strings coming from sysfs that often include trailing whitespace due to user input. Signed-off-by: Sebastian Capella <sebastian.capella@linaro.org> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Joe Perches <joe@perches.com> Cc: Mikulas Patocka <mpatocka@redhat.com> Cc: Rik van Riel <riel@redhat.com> (commit_signer:5/10=50%) Cc: Michel Lespinasse <walken@google.com> Cc: Shaohua Li <shli@kernel.org> Cc: Jerome Marchand <jmarchan@redhat.com> Cc: Mikulas Patocka <mpatocka@redhat.com> Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com> --- include/linux/string.h | 1 + mm/util.c | 30 ++++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+)