Re: [PATCH v3 01/14] fs/namei.c: use trailing_slashes()

[David Laight <david.laight.linux@gmail.com>]

On Mon, 06 Jul 2026 13:56:00 +1000
NeilBrown <neilb@ownmail.net> wrote:

> On Sun, 05 Jul 2026, Jori Koolstra wrote:
> > There are several places in fs/namei.c that can use the
> > trailing_slashes() function to improve intent.  
> 
> I think it would help to state here that trailing_slashes() is changed
> to take a qstr instead of a nameidata as that is useful in more places.

It is also taking the struct 'by value' so should really be always_inline
before something makes a compiler decide it should be a real function.

(Or make it take a pointer)

	David

> 
> Reviewed-by: NeilBrown <neil@brown.name>
> 
> Thanks,
> NeilBrown
> 
> 
> > 
> > Signed-off-by: Jori Koolstra <jkoolstra@xs4all.nl>
> > ---
> >  fs/namei.c | 26 +++++++++++++-------------
> >  1 file changed, 13 insertions(+), 13 deletions(-)
> > 
> > diff --git a/fs/namei.c b/fs/namei.c
> > index 19ce43c9a6e6..a8890bc0ae21 100644
> > --- a/fs/namei.c
> > +++ b/fs/namei.c
> > @@ -2781,9 +2781,14 @@ static const char *path_init(struct nameidata *nd, unsigned flags)
> >  	return s;
> >  }
> >  
> > +static inline bool trailing_slashes(const struct qstr last)
> > +{
> > +	return (bool)last.name[last.len];
> > +}
> > +
> >  static inline const char *lookup_last(struct nameidata *nd)
> >  {
> > -	if (nd->last_type == LAST_NORM && nd->last.name[nd->last.len])
> > +	if (nd->last_type == LAST_NORM && trailing_slashes(nd->last))
> >  		nd->flags |= LOOKUP_FOLLOW | LOOKUP_DIRECTORY;
> >  
> >  	return walk_component(nd, WALK_TRAILING);
> > @@ -4521,17 +4526,12 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
> >  	return ERR_PTR(error);
> >  }
> >  
> > -static inline bool trailing_slashes(struct nameidata *nd)
> > -{
> > -	return (bool)nd->last.name[nd->last.len];
> > -}
> > -
> >  static struct dentry *lookup_fast_for_open(struct nameidata *nd, int open_flag)
> >  {
> >  	struct dentry *dentry;
> >  
> >  	if (open_flag & O_CREAT) {
> > -		if (trailing_slashes(nd))
> > +		if (trailing_slashes(nd->last))
> >  			return ERR_PTR(-EISDIR);
> >  
> >  		/* Don't bother on an O_EXCL create */
> > @@ -4539,7 +4539,7 @@ static struct dentry *lookup_fast_for_open(struct nameidata *nd, int open_flag)
> >  			return NULL;
> >  	}
> >  
> > -	if (trailing_slashes(nd))
> > +	if (trailing_slashes(nd->last))
> >  		nd->flags |= LOOKUP_FOLLOW | LOOKUP_DIRECTORY;
> >  
> >  	dentry = lookup_fast(nd);
> > @@ -4950,7 +4950,7 @@ static struct dentry *filename_create(int dfd, struct filename *name,
> >  	 * Do the final lookup.  Suppress 'create' if there is a trailing
> >  	 * '/', and a directory wasn't requested.
> >  	 */
> > -	if (last.name[last.len] && !want_dir)
> > +	if (trailing_slashes(last) && !want_dir)
> >  		create_flags &= ~LOOKUP_CREATE;
> >  	dentry = start_dirop(path->dentry, &last, reval_flag | create_flags);
> >  	if (IS_ERR(dentry))
> > @@ -5569,7 +5569,7 @@ int filename_unlinkat(int dfd, struct filename *name)
> >  		goto exit_drop_write;
> >  
> >  	/* Why not before? Because we want correct error value */
> > -	if (unlikely(last.name[last.len])) {
> > +	if (unlikely(trailing_slashes(last))) {
> >  		if (d_is_dir(dentry))
> >  			error = -EISDIR;
> >  		else
> > @@ -6171,16 +6171,16 @@ int filename_renameat2(int olddfd, struct filename *from,
> >  	if (flags & RENAME_EXCHANGE) {
> >  		if (!d_is_dir(rd.new_dentry)) {
> >  			error = -ENOTDIR;
> > -			if (new_last.name[new_last.len])
> > +			if (trailing_slashes(new_last))
> >  				goto exit_unlock;
> >  		}
> >  	}
> >  	/* unless the source is a directory trailing slashes give -ENOTDIR */
> >  	if (!d_is_dir(rd.old_dentry)) {
> >  		error = -ENOTDIR;
> > -		if (old_last.name[old_last.len])
> > +		if (trailing_slashes(old_last))
> >  			goto exit_unlock;
> > -		if (!(flags & RENAME_EXCHANGE) && new_last.name[new_last.len])
> > +		if (!(flags & RENAME_EXCHANGE) && trailing_slashes(new_last))
> >  			goto exit_unlock;
> >  	}
> >  
> > -- 
> > 2.55.0
> > 
> >   
> 
>