From: Justin He <Justin.He@arm.com>
To: Al Viro <viro@zeniv.linux.org.uk>,
Linus Torvalds <torvalds@linux-foundation.org>
Cc: Petr Mladek <pmladek@suse.com>,
Steven Rostedt <rostedt@goodmis.org>,
Sergey Senozhatsky <senozhatsky@chromium.org>,
Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
Rasmus Villemoes <linux@rasmusvillemoes.dk>,
Jonathan Corbet <corbet@lwn.net>,
Heiko Carstens <hca@linux.ibm.com>,
Vasily Gorbik <gor@linux.ibm.com>,
Christian Borntraeger <borntraeger@de.ibm.com>,
"Eric W . Biederman" <ebiederm@xmission.com>,
"Darrick J. Wong" <darrick.wong@oracle.com>,
"Peter Zijlstra (Intel)" <peterz@infradead.org>,
Ira Weiny <ira.weiny@intel.com>,
Eric Biggers <ebiggers@google.com>,
"Ahmed S. Darwish" <a.darwish@linutronix.de>,
"open list:DOCUMENTATION" <linux-doc@vger.kernel.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
linux-s390 <linux-s390@vger.kernel.org>,
linux-fsdevel <linux-fsdevel@vger.kernel.org>
Subject: RE: [PATCH 09/14] d_path: introduce struct prepend_buffer
Date: Wed, 23 Jun 2021 13:28:44 +0000 [thread overview]
Message-ID: <AM6PR08MB4376D92F354CD17445DC4EC1F7089@AM6PR08MB4376.eurprd08.prod.outlook.com> (raw)
In-Reply-To: <20210519004901.3829541-9-viro@zeniv.linux.org.uk>
Hi Al
> -----Original Message-----
> From: Al Viro <viro@ftp.linux.org.uk> On Behalf Of Al Viro
> Sent: Wednesday, May 19, 2021 8:49 AM
> To: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Justin He <Justin.He@arm.com>; Petr Mladek <pmladek@suse.com>; Steven
> Rostedt <rostedt@goodmis.org>; Sergey Senozhatsky
> <senozhatsky@chromium.org>; Andy Shevchenko
> <andriy.shevchenko@linux.intel.com>; Rasmus Villemoes
> <linux@rasmusvillemoes.dk>; Jonathan Corbet <corbet@lwn.net>; Heiko
> Carstens <hca@linux.ibm.com>; Vasily Gorbik <gor@linux.ibm.com>; Christian
> Borntraeger <borntraeger@de.ibm.com>; Eric W . Biederman
> <ebiederm@xmission.com>; Darrick J. Wong <darrick.wong@oracle.com>; Peter
> Zijlstra (Intel) <peterz@infradead.org>; Ira Weiny <ira.weiny@intel.com>;
> Eric Biggers <ebiggers@google.com>; Ahmed S. Darwish
> <a.darwish@linutronix.de>; open list:DOCUMENTATION <linux-
> doc@vger.kernel.org>; Linux Kernel Mailing List <linux-
> kernel@vger.kernel.org>; linux-s390 <linux-s390@vger.kernel.org>; linux-
> fsdevel <linux-fsdevel@vger.kernel.org>
> Subject: [PATCH 09/14] d_path: introduce struct prepend_buffer
>
> We've a lot of places where we have pairs of form (pointer to end
> of buffer, amount of space left in front of that). These sit in pairs of
> variables located next to each other and usually passed by reference.
> Turn those into instances of new type (struct prepend_buffer) and pass
> reference to the pair instead of pairs of references to its fields.
>
> Declared and initialized by DECLARE_BUFFER(name, buf, buflen).
>
> extract_string(prepend_buffer) returns the buffer contents if
> no overflow has happened, ERR_PTR(ENAMETOOLONG) otherwise.
> All places where we used to have that boilerplate converted to use
> of that helper.
>
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
> fs/d_path.c | 142 ++++++++++++++++++++++++++++++++-----------------------
> -----
> 1 file changed, 75 insertions(+), 67 deletions(-)
>
> diff --git a/fs/d_path.c b/fs/d_path.c
> index 83db83446afd..06e93dd031bf 100644
> --- a/fs/d_path.c
> +++ b/fs/d_path.c
> @@ -8,12 +8,26 @@
> #include <linux/prefetch.h>
> #include "mount.h"
>
> -static void prepend(char **buffer, int *buflen, const char *str, int
> namelen)
> +struct prepend_buffer {
> + char *buf;
> + int len;
> +};
> +#define DECLARE_BUFFER(__name, __buf, __len) \
> + struct prepend_buffer __name = {.buf = __buf + __len, .len = __len}
> +
> +static char *extract_string(struct prepend_buffer *p)
> {
> - *buflen -= namelen;
> - if (likely(*buflen >= 0)) {
> - *buffer -= namelen;
> - memcpy(*buffer, str, namelen);
> + if (likely(p->len >= 0))
> + return p->buf;
> + return ERR_PTR(-ENAMETOOLONG);
> +}
> +
> +static void prepend(struct prepend_buffer *p, const char *str, int
> namelen)
> +{
> + p->len -= namelen;
> + if (likely(p->len >= 0)) {
> + p->buf -= namelen;
> + memcpy(p->buf, str, namelen);
> }
> }
>
> @@ -34,22 +48,22 @@ static void prepend(char **buffer, int *buflen, const
> char *str, int namelen)
> *
> * Load acquire is needed to make sure that we see that terminating NUL.
> */
> -static bool prepend_name(char **buffer, int *buflen, const struct qstr
> *name)
> +static bool prepend_name(struct prepend_buffer *p, const struct qstr
> *name)
Please also change the parameter description in the comments of
prepend_name(), otherwise "make C=1 W=1" will report warnings.
--
Cheers,
Justin (Jia He)
> {
> const char *dname = smp_load_acquire(&name->name); /* ^^^ */
> u32 dlen = READ_ONCE(name->len);
> - char *p;
> + char *s;
>
> - *buflen -= dlen + 1;
> - if (unlikely(*buflen < 0))
> + p->len -= dlen + 1;
> + if (unlikely(p->len < 0))
> return false;
> - p = *buffer -= dlen + 1;
> - *p++ = '/';
> + s = p->buf -= dlen + 1;
> + *s++ = '/';
> while (dlen--) {
> char c = *dname++;
> if (!c)
> break;
> - *p++ = c;
> + *s++ = c;
> }
> return true;
> }
> @@ -73,15 +87,14 @@ static bool prepend_name(char **buffer, int *buflen,
> const struct qstr *name)
> */
> static int prepend_path(const struct path *path,
> const struct path *root,
> - char **buffer, int *buflen)
> + struct prepend_buffer *p)
> {
> struct dentry *dentry;
> struct vfsmount *vfsmnt;
> struct mount *mnt;
> int error = 0;
> unsigned seq, m_seq = 0;
> - char *bptr;
> - int blen;
> + struct prepend_buffer b;
>
> rcu_read_lock();
> restart_mnt:
> @@ -89,8 +102,7 @@ static int prepend_path(const struct path *path,
> seq = 0;
> rcu_read_lock();
> restart:
> - bptr = *buffer;
> - blen = *buflen;
> + b = *p;
> error = 0;
> dentry = path->dentry;
> vfsmnt = path->mnt;
> @@ -105,8 +117,7 @@ static int prepend_path(const struct path *path,
>
> /* Escaped? */
> if (dentry != vfsmnt->mnt_root) {
> - bptr = *buffer;
> - blen = *buflen;
> + b = *p;
> error = 3;
> break;
> }
> @@ -127,7 +138,7 @@ static int prepend_path(const struct path *path,
> }
> parent = dentry->d_parent;
> prefetch(parent);
> - if (!prepend_name(&bptr, &blen, &dentry->d_name))
> + if (!prepend_name(&b, &dentry->d_name))
> break;
>
> dentry = parent;
> @@ -148,11 +159,10 @@ static int prepend_path(const struct path *path,
> }
> done_seqretry(&mount_lock, m_seq);
>
> - if (blen == *buflen)
> - prepend(&bptr, &blen, "/", 1);
> + if (b.len == p->len)
> + prepend(&b, "/", 1);
>
> - *buffer = bptr;
> - *buflen = blen;
> + *p = b;
> return error;
> }
>
> @@ -176,24 +186,24 @@ char *__d_path(const struct path *path,
> const struct path *root,
> char *buf, int buflen)
> {
> - char *res = buf + buflen;
> + DECLARE_BUFFER(b, buf, buflen);
>
> - prepend(&res, &buflen, "", 1);
> - if (prepend_path(path, root, &res, &buflen) > 0)
> + prepend(&b, "", 1);
> + if (prepend_path(path, root, &b) > 0)
> return NULL;
> - return buflen >= 0 ? res : ERR_PTR(-ENAMETOOLONG);
> + return extract_string(&b);
> }
>
> char *d_absolute_path(const struct path *path,
> char *buf, int buflen)
> {
> struct path root = {};
> - char *res = buf + buflen;
> + DECLARE_BUFFER(b, buf, buflen);
>
> - prepend(&res, &buflen, "", 1);
> - if (prepend_path(path, &root, &res, &buflen) > 1)
> + prepend(&b, "", 1);
> + if (prepend_path(path, &root, &b) > 1)
> return ERR_PTR(-EINVAL);
> - return buflen >= 0 ? res : ERR_PTR(-ENAMETOOLONG);
> + return extract_string(&b);
> }
>
> static void get_fs_root_rcu(struct fs_struct *fs, struct path *root)
> @@ -224,7 +234,7 @@ static void get_fs_root_rcu(struct fs_struct *fs,
> struct path *root)
> */
> char *d_path(const struct path *path, char *buf, int buflen)
> {
> - char *res = buf + buflen;
> + DECLARE_BUFFER(b, buf, buflen);
> struct path root;
>
> /*
> @@ -245,13 +255,13 @@ char *d_path(const struct path *path, char *buf, int
> buflen)
> rcu_read_lock();
> get_fs_root_rcu(current->fs, &root);
> if (unlikely(d_unlinked(path->dentry)))
> - prepend(&res, &buflen, " (deleted)", 11);
> + prepend(&b, " (deleted)", 11);
> else
> - prepend(&res, &buflen, "", 1);
> - prepend_path(path, &root, &res, &buflen);
> + prepend(&b, "", 1);
> + prepend_path(path, &root, &b);
> rcu_read_unlock();
>
> - return buflen >= 0 ? res : ERR_PTR(-ENAMETOOLONG);
> + return extract_string(&b);
> }
> EXPORT_SYMBOL(d_path);
>
> @@ -278,36 +288,34 @@ char *dynamic_dname(struct dentry *dentry, char
> *buffer, int buflen,
>
> char *simple_dname(struct dentry *dentry, char *buffer, int buflen)
> {
> - char *end = buffer + buflen;
> + DECLARE_BUFFER(b, buffer, buflen);
> /* these dentries are never renamed, so d_lock is not needed */
> - prepend(&end, &buflen, " (deleted)", 11);
> - prepend(&end, &buflen, dentry->d_name.name, dentry->d_name.len);
> - prepend(&end, &buflen, "/", 1);
> - return buflen >= 0 ? end : ERR_PTR(-ENAMETOOLONG);
> + prepend(&b, " (deleted)", 11);
> + prepend(&b, dentry->d_name.name, dentry->d_name.len);
> + prepend(&b, "/", 1);
> + return extract_string(&b);
> }
>
> /*
> * Write full pathname from the root of the filesystem into the buffer.
> */
> -static char *__dentry_path(const struct dentry *d, char *p, int buflen)
> +static char *__dentry_path(const struct dentry *d, struct prepend_buffer
> *p)
> {
> const struct dentry *dentry;
> - char *end;
> - int len, seq = 0;
> + struct prepend_buffer b;
> + int seq = 0;
>
> rcu_read_lock();
> restart:
> dentry = d;
> - end = p;
> - len = buflen;
> + b = *p;
> read_seqbegin_or_lock(&rename_lock, &seq);
> while (!IS_ROOT(dentry)) {
> const struct dentry *parent = dentry->d_parent;
>
> prefetch(parent);
> - if (!prepend_name(&end, &len, &dentry->d_name))
> + if (!prepend_name(&b, &dentry->d_name))
> break;
> -
> dentry = parent;
> }
> if (!(seq & 1))
> @@ -317,28 +325,29 @@ static char *__dentry_path(const struct dentry *d,
> char *p, int buflen)
> goto restart;
> }
> done_seqretry(&rename_lock, seq);
> - if (len == buflen)
> - prepend(&end, &len, "/", 1);
> - return len >= 0 ? end : ERR_PTR(-ENAMETOOLONG);
> + if (b.len == p->len)
> + prepend(&b, "/", 1);
> + return extract_string(&b);
> }
>
> char *dentry_path_raw(const struct dentry *dentry, char *buf, int buflen)
> {
> - char *p = buf + buflen;
> - prepend(&p, &buflen, "", 1);
> - return __dentry_path(dentry, p, buflen);
> + DECLARE_BUFFER(b, buf, buflen);
> +
> + prepend(&b, "", 1);
> + return __dentry_path(dentry, &b);
> }
> EXPORT_SYMBOL(dentry_path_raw);
>
> char *dentry_path(const struct dentry *dentry, char *buf, int buflen)
> {
> - char *p = buf + buflen;
> + DECLARE_BUFFER(b, buf, buflen);
>
> if (unlikely(d_unlinked(dentry)))
> - prepend(&p, &buflen, "//deleted", 10);
> + prepend(&b, "//deleted", 10);
> else
> - prepend(&p, &buflen, "", 1);
> - return __dentry_path(dentry, p, buflen);
> + prepend(&b, "", 1);
> + return __dentry_path(dentry, &b);
> }
>
> static void get_fs_root_and_pwd_rcu(struct fs_struct *fs, struct path
> *root,
> @@ -386,24 +395,23 @@ SYSCALL_DEFINE2(getcwd, char __user *, buf, unsigned
> long, size)
> error = -ENOENT;
> if (!d_unlinked(pwd.dentry)) {
> unsigned long len;
> - char *cwd = page + PATH_MAX;
> - int buflen = PATH_MAX;
> + DECLARE_BUFFER(b, page, PATH_MAX);
>
> - prepend(&cwd, &buflen, "", 1);
> - if (prepend_path(&pwd, &root, &cwd, &buflen) > 0)
> - prepend(&cwd, &buflen, "(unreachable)", 13);
> + prepend(&b, "", 1);
> + if (prepend_path(&pwd, &root, &b) > 0)
> + prepend(&b, "(unreachable)", 13);
> rcu_read_unlock();
>
> - if (buflen < 0) {
> + if (b.len < 0) {
> error = -ENAMETOOLONG;
> goto out;
> }
>
> error = -ERANGE;
> - len = PATH_MAX + page - cwd;
> + len = PATH_MAX - b.len;
> if (len <= size) {
> error = len;
> - if (copy_to_user(buf, cwd, len))
> + if (copy_to_user(buf, b.buf, len))
> error = -EFAULT;
> }
> } else {
> --
> 2.11.0
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
next prev parent reply other threads:[~2021-06-23 13:29 UTC|newest]
Thread overview: 75+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-08 12:25 [PATCH RFC 0/3] make '%pD' print full path for file Jia He
2021-05-08 12:25 ` [PATCH RFC 1/3] fs: introduce helper d_path_fast() Jia He
2021-05-08 15:30 ` Linus Torvalds
2021-05-08 19:13 ` Al Viro
2021-05-08 20:39 ` Linus Torvalds
2021-05-08 21:05 ` Al Viro
2021-05-08 22:17 ` Linus Torvalds
2021-05-08 22:46 ` Al Viro
2021-05-08 22:48 ` Linus Torvalds
2021-05-08 23:15 ` Al Viro
2021-05-08 23:18 ` Al Viro
2021-05-09 22:58 ` Eric W. Biederman
2021-05-10 12:51 ` Christian Brauner
2021-05-10 7:20 ` Christian Brauner
2021-05-08 22:42 ` Linus Torvalds
2021-05-08 22:47 ` Linus Torvalds
2021-05-09 2:28 ` Al Viro
2021-05-09 2:53 ` Linus Torvalds
2021-05-19 0:43 ` [PATCHSET] d_path cleanups Al Viro
2021-05-19 0:48 ` [PATCH 01/14] d_path: "\0" is {0,0}, not {0} Al Viro
2021-05-19 0:48 ` [PATCH 02/14] d_path: saner calling conventions for __dentry_path() Al Viro
2021-06-25 9:32 ` Justin He
2021-07-07 4:52 ` Justin He
2021-05-19 0:48 ` [PATCH 03/14] d_path: regularize handling of root dentry in __dentry_path() Al Viro
2021-07-07 4:50 ` Justin He
2021-05-19 0:48 ` [PATCH 04/14] d_path: get rid of path_with_deleted() Al Viro
2021-05-19 0:48 ` [PATCH 05/14] getcwd(2): saner logics around prepend_path() call Al Viro
2021-07-07 7:41 ` Justin He
2021-05-19 0:48 ` [PATCH 06/14] d_path: don't bother with return value of prepend() Al Viro
2021-06-24 6:13 ` Justin He
2021-05-19 0:48 ` [PATCH 07/14] d_path: lift -ENAMETOOLONG handling into callers of prepend_path() Al Viro
2021-06-25 9:18 ` Justin He
2021-06-28 5:20 ` Justin He
2021-05-19 0:48 ` [PATCH 08/14] d_path: make prepend_name() boolean Al Viro
2021-05-20 9:12 ` Justin He
2021-05-20 9:19 ` Andy Shevchenko
2021-05-20 14:53 ` Petr Mladek
2021-05-20 19:35 ` Al Viro
2021-07-07 7:43 ` Justin He
2021-05-19 0:48 ` [PATCH 09/14] d_path: introduce struct prepend_buffer Al Viro
2021-06-23 13:28 ` Justin He [this message]
2021-06-24 9:29 ` Enrico Weigelt, metux IT consult
2021-06-25 0:43 ` Justin He
2021-06-28 16:42 ` Enrico Weigelt, metux IT consult
2021-06-28 17:10 ` Andy Shevchenko
2021-05-19 0:48 ` [PATCH 10/14] d_path: prepend_path(): get rid of vfsmnt Al Viro
2021-05-19 0:48 ` [PATCH 11/14] d_path: prepend_path(): lift resetting b in case when we'd return 3 out of loop Al Viro
2021-05-19 0:48 ` [PATCH 12/14] d_path: prepend_path(): lift the inner loop into a new helper Al Viro
2021-05-19 8:07 ` Andy Shevchenko
2021-05-19 15:55 ` Al Viro
2021-07-07 7:52 ` Justin He
2021-05-19 0:49 ` [PATCH 13/14] d_path: prepend_path() is unlikely to return non-zero Al Viro
2021-06-25 8:00 ` Justin He
2021-06-25 17:58 ` Al Viro
2021-06-28 3:28 ` Justin He
2021-06-28 4:14 ` Al Viro
2021-06-28 4:36 ` Justin He
2021-06-28 4:37 ` Justin He
2021-05-19 0:49 ` [PATCH 14/14] getcwd(2): clean up error handling Al Viro
2021-07-07 8:03 ` Justin He
2021-06-24 6:05 ` [PATCH 01/14] d_path: "\0" is {0,0}, not {0} Justin He
2021-05-19 2:39 ` [PATCHSET] d_path cleanups Linus Torvalds
2021-06-22 14:00 ` Justin He
2021-05-09 2:20 ` [PATCH RFC 1/3] fs: introduce helper d_path_fast() Al Viro
2021-05-09 4:58 ` Al Viro
2021-05-10 16:16 ` Eric W. Biederman
2021-05-10 15:07 ` Justin He
2021-05-10 17:03 ` Linus Torvalds
2021-05-08 12:25 ` [PATCH RFC 2/3] lib/vsprintf.c: make %pD print full path for file Jia He
2021-05-10 3:46 ` Sergey Senozhatsky
2021-05-10 13:04 ` Petr Mladek
2021-05-10 14:25 ` Justin He
2021-05-27 7:20 ` Justin He
2021-05-27 9:14 ` Petr Mladek
2021-05-08 12:25 ` [PATCH RFC 3/3] s390/hmcdrv: remove the redundant directory path in debug message Jia He
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=AM6PR08MB4376D92F354CD17445DC4EC1F7089@AM6PR08MB4376.eurprd08.prod.outlook.com \
--to=justin.he@arm.com \
--cc=a.darwish@linutronix.de \
--cc=andriy.shevchenko@linux.intel.com \
--cc=borntraeger@de.ibm.com \
--cc=corbet@lwn.net \
--cc=darrick.wong@oracle.com \
--cc=ebiederm@xmission.com \
--cc=ebiggers@google.com \
--cc=gor@linux.ibm.com \
--cc=hca@linux.ibm.com \
--cc=ira.weiny@intel.com \
--cc=linux-doc@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-s390@vger.kernel.org \
--cc=linux@rasmusvillemoes.dk \
--cc=peterz@infradead.org \
--cc=pmladek@suse.com \
--cc=rostedt@goodmis.org \
--cc=senozhatsky@chromium.org \
--cc=torvalds@linux-foundation.org \
--cc=viro@zeniv.linux.org.uk \
/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).