[RFC][PATCH] lib/string: introduce sysfs_strncpy() and sysfs_strlcpy()

[RFC][PATCH] lib/string: introduce sysfs_strncpy() and sysfs_strlcpy()

[Sergey Senozhatsky]

In sysfs ->store() callbacks we usually need to remember that a
supplied sysfs input string might or might not contain whitespaces
and a trailing new-line symbol, which we need to take care of.
Examples:
	echo "newline" > /sys/.../attr
	echo -n "no_newine" > /sys/.../attr

That's why a typical sysfs ->store() should do something like
below when it copies a string:

	ssize_t FOO_store(struct device *dev, ....)
	{
		....
		strlcpy(value, buf, MAX_SZ);
		sz = strlen(value);
		if (sz > 0 && value[sz - 1] == '\n')
			value[sz - 1] = 0x00;
		...
	}

or use a sysfs-string friendly strcmp() function when comparing
a given string to a pre-defined one:

	ssize_t FOO_store(struct device *dev, ....)
	{
		...
		if (sysfs_streq(buf, "normal"))
			mode = m_normal;
		...
	}

Per Andrew Morton:
: There's a LOT of code which does basically-the-same-thing with sysfs
: input. And a lot of it misses things, such as leading whitespace.
: Passing all this through helpers would provide consistency as well
: as code-size reductions, improved reviewability, etc.

This patch introduces two such helpers - sysfs_strncpy() and
sysfs_strlcpy(), which, basically, do what strncpy() and strlcpy() do,
but additionally they remove leading and trailing white-spaces and
tailing new-line symbols. So a FOO_store() example which was posted
above may be rewritten to:

	ssize_t FOO_store(struct device *dev, ....)
	{
		....
		sysfs_strlcpy(value, buf, MAX_SZ);
		...
	}

Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Suggested-by: Andrew Morton <akpm@linux-foundation.org>
---
 include/linux/string.h |  3 +++
 lib/string.c           | 51 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+)

diff --git a/include/linux/string.h b/include/linux/string.h
index 4a5a0eb7df51..62b08bbc3ada 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -175,6 +175,9 @@ extern char **argv_split(gfp_t gfp, const char *str, int *argcp);
 extern void argv_free(char **argv);
 
 extern bool sysfs_streq(const char *s1, const char *s2);
+extern char *sysfs_strncpy(char *dest, const char *src, size_t count);
+extern size_t sysfs_strlcpy(char *dest, const char *src, size_t size);
+
 extern int kstrtobool(const char *s, bool *res);
 static inline int strtobool(const char *s, bool *res)
 {
diff --git a/lib/string.c b/lib/string.c
index 2c0900a5d51a..e81b1be00796 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -631,6 +631,57 @@ bool sysfs_streq(const char *s1, const char *s2)
 }
 EXPORT_SYMBOL(sysfs_streq);
 
+/**
+ * sysfs_strncpy - Trim a length-limited C-string (wgutesoaces and a trailing
+ *                 newline symbol) and copy into a buffer
+ * @dest: Where to copy the string to
+ * @src: Where to copy the string from
+ * @count: The maximum number of bytes to copy
+ *
+ * A wrapper around strncpy().
+ *
+ */
+char *sysfs_strncpy(char *dest, const char *src, size_t count)
+{
+	char *c;
+
+	strncpy(dest, skip_spaces(src), count);
+
+	c = dest + count - 1;
+	while (c >= dest && (isspace(*c) || *c == '\n' || *c == '\0')) {
+		*c = '\0';
+		c--;
+	}
+	return dest;
+}
+EXPORT_SYMBOL(sysfs_strncpy);
+
+/**
+ * sysfs_strlcpy - Trim a C-string (whitespaces and a trailing newline symbol)
+ *                 and copy it into a sized buffer
+ * @dest: Where to copy the string to
+ * @src: Where to copy the string from
+ * @size: size of destination buffer
+ *
+ * A wrapper around strlcpy().
+ *
+ */
+size_t sysfs_strlcpy(char *dest, const char *src, size_t size)
+{
+	size_t ret;
+	char *c;
+
+	ret = strlcpy(dest, skip_spaces(src), size);
+
+	size = strlen(dest);
+	c = dest + size - 1;
+	while (c >= dest && (isspace(*c) || *c == '\n'))
+		c--;
+	*(c + 1) = '\0';
+	return ret;
+}
+EXPORT_SYMBOL(sysfs_strlcpy);
+
 /**
  * match_string - matches given string in an array
  * @array:	array of strings
-- 
2.18.0

Re: [RFC][PATCH] lib/string: introduce sysfs_strncpy() and sysfs_strlcpy()

[Rasmus Villemoes]

On 2018-08-21 08:24, Sergey Senozhatsky wrote:
> +/**
> + * sysfs_strncpy - Trim a length-limited C-string (wgutesoaces and a trailing
> + *                 newline symbol) and copy into a buffer
> + * @dest: Where to copy the string to
> + * @src: Where to copy the string from
> + * @count: The maximum number of bytes to copy
> + *
> + * A wrapper around strncpy().
> + *
> + */
> +char *sysfs_strncpy(char *dest, const char *src, size_t count)
> +{
> +	char *c;
> +
> +	strncpy(dest, skip_spaces(src), count);

I'd like to see where and how you'd use this, but I'm very skeptical of
count being used both for the size of the dest buffer as well as an
essentially random argument to strncpy - if count is also the maximum
number of bytes to read from the src, you'd need to take the
skip_spaces() into account, because there are not count bytes left after
that... And if src is not necessarily nul-terminated, skip_spaces() by
itself is wrong.

Moreover, I don't think we should add more users or wrappers for strncpy
- I highly doubt the sysfs users you have in mind want the "fill the
rest of the buffer with '\0'" nor the "not enough room for a terminating
'\0'? Oh well, what could possibly go wrong" semantics.

> +	c = dest + count - 1;
> +	while (c >= dest && (isspace(*c) || *c == '\n' || *c == '\0')) {

nit: '\n' certainly already passes the isspace() test.

> +		*c = '\0';
> +		c--;
> +	}
> +	return dest;
> +}
> +EXPORT_SYMBOL(sysfs_strncpy);
> +
> +/**
> + * sysfs_strlcpy - Trim a C-string (whitespaces and a trailing newline symbol)
> + *                 and copy it into a sized buffer
> + * @dest: Where to copy the string to
> + * @src: Where to copy the string from
> + * @size: size of destination buffer
> + *
> + * A wrapper around strlcpy().
> + *
> + */
> +size_t sysfs_strlcpy(char *dest, const char *src, size_t size)
> +{
> +	size_t ret;
> +	char *c;
> +
> +	ret = strlcpy(dest, skip_spaces(src), size);
> +
> +	size = strlen(dest);
> +	c = dest + size - 1;
> +	while (c >= dest && (isspace(*c) || *c == '\n'))
> +		c--;
> +	*(c + 1) = '\0';
> +	return ret;
> +}

What exactly is the return value? The string length of the src after
skipping leading whitespace? What does that really have to do with the
string now in dest, and what is the user supposed to do with it? A more
useful return value would either be "the length of the string now in
dest", or some sort of indicator that the input was truncated, if that
is ever possible.

I think you're too focused on making wrappers around str[ln]cpy
preserving parts of those functions' API. Instead, try to figure out
what sysfs users actually want, name the functions after that, and then
whether they use strncpy or sprintf or strscpy internally is completely
irrelevant.

Maybe

int strcpy_trim(char *dst, size_t dstsize, const char *src, size_t
srcsize) - copy (potentially not '\0'-terminated) src to dst, trimming
leading and trailing whitespace. dstsize must be positive, and dst is
guaranteed to be '\0'-terminated. Returns the length of the string now
in dst, or -EOVERFLOW if some none-whitespace character was chopped.

would cover all use cases?

Rasmus

Re: [RFC][PATCH] lib/string: introduce sysfs_strncpy() and sysfs_strlcpy()

[Sergey Senozhatsky]

Hi Rasmus,

On (08/21/18 09:59), Rasmus Villemoes wrote:
> > +char *sysfs_strncpy(char *dest, const char *src, size_t count)
> > +{
> > +	char *c;
> > +
> > +	strncpy(dest, skip_spaces(src), count);
> 
> I'd like to see where and how you'd use this, but I'm very skeptical of
> count being used both for the size of the dest buffer as well as an
> essentially random argument to strncpy - if count is also the maximum
> number of bytes to read from the src, you'd need to take the
> skip_spaces() into account, because there are not count bytes left after
> that...
> And if src is not necessarily nul-terminated, skip_spaces() by
> itself is wrong.

I think that sysfs input is always properly NULL-terminated. It may or
may not contain \n, but \0 is expected to be there. Am I wrong?

> Moreover, I don't think we should add more users or wrappers for strncpy
> - I highly doubt the sysfs users you have in mind want the "fill the
> rest of the buffer with '\0'" nor the "not enough room for a terminating
> '\0'? Oh well, what could possibly go wrong" semantics.

The reason I added both strncpy() and strlcpy() was that there are lots
of sysfs ->store() callbacks which use strncpy().

E.g.
	channel_dimm_label_store()
	dimmdev_label_store()
	pmbus_add_label()
	axp20x_store_attr()
	cmdline_store()
	
	and so on and on.

> > +	c = dest + count - 1;
> > +	while (c >= dest && (isspace(*c) || *c == '\n' || *c == '\0')) {
> 
> nit: '\n' certainly already passes the isspace() test.

Hah, indeed, it should.
"\n" & 0x20 must be positive. Andrew had the same comment. But I didn't
check what actually isspace() was doing, until now.

> > +size_t sysfs_strlcpy(char *dest, const char *src, size_t size)
> > +{
> > +	size_t ret;
> > +	char *c;
> > +
> > +	ret = strlcpy(dest, skip_spaces(src), size);
> > +
> > +	size = strlen(dest);
> > +	c = dest + size - 1;
> > +	while (c >= dest && (isspace(*c) || *c == '\n'))
> > +		c--;
> > +	*(c + 1) = '\0';
> > +	return ret;
> > +}
> 
> What exactly is the return value?

Honestly, I didn't think about it. I wasn't sure if we want to return
anything from this function and from sysfs_strncpy(). I glanced through
a number of ->store() callbacks, and it seems that mostly people don't
bother to check strlcpy() return value at all.

> A more useful return value would either be "the length of the string
> now in dest", or some sort of indicator that the input was truncated,
> if that is ever possible.

Agreed.

> I think you're too focused on making wrappers around str[ln]cpy
> preserving parts of those functions' API. Instead, try to figure out
> what sysfs users actually want, name the functions after that, and then
> whether they use strncpy or sprintf or strscpy internally is completely
> irrelevant.

Going point, that's why the patch is in RFC stage: to figure out
what do we actually want.

> int strcpy_trim(char *dst, size_t dstsize, const char *src, size_t
> srcsize) - copy (potentially not '\0'-terminated) src to dst, trimming
> leading and trailing whitespace. dstsize must be positive, and dst is
> guaranteed to be '\0'-terminated. Returns the length of the string now
> in dst, or -EOVERFLOW if some none-whitespace character was chopped.
>
> would cover all use cases?

I like it in general. Sounds like a plan to me. Maybe the "-EOVERFLOW if
some none-whitespace character was chopped" part can be changed: if we
would trim leading and trailing whitespaces before we copy a string then
only valid input chars can get chopped.

	-ss

Re: [RFC][PATCH] lib/string: introduce sysfs_strncpy() and sysfs_strlcpy()

[Sergey Senozhatsky]

On (08/21/18 18:50), Sergey Senozhatsky wrote:
> 
> Going point, that's why the patch is in RFC stage: to figure out
> what do we actually want.

s/Going/Good/       don't know how did that happen.

	-ss

Re: [RFC][PATCH] lib/string: introduce sysfs_strncpy() and sysfs_strlcpy()

[Sergey Senozhatsky]

On (08/21/18 18:50), Sergey Senozhatsky wrote:
> 
> > int strcpy_trim(char *dst, size_t dstsize, const char *src, size_t
> > srcsize) - copy (potentially not '\0'-terminated) src to dst, trimming
> > leading and trailing whitespace. dstsize must be positive, and dst is
> > guaranteed to be '\0'-terminated. Returns the length of the string now
> > in dst, or -EOVERFLOW if some none-whitespace character was chopped.
> >
> > would cover all use cases?
> 

Something like below? Not tested, since we are still in "is this
what we want" phase.
Returning the length of dst/-EOVERFLOW is a bit inconvenient, because
"the length" forces us to have size_t return, which is unsigned.

This one returns a number of non-NULL bytes, which were copied
to dst, or 0.

---

size_t strcpy_trim(char *dst, size_t dstsz, const char *src, size_t srcsz)
{
	const char *end;
	size_t ret = 0;
	size_t len = 0;

	if (!dstsz || !srcsz)
		goto out;

	end = src + srcsz - 1;
	while (end >= src && isspace(*end))
		end--;
	end++;
	while (src < end && isspace(*src))
		src++;
	len = (src >= end) ? 0 : end - src;
	if (!len)
		goto out;

	ret = len;
	if (len >= dstsz)
		len = dstsz - 1;
	memcpy(dst, src, len);
out:
	dst[len] = '\0';
	return ret == len ? ret : -EOVERFLOW;
}

Re: [RFC][PATCH] lib/string: introduce sysfs_strncpy() and sysfs_strlcpy()

[Andy Shevchenko]

On Tue, 2018-08-21 at 20:44 +0900, Sergey Senozhatsky wrote:

> Something like below? Not tested, since we are still in "is this
> what we want" phase.

> Returning the length of dst/-EOVERFLOW is a bit inconvenient, because
> "the length" forces us to have size_t return, which is unsigned.

We have for ages ssize_t to workaround that.

> size_t strcpy_trim(char *dst, size_t dstsz, const char *src, size_t
> srcsz)
> {
> 	const char *end;
> 	size_t ret = 0;
> 	size_t len = 0;
> 
> 	if (!dstsz || !srcsz)
> 		goto out;
> 
> 	end = src + srcsz - 1;
> 	while (end >= src && isspace(*end))
> 		end--;
> 	end++;
> 	while (src < end && isspace(*src))
> 		src++;
> 	len = (src >= end) ? 0 : end - src;
> 	if (!len)
> 		goto out;
> 
> 	ret = len;
> 	if (len >= dstsz)
> 		len = dstsz - 1;
> 	memcpy(dst, src, len);
> out:
> 	dst[len] = '\0';
> 	return ret == len ? ret : -EOVERFLOW;
> }

Wouldn't be better to split out something like

strnstrip() out of strim(), with simultaneous changes to strim(),
strstrip(), and use it here?


-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [RFC][PATCH] lib/string: introduce sysfs_strncpy() and sysfs_strlcpy()

[Rasmus Villemoes]

On 2018-08-21 11:50, Sergey Senozhatsky wrote:
> Hi Rasmus,
> 
> On (08/21/18 09:59), Rasmus Villemoes wrote:
>>> +char *sysfs_strncpy(char *dest, const char *src, size_t count)
>>> +{
>>> +	char *c;
>>> +
>>> +	strncpy(dest, skip_spaces(src), count);
>>
>> I'd like to see where and how you'd use this, but I'm very skeptical of
>> count being used both for the size of the dest buffer as well as an
>> essentially random argument to strncpy - if count is also the maximum
>> number of bytes to read from the src, you'd need to take the
>> skip_spaces() into account, because there are not count bytes left after
>> that...
>> And if src is not necessarily nul-terminated, skip_spaces() by
>> itself is wrong.
> 
> I think that sysfs input is always properly NULL-terminated. It may or
> may not contain \n, but \0 is expected to be there.

Maybe. But it shouldn't hurt to make it accept a src size (the caller
can always pass -1 is he's sure the input is nul-terminated) - I'm
thinking a new interface might also have uses outside sysfs.

>> Moreover, I don't think we should add more users or wrappers for strncpy
>> - I highly doubt the sysfs users you have in mind want the "fill the
>> rest of the buffer with '\0'" nor the "not enough room for a terminating
>> '\0'? Oh well, what could possibly go wrong" semantics.
> 
> The reason I added both strncpy() and strlcpy() was that there are lots
> of sysfs ->store() callbacks which use strncpy().
> 
> E.g.
> 	channel_dimm_label_store()
> 	dimmdev_label_store()
> 	pmbus_add_label()
> 	axp20x_store_attr()
> 	cmdline_store()

Hm, do any of those actually want the skipping of leading whitespace?
pmbus_add_label doesn't even seem to kill a trailing newline?

It's probably hard to create one interface that will work for all cases.
Some are happy with truncating the user input, some refuse to do the
copy if truncation might happen, some probably do the copy (clobbering
what was there) but then detect the truncation somehow and return EINVAL
or whatever. Also, some copy to a pre-existing buffer, while others
kmalloc() a buffer and populate it with strncpy. The latter would
probably be better served using kstrndup or a small variant.

>> I think you're too focused on making wrappers around str[ln]cpy
>> preserving parts of those functions' API. Instead, try to figure out
>> what sysfs users actually want, name the functions after that, and then
>> whether they use strncpy or sprintf or strscpy internally is completely
>> irrelevant.
> 
> [Good] point, that's why the patch is in RFC stage: to figure out
> what do we actually want.

Dunno, looking at your examples above I'm a bit more skeptical that
there is a lot of common ground. But of course there are hundreds of
other store callbacks, so surely one can simplify some with a new helper.

Rasmus

Re: [RFC][PATCH] lib/string: introduce sysfs_strncpy() and sysfs_strlcpy()

[Greg Kroah-Hartman]

On Tue, Aug 21, 2018 at 06:50:55PM +0900, Sergey Senozhatsky wrote:
> Hi Rasmus,
> 
> On (08/21/18 09:59), Rasmus Villemoes wrote:
> > > +char *sysfs_strncpy(char *dest, const char *src, size_t count)
> > > +{
> > > +	char *c;
> > > +
> > > +	strncpy(dest, skip_spaces(src), count);
> > 
> > I'd like to see where and how you'd use this, but I'm very skeptical of
> > count being used both for the size of the dest buffer as well as an
> > essentially random argument to strncpy - if count is also the maximum
> > number of bytes to read from the src, you'd need to take the
> > skip_spaces() into account, because there are not count bytes left after
> > that...
> > And if src is not necessarily nul-terminated, skip_spaces() by
> > itself is wrong.
> 
> I think that sysfs input is always properly NULL-terminated. It may or
> may not contain \n, but \0 is expected to be there. Am I wrong?

sysfs data is always null terminated.

What exactly are you trying to do here?  If a user sends you crappy data
in a sysfs file (like leading or trailing whitespace), well, you can
always just error out, no problem.  You can be very strict in the
SINGLE_VALUE that you accept in your sysfs file.  And yes, sysfs files
are single values, if you want to do more than that, you need to use
something else, so I really do not understand what problem you are
trying to solve here.

Please always post a user of your new api when you make stuff like this
otherwise we do not know how it is used, or even why you are adding it.

thanks,

greg k-h

Re: [RFC][PATCH] lib/string: introduce sysfs_strncpy() and sysfs_strlcpy()

[Sergey Senozhatsky]

On (08/21/18 15:00), Andy Shevchenko wrote:
> > Returning the length of dst/-EOVERFLOW is a bit inconvenient, because
> > "the length" forces us to have size_t return, which is unsigned.
> 
> We have for ages ssize_t to workaround that.

OK.

[..]
> Wouldn't be better to split out something like
> 
> strnstrip() out of strim(), with simultaneous changes to strim(),
> strstrip(), and use it here?

Maybe yes, maybe not. strim() modifies the original string right after
it's done moving the end pointer. We can't do that in strcpy_trim() and
need to keep the original source string. So probably these two functions
don't have that much of a common code after all.

	-ss

Re: [RFC][PATCH] lib/string: introduce sysfs_strncpy() and sysfs_strlcpy()

[Sergey Senozhatsky]

Hello Greg,

On (08/21/18 15:57), Greg Kroah-Hartman wrote:
> > I think that sysfs input is always properly NULL-terminated. It may or
> > may not contain \n, but \0 is expected to be there. Am I wrong?
> 
> sysfs data is always null terminated.
> 
> What exactly are you trying to do here?  If a user sends you crappy data
> in a sysfs file (like leading or trailing whitespace), well, you can
> always just error out, no problem.

So what we are thinking about is sort of clean up / unification of the
way ->store() callbacks handle sysfs input. They all have to do the
same things for "corner case" handling and many of them forget to handle
some specific cases; or have to come up with extra code; or are not aware
of the existing API; etc.

For instance, let's look at 325c4b3b81027068 [well, a small part of]

---

@@ -245,7 +239,7 @@ static ssize_t pm_qos_resume_latency_store(struct device *dev,
 
                if (value == 0)
                        value = PM_QOS_RESUME_LATENCY_NO_CONSTRAINT;
-       } else if (!strcmp(buf, "n/a") || !strcmp(buf, "n/a\n")) {
+       } else if (sysfs_streq(buf, "n/a")) {
                value = 0;
        } else {
                return -EINVAL;
@@ -285,9 +279,9 @@ static ssize_t pm_qos_latency_tolerance_store(struct device *dev,
                if (value < 0)
                        return -EINVAL;
        } else {
-               if (!strcmp(buf, "auto") || !strcmp(buf, "auto\n"))
+               if (sysfs_streq(buf, "auto"))
                        value = PM_QOS_LATENCY_TOLERANCE_NO_CONSTRAINT;
-               else if (!strcmp(buf, "any") || !strcmp(buf, "any\n"))
+               else if (sysfs_streq(buf, "any"))
                        value = PM_QOS_LATENCY_ANY;
                else
                        return -EINVAL;
@@ -342,20 +336,12 @@ static ssize_t
 wake_store(struct device * dev, struct device_attribute *attr,
        const char * buf, size_t n)
 {
-       char *cp;
-       int len = n;
-
        if (!device_can_wakeup(dev))
                return -EINVAL;
 
-       cp = memchr(buf, '\n', n);
-       if (cp)
-               len = cp - buf;
-       if (len == sizeof _enabled - 1
-                       && strncmp(buf, _enabled, sizeof _enabled - 1) == 0)
+       if (sysfs_streq(buf, _enabled))
                device_set_wakeup_enable(dev, 1);
-       else if (len == sizeof _disabled - 1
-                       && strncmp(buf, _disabled, sizeof _disabled - 1) == 0)
+       else if (sysfs_streq(buf, _disabled))
                device_set_wakeup_enable(dev, 0);
        else
                return -EINVAL;
---

There was quite a bit of code. But these things still can and do happen;
Andy tweaked the existing code only.

So what we are looking at is a way which would let us to make that part
of drivers to be simpler and less fragile, perhaps.

> Please always post a user of your new api when you make stuff like this
> otherwise we do not know how it is used, or even why you are adding it.

Sure, I agree. There is no API proposal yet; so I gave a simple example
in the commit message and didn't bother to convert any of the existing
users.

I'm not even sure yet if we want to have a new API. The sort of a
root cause [it seems so] here is that sysfs input data has irregular
format. That's why we have irregular handling of

Either in a form of

		if (!strcmp(buf, "auto") || !strcmp(buf, "auto\n"))
			...
or in a form of

		if (sz > 0 && value[sz - 1] == '\n')
			value[sz - 1] = 0x00;
		if (!strcmp(value, "auto"))

and so on.

So may be, instead of new API which was meant to help make sysfs data
look uniform, we can do tweaks to sysfs and pass to ->store() callbacks
data which already has no trailing newline and whitespaces. IOW, make it
uniform within sysfs. Then we can remove a bunch of code from the existing
drivers and make it easier for future drivers.
So sysfs could do strim-ing, etc. and ->store() would always receive data
which can be directly used as strcmp/strcpy/etc input. Because this is what
people want to do after all; but they learn at some point that they can't
and there are newline symbols, etc. to take care of.

What do you think? A new API is probably safer option here; but then,
again, people can forget to use it, or be unaware of it.

	-ss

Re: [RFC][PATCH] lib/string: introduce sysfs_strncpy() and sysfs_strlcpy()

[Sergey Senozhatsky]

Hi,

On (08/21/18 14:43), Rasmus Villemoes wrote:
> > I think that sysfs input is always properly NULL-terminated. It may or
> > may not contain \n, but \0 is expected to be there.
> 
> Maybe. But it shouldn't hurt to make it accept a src size (the caller
> can always pass -1 is he's sure the input is nul-terminated) - I'm
> thinking a new interface might also have uses outside sysfs.

Makes sense.

> > E.g.
> > 	channel_dimm_label_store()
> > 	dimmdev_label_store()
> > 	pmbus_add_label()
> > 	axp20x_store_attr()
> > 	cmdline_store()
> 
> Hm, do any of those actually want the skipping of leading whitespace?

Hm, that's a good question. I'd probably say "mostly yes". The input
usually goes to strcmp/strcpy/strtol/etc. But we don't have to trim
leading whitespaces and can just let the driver error out [hopefully
the driver has a well tested error-out path].

> pmbus_add_label doesn't even seem to kill a trailing newline?

Sorry, I think pmbus_add_label() made it to the list by mistake. It doesn't
appear to be a ->store() callback.

> It's probably hard to create one interface that will work for all cases.

Agreed, that's why I targeted sysfs input data only ;)

> Dunno, looking at your examples above I'm a bit more skeptical that
> there is a lot of common ground. But of course there are hundreds of
> other store callbacks, so surely one can simplify some with a new helper.

Yep.

	-ss