diff mbox series

[RFC,v4,15/69] new step_into() flag: WALK_NOFOLLOW

Message ID 20200313235357.2646756-15-viro@ZenIV.linux.org.uk (mailing list archive)
State New, archived
Headers show
Series [RFC,v4,01/69] do_add_mount(): lift lock_mount/unlock_mount into callers | expand

Commit Message

Al Viro March 13, 2020, 11:53 p.m. UTC 
From: Al Viro <viro@zeniv.linux.org.uk>

Tells step_into() not to follow symlinks, regardless of LOOKUP_FOLLOW.
Allows to switch handle_lookup_down() to of step_into(), getting
all follow_managed() and step_into() calls paired.

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

Comments

Linus Torvalds March 14, 2020, 12:32 a.m. UTC | #1 
I mentioned this last time (perhaps for a different sequence):

On Fri, Mar 13, 2020 at 4:54 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
>         if (likely(!d_is_symlink(path->dentry)) ||
> -          !(flags & WALK_FOLLOW || nd->flags & LOOKUP_FOLLOW)) {
> +          !(flags & WALK_FOLLOW || nd->flags & LOOKUP_FOLLOW) ||
> +          flags & WALK_NOFOLLOW) {

Yes, I know that bitwise operations have higher precedence than the
logical ones. And I know & (and &&) have higher precedence than | (and
||).

But I have to _think_ about it every time I see code like this.

I'd really prefer to see

   if ((a & BIT) || (b & ANOTHER_BIT))

over the "equivalent" and shorter

   if (a & BIT || b & ANOTHER_BIT)

Please make it explicit. It wasn't before either, but it _could_ be.

              Linus
Al Viro March 14, 2020, 1:06 a.m. UTC | #2 
On Fri, Mar 13, 2020 at 05:32:32PM -0700, Linus Torvalds wrote:
> I mentioned this last time (perhaps for a different sequence):
> 
> On Fri, Mar 13, 2020 at 4:54 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
> >
> >         if (likely(!d_is_symlink(path->dentry)) ||
> > -          !(flags & WALK_FOLLOW || nd->flags & LOOKUP_FOLLOW)) {
> > +          !(flags & WALK_FOLLOW || nd->flags & LOOKUP_FOLLOW) ||
> > +          flags & WALK_NOFOLLOW) {
> 
> Yes, I know that bitwise operations have higher precedence than the
> logical ones. And I know & (and &&) have higher precedence than | (and
> ||).
> 
> But I have to _think_ about it every time I see code like this.
> 
> I'd really prefer to see
> 
>    if ((a & BIT) || (b & ANOTHER_BIT))
> 
> over the "equivalent" and shorter
> 
>    if (a & BIT || b & ANOTHER_BIT)
> 
> Please make it explicit. It wasn't before either, but it _could_ be.

Not a problem (actually, I'd done that several commits later when I was
rewriting the expression anyway).  Folded the following into it now:

diff --git a/fs/namei.c b/fs/namei.c
index e47b376cf442..79f06be7f5d4 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1839,8 +1839,8 @@ static inline int step_into(struct nameidata *nd, struct path *path,
 			    int flags, struct inode *inode, unsigned seq)
 {
 	if (likely(!d_is_symlink(path->dentry)) ||
-	   !(flags & WALK_FOLLOW || nd->flags & LOOKUP_FOLLOW) ||
-	   flags & WALK_NOFOLLOW) {
+	   !((flags & WALK_FOLLOW) || (nd->flags & LOOKUP_FOLLOW)) ||
+	   (flags & WALK_NOFOLLOW)) {
 		/* not a symlink or should not follow */
 		path_to_nameidata(path, nd);
 		nd->inode = inode;
diff mbox series

Patch

diff --git a/fs/namei.c b/fs/namei.c
index 3097edcb4a1a..e47b376cf442 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1827,7 +1827,7 @@  static int pick_link(struct nameidata *nd, struct path *link,
 	return 1;
 }
 
-enum {WALK_FOLLOW = 1, WALK_MORE = 2};
+enum {WALK_FOLLOW = 1, WALK_MORE = 2, WALK_NOFOLLOW = 4};
 
 /*
  * Do we need to follow links? We _really_ want to be able
@@ -1839,7 +1839,8 @@  static inline int step_into(struct nameidata *nd, struct path *path,
 			    int flags, struct inode *inode, unsigned seq)
 {
 	if (likely(!d_is_symlink(path->dentry)) ||
-	   !(flags & WALK_FOLLOW || nd->flags & LOOKUP_FOLLOW)) {
+	   !(flags & WALK_FOLLOW || nd->flags & LOOKUP_FOLLOW) ||
+	   flags & WALK_NOFOLLOW) {
 		/* not a symlink or should not follow */
 		path_to_nameidata(path, nd);
 		nd->inode = inode;
@@ -2363,10 +2364,7 @@  static int handle_lookup_down(struct nameidata *nd)
 	err = handle_mounts(nd, nd->path.dentry, &path, &inode, &seq);
 	if (unlikely(err < 0))
 		return err;
-	path_to_nameidata(&path, nd);
-	nd->inode = inode;
-	nd->seq = seq;
-	return 0;
+	return step_into(nd, &path, WALK_NOFOLLOW, inode, seq);
 }
 
 /* Returns 0 and nd will be valid on success; Retuns error, otherwise. */