diff mbox

[40/79] link_path_walk: turn inner loop into explicit goto

Message ID 1430803373-4948-40-git-send-email-viro@ZenIV.linux.org.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Al Viro May 5, 2015, 5:22 a.m. UTC 
From: Al Viro <viro@zeniv.linux.org.uk>

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/namei.c | 61 ++++++++++++++++++++++++++++++++-----------------------------
 1 file changed, 32 insertions(+), 29 deletions(-)

Comments

Linus Torvalds May 5, 2015, 3:09 p.m. UTC | #1 
On Mon, May 4, 2015 at 10:22 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> +l:

This looked like noise.

PLEASE. We're not programming in Pascal (and thank all Gods for that),
so we can have labels that have meaningful names. Also, we're not
ashamed of using goto where it makes sense, so we don't need to try to
hide the labels by making them look like specks of dirt on our
monitor.

So give the label a real name that describes what it is. Not this kind of thing.

                              Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Al Viro May 5, 2015, 3:17 p.m. UTC | #2 
On Tue, May 05, 2015 at 08:09:04AM -0700, Linus Torvalds wrote:
> On Mon, May 4, 2015 at 10:22 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> > +l:
> 
> This looked like noise.
> 
> PLEASE. We're not programming in Pascal (and thank all Gods for that),
> so we can have labels that have meaningful names. Also, we're not
> ashamed of using goto where it makes sense, so we don't need to try to
> hide the labels by making them look like specks of dirt on our
> monitor.
> 
> So give the label a real name that describes what it is. Not this kind of thing.

Something like this_used_to_be_the_beginning_of_loop_body?

Seriously, it's strictly temporary thing - it lives for two commits and
gets killed after that.  Exactly because it has no good inherent meaning.
The label replacing it two commits later (several lines upstream) does,
and it gets better name...
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Linus Torvalds May 5, 2015, 3:21 p.m. UTC | #3 
On Tue, May 5, 2015 at 8:17 AM, Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> Something like this_used_to_be_the_beginning_of_loop_body?

No, but at least "loop:" or "repeat:" or something.

That "l:" really looked like a fly shat on my monitor.

                    Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" 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/fs/namei.c b/fs/namei.c
index d6235ea..d46df22 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1767,6 +1767,10 @@  static int link_path_walk(const char *name, struct nameidata *nd)
 			return err;
 
 		if (err) {
+			struct path link;
+			void *cookie;
+			const char *s;
+
 			if (unlikely(current->link_count >= MAX_NESTED_LINKS)) {
 				path_put_conditional(&nd->link, nd);
 				path_put(&nd->path);
@@ -1777,41 +1781,40 @@  static int link_path_walk(const char *name, struct nameidata *nd)
 			nd->depth++;
 			current->link_count++;
 
-			do {
-				struct path link = nd->link;
-				void *cookie;
-				const char *s = get_link(&link, nd, &cookie);
-
-				if (unlikely(IS_ERR(s))) {
-					err = PTR_ERR(s);
-					current->link_count--;
-					nd->depth--;
-					return err;
+l:
+			link = nd->link;
+			s = get_link(&link, nd, &cookie);
+
+			if (unlikely(IS_ERR(s))) {
+				err = PTR_ERR(s);
+				current->link_count--;
+				nd->depth--;
+				return err;
+			}
+			err = 0;
+			if (unlikely(!s)) {
+				/* jumped */
+				put_link(nd, &link, cookie);
+			} else {
+				if (*s == '/') {
+					if (!nd->root.mnt)
+						set_root(nd);
+					path_put(&nd->path);
+					nd->path = nd->root;
+					path_get(&nd->root);
+					nd->flags |= LOOKUP_JUMPED;
 				}
-				err = 0;
-				if (unlikely(!s)) {
-					/* jumped */
+				nd->inode = nd->path.dentry->d_inode;
+				err = link_path_walk(s, nd);
+				if (unlikely(err)) {
 					put_link(nd, &link, cookie);
-					break;
 				} else {
-					if (*s == '/') {
-						if (!nd->root.mnt)
-							set_root(nd);
-						path_put(&nd->path);
-						nd->path = nd->root;
-						path_get(&nd->root);
-						nd->flags |= LOOKUP_JUMPED;
-					}
-					nd->inode = nd->path.dentry->d_inode;
-					err = link_path_walk(s, nd);
-					if (unlikely(err)) {
-						put_link(nd, &link, cookie);
-						break;
-					}
 					err = walk_component(nd, LOOKUP_FOLLOW);
 					put_link(nd, &link, cookie);
+					if (err > 0)
+						goto l;
 				}
-			} while (err > 0);
+			}
 
 			current->link_count--;
 			nd->depth--;