diff mbox

[v5,1/2] mm: add kstrimdup function

Message ID 1391116318-17253-2-git-send-email-sebastian.capella@linaro.org (mailing list archive)
State Superseded, archived
Headers show

Commit Message

Sebastian Capella Jan. 30, 2014, 9:11 p.m. UTC
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(+)

Comments

Andrew Morton Jan. 30, 2014, 9:22 p.m. UTC | #1
On Thu, 30 Jan 2014 13:11:57 -0800 Sebastian Capella <sebastian.capella@linaro.org> wrote:

> 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.
>
> ...
> 
> --- a/include/linux/string.h
> +++ b/include/linux/string.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--;

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;
> +}
> +EXPORT_SYMBOL(kstrimdup);
> +

--
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
David Rientjes Jan. 30, 2014, 9:44 p.m. UTC | #2
On Thu, 30 Jan 2014, Andrew Morton wrote:

> > @@ -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--;
> 
> That's off-by-one isn't it?  kstrimdup("   ") should return "", not " ".
> 

Yeah, this is an incorrect copy-and-paste of Joe Perches' suggested code 
from http://marc.info/?l=linux-kernel&m=139104508317989.
--
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
Andrew Morton Jan. 30, 2014, 9:51 p.m. UTC | #3
On Thu, 30 Jan 2014 13:45:45 -0800 Sebastian Capella <sebastian.capella@linaro.org> wrote:

> 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.

heh, OK, tricky.
--
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 mbox

Patch

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