linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Sebastian Capella <sebastian.capella@linaro.org>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	linux-pm@vger.kernel.org, linaro-kernel@lists.linaro.org,
	patches@linaro.org, Michel Lespinasse <walken@google.com>,
	Shaohua Li <shli@kernel.org>,
	Jerome Marchand <jmarchan@redhat.com>,
	Mikulas Patocka <mpatocka@redhat.com>,
	Joonsoo Kim <iamjoonsoo.kim@lge.com>,
	Joe Perches <joe@perches.com>,
	David Rientjes <rientjes@google.com>,
	Alexey Dobriyan <adobriyan@gmail.com>,
	Pavel Machek <pavel@ucw.cz>
Subject: Re: [PATCH v7 1/3] mm: add kstrdup_trimnl function
Date: Wed, 05 Feb 2014 14:55:52 -0800	[thread overview]
Message-ID: <20140205225552.16730.1677@capellas-linux> (raw)
In-Reply-To: <20140205135052.4066b67689cbf47c551d30a9@linux-foundation.org>

Quoting Andrew Morton (2014-02-05 13:50:52)
> On Tue,  4 Feb 2014 12:43:49 -0800 Sebastian Capella <sebastian.capella@linaro.org> wrote:
> 
> > kstrdup_trimnl creates a duplicate of the passed in
> > null-terminated string.  If a trailing newline is found, it
> > is removed before duplicating.  This is useful for strings
> > coming from sysfs that often include trailing whitespace due to
> > user input.
> 
> hm, why?  I doubt if any caller of this wants to retain leading and/or
> trailing spaces and/or tabs.

Hi Andrew,

I agree the common case doesn't usually need leading or trailing whitespace.

Pavel and others pointed out that a valid filename could contain
newlines/whitespace at any position.

If we allow for this, then it would be incorrect to strip whitespace
from the input.  Comments also went down the lines that it would be
better for the kernel not to second guess what is being passed in.

I find stripping the trailing newline to be very useful, and there are
many examples of kernel code doing this.  I think it would be a mistake
to remove this now, and would be confusing for users.  A compromise
is to strip the final newline only if it's present before the null.

This allows the common case of echoing a simple string onto
/sys/power/resume, and behaves as expected in that case.

A complex string without a trailing newline is also handled by quoting or
dding a file onto /sys/power/resume.

In the unlikely event a user has trailing newline as part of the input, then
adding an additional newline to the end will cover that case.  This
is not ideal, but it puts the additional burden onto the complex case
rather than the common case.

Thanks,

Sebastian

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2014-02-05 22:55 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-02-04 20:43 [PATCH v7 0/3] hibernation related patches Sebastian Capella
2014-02-04 20:43 ` [PATCH v7 1/3] mm: add kstrdup_trimnl function Sebastian Capella
2014-02-05 21:50   ` Andrew Morton
2014-02-05 22:55     ` Sebastian Capella [this message]
2014-02-05 23:01       ` Andrew Morton
2014-02-06 23:48         ` Sebastian Capella
2014-02-04 20:43 ` [PATCH v7 2/3] trivial: PM / Hibernate: clean up checkpatch in hibernate.c Sebastian Capella
2014-02-04 21:21   ` Joe Perches
2014-02-04 22:05     ` Sebastian Capella
2014-02-04 23:45       ` Joe Perches
2014-02-04 21:36   ` Rafael J. Wysocki
2014-02-04 22:37     ` Sebastian Capella
2014-02-04 23:22       ` Sebastian Capella
2014-02-05  0:03         ` Rafael J. Wysocki
2014-02-05  0:06           ` Sebastian Capella
2014-02-05  0:28             ` Rafael J. Wysocki
2014-02-05  0:24               ` Sebastian Capella
2014-02-05 11:07                 ` Rafael J. Wysocki
2014-02-04 23:59       ` Rafael J. Wysocki
2014-02-04 20:43 ` [PATCH v7 3/3] PM / Hibernate: use name_to_dev_t to parse resume Sebastian Capella
2014-02-04 21:39   ` Rafael J. Wysocki
2014-02-04 23:17     ` Sebastian Capella

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140205225552.16730.1677@capellas-linux \
    --to=sebastian.capella@linaro.org \
    --cc=adobriyan@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=iamjoonsoo.kim@lge.com \
    --cc=jmarchan@redhat.com \
    --cc=joe@perches.com \
    --cc=linaro-kernel@lists.linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=mpatocka@redhat.com \
    --cc=patches@linaro.org \
    --cc=pavel@ucw.cz \
    --cc=rientjes@google.com \
    --cc=shli@kernel.org \
    --cc=walken@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).