[PATCH v2] vfs: replace ints with enum component_type for LAST_XXX

[PATCH v2] vfs: replace ints with enum component_type for LAST_XXX

[Jori Koolstra]

Several functions in namei.c take an "int *type" parameter, such as
filename_parentat(). To know what values this can take you have to find
the anonymous struct that defines the LAST_XXX values. I would argue
that the readability of the code is improved by making this an explicit
type.

Signed-off-by: Jori Koolstra <jkoolstra@xs4all.nl>
Reviewed-by: Jan Kara <jack@suse.cz>

---

v2: move back to LAST_XXX and change int to component_type in
fs/smb/server/vfs.c.

---
 fs/namei.c            | 41 ++++++++++++++++++++++-------------------
 fs/smb/server/vfs.c   |  5 +++--
 include/linux/namei.h |  4 ++--
 3 files changed, 27 insertions(+), 23 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 9e5500dad14f..a880454a6415 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -721,15 +721,15 @@ EXPORT_SYMBOL(path_put);
 
 #define EMBEDDED_LEVELS 2
 struct nameidata {
-	struct path	path;
-	struct qstr	last;
-	struct path	root;
-	struct inode	*inode; /* path.dentry.d_inode */
-	unsigned int	flags, state;
-	unsigned	seq, next_seq, m_seq, r_seq;
-	int		last_type;
-	unsigned	depth;
-	int		total_link_count;
+	struct path		path;
+	struct qstr		last;
+	struct path		root;
+	struct inode		*inode; /* path.dentry.d_inode */
+	unsigned int		flags, state;
+	unsigned		seq, next_seq, m_seq, r_seq;
+	enum component_type	last_type;
+	unsigned		depth;
+	int			total_link_count;
 	struct saved {
 		struct path link;
 		struct delayed_call done;
@@ -2221,7 +2221,7 @@ static struct dentry *follow_dotdot(struct nameidata *nd)
 	return dget(nd->path.dentry);
 }
 
-static const char *handle_dots(struct nameidata *nd, int type)
+static const char *handle_dots(struct nameidata *nd, enum component_type type)
 {
 	if (type == LAST_DOTDOT) {
 		const char *error = NULL;
@@ -2869,7 +2869,7 @@ static int path_parentat(struct nameidata *nd, unsigned flags,
 /* Note: this does not consume "name" */
 static int __filename_parentat(int dfd, struct filename *name,
 			       unsigned int flags, struct path *parent,
-			       struct qstr *last, int *type,
+			       struct qstr *last, enum component_type *type,
 			       const struct path *root)
 {
 	int retval;
@@ -2894,7 +2894,7 @@ static int __filename_parentat(int dfd, struct filename *name,
 
 static int filename_parentat(int dfd, struct filename *name,
 			     unsigned int flags, struct path *parent,
-			     struct qstr *last, int *type)
+			     struct qstr *last, enum component_type *type)
 {
 	return __filename_parentat(dfd, name, flags, parent, last, type, NULL);
 }
@@ -2963,7 +2963,8 @@ static struct dentry *__start_removing_path(int dfd, struct filename *name,
 	struct path parent_path __free(path_put) = {};
 	struct dentry *d;
 	struct qstr last;
-	int type, error;
+	enum component_type type;
+	int error;
 
 	error = filename_parentat(dfd, name, 0, &parent_path, &last, &type);
 	if (error)
@@ -3009,7 +3010,8 @@ struct dentry *kern_path_parent(const char *name, struct path *path)
 	CLASS(filename_kernel, filename)(name);
 	struct dentry *d;
 	struct qstr last;
-	int type, error;
+	enum component_type type;
+	int error;
 
 	error = filename_parentat(AT_FDCWD, filename, 0, &parent_path, &last, &type);
 	if (error)
@@ -3057,7 +3059,7 @@ EXPORT_SYMBOL(kern_path);
  * @root: pointer to struct path of the base directory
  */
 int vfs_path_parent_lookup(struct filename *filename, unsigned int flags,
-			   struct path *parent, struct qstr *last, int *type,
+			   struct path *parent, struct qstr *last, enum component_type *type,
 			   const struct path *root)
 {
 	return  __filename_parentat(AT_FDCWD, filename, flags, parent, last,
@@ -4903,7 +4905,7 @@ static struct dentry *filename_create(int dfd, struct filename *name,
 	bool want_dir = lookup_flags & LOOKUP_DIRECTORY;
 	unsigned int reval_flag = lookup_flags & LOOKUP_REVAL;
 	unsigned int create_flags = LOOKUP_CREATE | LOOKUP_EXCL;
-	int type;
+	enum component_type type;
 	int error;
 
 	error = filename_parentat(dfd, name, reval_flag, path, &last, &type);
@@ -5365,7 +5367,7 @@ int filename_rmdir(int dfd, struct filename *name)
 	struct dentry *dentry;
 	struct path path;
 	struct qstr last;
-	int type;
+	enum component_type type;
 	unsigned int lookup_flags = 0;
 	struct delegated_inode delegated_inode = { };
 retry:
@@ -5383,6 +5385,7 @@ int filename_rmdir(int dfd, struct filename *name)
 	case LAST_ROOT:
 		error = -EBUSY;
 		goto exit2;
+	case LAST_NORM: ; // OK
 	}
 
 	error = mnt_want_write(path.mnt);
@@ -5507,7 +5510,7 @@ int filename_unlinkat(int dfd, struct filename *name)
 	struct dentry *dentry;
 	struct path path;
 	struct qstr last;
-	int type;
+	enum component_type type;
 	struct inode *inode;
 	struct delegated_inode delegated_inode = { };
 	unsigned int lookup_flags = 0;
@@ -6074,7 +6077,7 @@ int filename_renameat2(int olddfd, struct filename *from,
 	struct renamedata rd;
 	struct path old_path, new_path;
 	struct qstr old_last, new_last;
-	int old_type, new_type;
+	enum component_type old_type, new_type;
 	struct delegated_inode delegated_inode = { };
 	unsigned int lookup_flags = 0;
 	bool should_retry = false;
diff --git a/fs/smb/server/vfs.c b/fs/smb/server/vfs.c
index d08973b288e5..b12d481f5ba9 100644
--- a/fs/smb/server/vfs.c
+++ b/fs/smb/server/vfs.c
@@ -56,7 +56,8 @@ static int ksmbd_vfs_path_lookup(struct ksmbd_share_config *share_conf,
 {
 	struct qstr last;
 	const struct path *root_share_path = &share_conf->vfs_path;
-	int err, type;
+	int err;
+	enum component_type type;
 	struct dentry *d;
 
 	if (pathname[0] == '\0') {
@@ -668,7 +669,7 @@ int ksmbd_vfs_rename(struct ksmbd_work *work, const struct path *old_path,
 	struct renamedata rd;
 	struct ksmbd_share_config *share_conf = work->tcon->share_conf;
 	struct ksmbd_file *parent_fp;
-	int new_type;
+	enum component_type new_type;
 	int err, lookup_flags = LOOKUP_NO_SYMLINKS;
 
 	if (ksmbd_override_fsids(work))
diff --git a/include/linux/namei.h b/include/linux/namei.h
index 58600cf234bc..fa3ae87762b7 100644
--- a/include/linux/namei.h
+++ b/include/linux/namei.h
@@ -16,7 +16,7 @@ enum { MAX_NESTED_LINKS = 8 };
 /*
  * Type of the last component on LOOKUP_PARENT
  */
-enum {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT};
+enum component_type {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT};
 
 /* pathwalk mode */
 #define LOOKUP_FOLLOW		BIT(0)	/* follow links at the end */
@@ -70,7 +70,7 @@ static inline void end_removing_path(const struct path *path , struct dentry *de
 	end_creating_path(path, dentry);
 }
 int vfs_path_parent_lookup(struct filename *filename, unsigned int flags,
-			   struct path *parent, struct qstr *last, int *type,
+			   struct path *parent, struct qstr *last, enum component_type *type,
 			   const struct path *root);
 int vfs_path_lookup(struct dentry *, struct vfsmount *, const char *,
 		    unsigned int, struct path *);
-- 
2.53.0

Re: [PATCH v2] vfs: replace ints with enum component_type for LAST_XXX

[Amir Goldstein]

On Sun, Apr 19, 2026 at 6:16 PM Jori Koolstra <jkoolstra@xs4all.nl> wrote:
>
> Several functions in namei.c take an "int *type" parameter, such as
> filename_parentat(). To know what values this can take you have to find
> the anonymous struct that defines the LAST_XXX values. I would argue
> that the readability of the code is improved by making this an explicit
> type.
>
> Signed-off-by: Jori Koolstra <jkoolstra@xs4all.nl>
> Reviewed-by: Jan Kara <jack@suse.cz>
>
> ---
>
> v2: move back to LAST_XXX and change int to component_type in
> fs/smb/server/vfs.c.
>
> ---
>  fs/namei.c            | 41 ++++++++++++++++++++++-------------------
>  fs/smb/server/vfs.c   |  5 +++--
>  include/linux/namei.h |  4 ++--
>  3 files changed, 27 insertions(+), 23 deletions(-)
>
> diff --git a/fs/namei.c b/fs/namei.c
> index 9e5500dad14f..a880454a6415 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -721,15 +721,15 @@ EXPORT_SYMBOL(path_put);
>
>  #define EMBEDDED_LEVELS 2
>  struct nameidata {
> -       struct path     path;
> -       struct qstr     last;
> -       struct path     root;
> -       struct inode    *inode; /* path.dentry.d_inode */
> -       unsigned int    flags, state;
> -       unsigned        seq, next_seq, m_seq, r_seq;
> -       int             last_type;

Please update documentation entry for last_type in
Documentation/filesystems/path-lookup.rst

> -       unsigned        depth;
> -       int             total_link_count;
> +       struct path             path;
> +       struct qstr             last;
> +       struct path             root;
> +       struct inode            *inode; /* path.dentry.d_inode */
> +       unsigned int            flags, state;
> +       unsigned                seq, next_seq, m_seq, r_seq;
> +       enum component_type     last_type;

I really dislike all this white space churn just because the enum name
is so long. If it was long for a good reason - increasing readability -
I may have conceded, but I don't think that is the case.

I do not think that 'component_type' is more clear than 'last_type'
for a developer passing by reading the code.
If anything, the opposite, because 'last_type' or 'type' variables always
appear in the same function with 'last' variable, so it is more clear WHICH
object the type is referring to and matches the LAST_ enum prefix.

> +       unsigned                depth;
> +       int                     total_link_count;
>         struct saved {
>                 struct path link;
>                 struct delayed_call done;
> @@ -2221,7 +2221,7 @@ static struct dentry *follow_dotdot(struct nameidata *nd)
>         return dget(nd->path.dentry);
>  }
>
> -static const char *handle_dots(struct nameidata *nd, int type)
> +static const char *handle_dots(struct nameidata *nd, enum component_type type)
>  {
>         if (type == LAST_DOTDOT) {
>                 const char *error = NULL;
> @@ -2869,7 +2869,7 @@ static int path_parentat(struct nameidata *nd, unsigned flags,
>  /* Note: this does not consume "name" */
>  static int __filename_parentat(int dfd, struct filename *name,
>                                unsigned int flags, struct path *parent,
> -                              struct qstr *last, int *type,
> +                              struct qstr *last, enum component_type *type,
>                                const struct path *root)
>  {
>         int retval;
> @@ -2894,7 +2894,7 @@ static int __filename_parentat(int dfd, struct filename *name,
>
>  static int filename_parentat(int dfd, struct filename *name,
>                              unsigned int flags, struct path *parent,
> -                            struct qstr *last, int *type)
> +                            struct qstr *last, enum component_type *type)
>  {
>         return __filename_parentat(dfd, name, flags, parent, last, type, NULL);
>  }
> @@ -2963,7 +2963,8 @@ static struct dentry *__start_removing_path(int dfd, struct filename *name,
>         struct path parent_path __free(path_put) = {};
>         struct dentry *d;
>         struct qstr last;
> -       int type, error;
> +       enum component_type type;
> +       int error;
>
>         error = filename_parentat(dfd, name, 0, &parent_path, &last, &type);
>         if (error)
> @@ -3009,7 +3010,8 @@ struct dentry *kern_path_parent(const char *name, struct path *path)
>         CLASS(filename_kernel, filename)(name);
>         struct dentry *d;
>         struct qstr last;
> -       int type, error;
> +       enum component_type type;
> +       int error;
>
>         error = filename_parentat(AT_FDCWD, filename, 0, &parent_path, &last, &type);
>         if (error)
> @@ -3057,7 +3059,7 @@ EXPORT_SYMBOL(kern_path);
>   * @root: pointer to struct path of the base directory
>   */
>  int vfs_path_parent_lookup(struct filename *filename, unsigned int flags,
> -                          struct path *parent, struct qstr *last, int *type,
> +                          struct path *parent, struct qstr *last, enum component_type *type,
>                            const struct path *root)
>  {
>         return  __filename_parentat(AT_FDCWD, filename, flags, parent, last,
> @@ -4903,7 +4905,7 @@ static struct dentry *filename_create(int dfd, struct filename *name,
>         bool want_dir = lookup_flags & LOOKUP_DIRECTORY;
>         unsigned int reval_flag = lookup_flags & LOOKUP_REVAL;
>         unsigned int create_flags = LOOKUP_CREATE | LOOKUP_EXCL;
> -       int type;
> +       enum component_type type;
>         int error;
>
>         error = filename_parentat(dfd, name, reval_flag, path, &last, &type);
> @@ -5365,7 +5367,7 @@ int filename_rmdir(int dfd, struct filename *name)
>         struct dentry *dentry;
>         struct path path;
>         struct qstr last;
> -       int type;
> +       enum component_type type;
>         unsigned int lookup_flags = 0;
>         struct delegated_inode delegated_inode = { };
>  retry:
> @@ -5383,6 +5385,7 @@ int filename_rmdir(int dfd, struct filename *name)
>         case LAST_ROOT:
>                 error = -EBUSY;
>                 goto exit2;
> +       case LAST_NORM: ; // OK

I was not aware that the compiler warns about missing cases for enums -
cool feature.
but that needs to be a break; rather than fallthough the end
and as a matter of taste, I would put the valid case first - not critical.

>         }
>
>         error = mnt_want_write(path.mnt);
> @@ -5507,7 +5510,7 @@ int filename_unlinkat(int dfd, struct filename *name)
>         struct dentry *dentry;
>         struct path path;
>         struct qstr last;
> -       int type;
> +       enum component_type type;
>         struct inode *inode;
>         struct delegated_inode delegated_inode = { };
>         unsigned int lookup_flags = 0;
> @@ -6074,7 +6077,7 @@ int filename_renameat2(int olddfd, struct filename *from,
>         struct renamedata rd;
>         struct path old_path, new_path;
>         struct qstr old_last, new_last;
> -       int old_type, new_type;
> +       enum component_type old_type, new_type;
>         struct delegated_inode delegated_inode = { };
>         unsigned int lookup_flags = 0;
>         bool should_retry = false;
> diff --git a/fs/smb/server/vfs.c b/fs/smb/server/vfs.c
> index d08973b288e5..b12d481f5ba9 100644
> --- a/fs/smb/server/vfs.c
> +++ b/fs/smb/server/vfs.c
> @@ -56,7 +56,8 @@ static int ksmbd_vfs_path_lookup(struct ksmbd_share_config *share_conf,
>  {
>         struct qstr last;
>         const struct path *root_share_path = &share_conf->vfs_path;
> -       int err, type;
> +       int err;
> +       enum component_type type;
>         struct dentry *d;
>
>         if (pathname[0] == '\0') {
> @@ -668,7 +669,7 @@ int ksmbd_vfs_rename(struct ksmbd_work *work, const struct path *old_path,
>         struct renamedata rd;
>         struct ksmbd_share_config *share_conf = work->tcon->share_conf;
>         struct ksmbd_file *parent_fp;
> -       int new_type;
> +       enum component_type new_type;
>         int err, lookup_flags = LOOKUP_NO_SYMLINKS;
>
>         if (ksmbd_override_fsids(work))
> diff --git a/include/linux/namei.h b/include/linux/namei.h
> index 58600cf234bc..fa3ae87762b7 100644
> --- a/include/linux/namei.h
> +++ b/include/linux/namei.h
> @@ -16,7 +16,7 @@ enum { MAX_NESTED_LINKS = 8 };
>  /*
>   * Type of the last component on LOOKUP_PARENT
>   */
> -enum {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT};
> +enum component_type {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT};

Please use last_type to match the LAST_ prefix.

TBH, the thing that bothers me most is why is this enum exposed
outside of namei.c
in the first place?

>
>  /* pathwalk mode */
>  #define LOOKUP_FOLLOW          BIT(0)  /* follow links at the end */
> @@ -70,7 +70,7 @@ static inline void end_removing_path(const struct path *path , struct dentry *de
>         end_creating_path(path, dentry);
>  }
>  int vfs_path_parent_lookup(struct filename *filename, unsigned int flags,
> -                          struct path *parent, struct qstr *last, int *type,
> +                          struct path *parent, struct qstr *last, enum component_type *type,
>                            const struct path *root);

As your patch demonstrates, the only use of enum last_type outside of namei.c is
in ksmbd calls to vfs_path_parent_lookup().

This is an "internal" lookup helper that was exposed by commit
74d7970febf7e ("ksmbd: fix racy issue from using ->d_parent and ->d_name")

This commit appears to be using the helper inconsistently in
its two call sites, because ksmbd_vfs_rename(), is not checking the
unlikely(type != LAST_NORM) case.

Perhaps it would be wiser not to expose enum last_type at all and move
the unlikely(type != LAST_NORM) check into the helper itself in namei.c.

WDYT?

Thanks,
Amir.

Re: [PATCH v2] vfs: replace ints with enum component_type for LAST_XXX

[Jori Koolstra]

> Op 20-04-2026 12:29 CEST schreef Amir Goldstein <amir73il@gmail.com>:
> 
> Please update documentation entry for last_type in
> Documentation/filesystems/path-lookup.rst
> 

Yes, thanks for the reminder!

> > -       unsigned        depth;
> > -       int             total_link_count;
> > +       struct path             path;
> > +       struct qstr             last;
> > +       struct path             root;
> > +       struct inode            *inode; /* path.dentry.d_inode */
> > +       unsigned int            flags, state;
> > +       unsigned                seq, next_seq, m_seq, r_seq;
> > +       enum component_type     last_type;
> 
> I really dislike all this white space churn just because the enum name
> is so long. If it was long for a good reason - increasing readability -
> I may have conceded, but I don't think that is the case.
> 
> I do not think that 'component_type' is more clear than 'last_type'
> for a developer passing by reading the code.
> If anything, the opposite, because 'last_type' or 'type' variables always
> appear in the same function with 'last' variable, so it is more clear WHICH
> object the type is referring to and matches the LAST_ enum prefix.

OK, I see your point. I have changed it back. The reason I preferred
component_type is that the whole "last" thing is just incidental. There is
nothing dependent on these components being "last", it's just that the
enum values are only used in that context. It would make more sense to
restrict the enum to namei.c and use NORMAL, DOT, or something like that.
But I do agree that isn't worth the churn :) 

> 
> > +       unsigned                depth;
> > +       int                     total_link_count;
> >         struct saved {
> >                 struct path link;
> >                 struct delayed_call done;
> > @@ -2221,7 +2221,7 @@ static struct dentry *follow_dotdot(struct nameidata *nd)
> >         return dget(nd->path.dentry);
> >  }
> >
> > -static const char *handle_dots(struct nameidata *nd, int type)
> > +static const char *handle_dots(struct nameidata *nd, enum component_type type)
> >  {
> >         if (type == LAST_DOTDOT) {
> >                 const char *error = NULL;
> > @@ -2869,7 +2869,7 @@ static int path_parentat(struct nameidata *nd, unsigned flags,
> >  /* Note: this does not consume "name" */
> >  static int __filename_parentat(int dfd, struct filename *name,
> >                                unsigned int flags, struct path *parent,
> > -                              struct qstr *last, int *type,
> > +                              struct qstr *last, enum component_type *type,
> >                                const struct path *root)
> >  {
> >         int retval;
> > @@ -2894,7 +2894,7 @@ static int __filename_parentat(int dfd, struct filename *name,
> >
> >  static int filename_parentat(int dfd, struct filename *name,
> >                              unsigned int flags, struct path *parent,
> > -                            struct qstr *last, int *type)
> > +                            struct qstr *last, enum component_type *type)
> >  {
> >         return __filename_parentat(dfd, name, flags, parent, last, type, NULL);
> >  }
> > @@ -2963,7 +2963,8 @@ static struct dentry *__start_removing_path(int dfd, struct filename *name,
> >         struct path parent_path __free(path_put) = {};
> >         struct dentry *d;
> >         struct qstr last;
> > -       int type, error;
> > +       enum component_type type;
> > +       int error;
> >
> >         error = filename_parentat(dfd, name, 0, &parent_path, &last, &type);
> >         if (error)
> > @@ -3009,7 +3010,8 @@ struct dentry *kern_path_parent(const char *name, struct path *path)
> >         CLASS(filename_kernel, filename)(name);
> >         struct dentry *d;
> >         struct qstr last;
> > -       int type, error;
> > +       enum component_type type;
> > +       int error;
> >
> >         error = filename_parentat(AT_FDCWD, filename, 0, &parent_path, &last, &type);
> >         if (error)
> > @@ -3057,7 +3059,7 @@ EXPORT_SYMBOL(kern_path);
> >   * @root: pointer to struct path of the base directory
> >   */
> >  int vfs_path_parent_lookup(struct filename *filename, unsigned int flags,
> > -                          struct path *parent, struct qstr *last, int *type,
> > +                          struct path *parent, struct qstr *last, enum component_type *type,
> >                            const struct path *root)
> >  {
> >         return  __filename_parentat(AT_FDCWD, filename, flags, parent, last,
> > @@ -4903,7 +4905,7 @@ static struct dentry *filename_create(int dfd, struct filename *name,
> >         bool want_dir = lookup_flags & LOOKUP_DIRECTORY;
> >         unsigned int reval_flag = lookup_flags & LOOKUP_REVAL;
> >         unsigned int create_flags = LOOKUP_CREATE | LOOKUP_EXCL;
> > -       int type;
> > +       enum component_type type;
> >         int error;
> >
> >         error = filename_parentat(dfd, name, reval_flag, path, &last, &type);
> > @@ -5365,7 +5367,7 @@ int filename_rmdir(int dfd, struct filename *name)
> >         struct dentry *dentry;
> >         struct path path;
> >         struct qstr last;
> > -       int type;
> > +       enum component_type type;
> >         unsigned int lookup_flags = 0;
> >         struct delegated_inode delegated_inode = { };
> >  retry:
> > @@ -5383,6 +5385,7 @@ int filename_rmdir(int dfd, struct filename *name)
> >         case LAST_ROOT:
> >                 error = -EBUSY;
> >                 goto exit2;
> > +       case LAST_NORM: ; // OK
> 
> I was not aware that the compiler warns about missing cases for enums -
> cool feature.
> but that needs to be a break; rather than fallthough the end
> and as a matter of taste, I would put the valid case first - not critical.
> 
> >         }
> >
> >         error = mnt_want_write(path.mnt);
> > @@ -5507,7 +5510,7 @@ int filename_unlinkat(int dfd, struct filename *name)
> >         struct dentry *dentry;
> >         struct path path;
> >         struct qstr last;
> > -       int type;
> > +       enum component_type type;
> >         struct inode *inode;
> >         struct delegated_inode delegated_inode = { };
> >         unsigned int lookup_flags = 0;
> > @@ -6074,7 +6077,7 @@ int filename_renameat2(int olddfd, struct filename *from,
> >         struct renamedata rd;
> >         struct path old_path, new_path;
> >         struct qstr old_last, new_last;
> > -       int old_type, new_type;
> > +       enum component_type old_type, new_type;
> >         struct delegated_inode delegated_inode = { };
> >         unsigned int lookup_flags = 0;
> >         bool should_retry = false;
> > diff --git a/fs/smb/server/vfs.c b/fs/smb/server/vfs.c
> > index d08973b288e5..b12d481f5ba9 100644
> > --- a/fs/smb/server/vfs.c
> > +++ b/fs/smb/server/vfs.c
> > @@ -56,7 +56,8 @@ static int ksmbd_vfs_path_lookup(struct ksmbd_share_config *share_conf,
> >  {
> >         struct qstr last;
> >         const struct path *root_share_path = &share_conf->vfs_path;
> > -       int err, type;
> > +       int err;
> > +       enum component_type type;
> >         struct dentry *d;
> >
> >         if (pathname[0] == '\0') {
> > @@ -668,7 +669,7 @@ int ksmbd_vfs_rename(struct ksmbd_work *work, const struct path *old_path,
> >         struct renamedata rd;
> >         struct ksmbd_share_config *share_conf = work->tcon->share_conf;
> >         struct ksmbd_file *parent_fp;
> > -       int new_type;
> > +       enum component_type new_type;
> >         int err, lookup_flags = LOOKUP_NO_SYMLINKS;
> >
> >         if (ksmbd_override_fsids(work))
> > diff --git a/include/linux/namei.h b/include/linux/namei.h
> > index 58600cf234bc..fa3ae87762b7 100644
> > --- a/include/linux/namei.h
> > +++ b/include/linux/namei.h
> > @@ -16,7 +16,7 @@ enum { MAX_NESTED_LINKS = 8 };
> >  /*
> >   * Type of the last component on LOOKUP_PARENT
> >   */
> > -enum {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT};
> > +enum component_type {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT};
> 
> Please use last_type to match the LAST_ prefix.
> 
> TBH, the thing that bothers me most is why is this enum exposed
> outside of namei.c
> in the first place?
> 
> >
> >  /* pathwalk mode */
> >  #define LOOKUP_FOLLOW          BIT(0)  /* follow links at the end */
> > @@ -70,7 +70,7 @@ static inline void end_removing_path(const struct path *path , struct dentry *de
> >         end_creating_path(path, dentry);
> >  }
> >  int vfs_path_parent_lookup(struct filename *filename, unsigned int flags,
> > -                          struct path *parent, struct qstr *last, int *type,
> > +                          struct path *parent, struct qstr *last, enum component_type *type,
> >                            const struct path *root);
> 
> As your patch demonstrates, the only use of enum last_type outside of namei.c is
> in ksmbd calls to vfs_path_parent_lookup().
> 
> This is an "internal" lookup helper that was exposed by commit
> 74d7970febf7e ("ksmbd: fix racy issue from using ->d_parent and ->d_name")
> 
> This commit appears to be using the helper inconsistently in
> its two call sites, because ksmbd_vfs_rename(), is not checking the
> unlikely(type != LAST_NORM) case.
> 
> Perhaps it would be wiser not to expose enum last_type at all and move
> the unlikely(type != LAST_NORM) check into the helper itself in namei.c.
> 
> WDYT?
> 

Agreed. See my email to Al in this thread.

> Thanks,
> Amir.

Thanks,
Jori.

Re: [PATCH v2] vfs: replace ints with enum component_type for LAST_XXX

[Al Viro]

On Sun, Apr 19, 2026 at 06:16:16PM +0200, Jori Koolstra wrote:
> Several functions in namei.c take an "int *type" parameter, such as
> filename_parentat(). To know what values this can take you have to find
> the anonymous struct that defines the LAST_XXX values.

To find _what_, again?

> I would argue
> that the readability of the code is improved by making this an explicit
> type.

Do argue, then.  How does that improve readability?

What I see is a lot of churn.  Incidentally, I'm not at all sure that we
should expose those outside of fs/namei.c - if you look at the sole place
where any of those are used anywhere else you'll see this:
	err = vfs_path_parent_lookup(filename, flags,
				     path, &last, &type,
				     root_share_path);
	if (err)
		return err;

	if (unlikely(type != LAST_NORM)) {
		path_put(path);
		return -ENOENT;
	}
with the only other caller of vfs_path_parent_lookup() being
	err = vfs_path_parent_lookup(to, lookup_flags | LOOKUP_BENEATH,
				     &new_path, &new_last, &new_type,
				     &share_conf->vfs_path);
	if (err)
		goto out1;
and having no uses of new_type whatsoever.  Smells like missing
check _and_ wrong calling conventions...

Do we ever want to allow anything other thant LAST_NORM in any of
the users?

Re: [PATCH v2] vfs: replace ints with enum component_type for LAST_XXX

[Jori Koolstra]

Hi Al,

> Op 22-04-2026 05:39 CEST schreef Al Viro <viro@zeniv.linux.org.uk>:
> 
>  
> On Sun, Apr 19, 2026 at 06:16:16PM +0200, Jori Koolstra wrote:
> > Several functions in namei.c take an "int *type" parameter, such as
> > filename_parentat(). To know what values this can take you have to find
> > the anonymous struct that defines the LAST_XXX values.
> 
> To find _what_, again?
> 
> > I would argue
> > that the readability of the code is improved by making this an explicit
> > type.
> 
> Do argue, then.  How does that improve readability?

It is certainly in part a matter of taste, so I certainly won't say this
is "wrong."

There is the matter of type safety, and the (possible) benefits this entails
such as automatic case coverage checking for switch-statements. However, I don't
doubt for a second you aren't aware of this, so that probably won't convince
you :) . We indeed also don't do this for flag-types, but at least that
is a kernel-wide established pattern (now I am curious how the Rust people
deal with flags).

What I find somewhat unfortunate is that if I am looking at
__filename_parentat(), for instance, the signature does not give me any
clue what the purpose is of int *type. Then I look through the code and see

		*type = nd.last_type;

and I have to jump to struct nameidata and it still tells me nothing. I have
to grep last_type in nameidata to see it is used in conjunction with the 
anonymous enum.

I can only tell that it would have slightly decreased cognitive load and jumping
around for me when I first looked into namei.c. How that compares to some churn I
can't say.

> 
> What I see is a lot of churn.  Incidentally, I'm not at all sure that we
> should expose those outside of fs/namei.c - if you look at the sole place
> where any of those are used anywhere else you'll see this:
> 	err = vfs_path_parent_lookup(filename, flags,
> 				     path, &last, &type,
> 				     root_share_path);
> 	if (err)
> 		return err;
> 
> 	if (unlikely(type != LAST_NORM)) {
> 		path_put(path);
> 		return -ENOENT;
> 	}
> with the only other caller of vfs_path_parent_lookup() being
> 	err = vfs_path_parent_lookup(to, lookup_flags | LOOKUP_BENEATH,
> 				     &new_path, &new_last, &new_type,
> 				     &share_conf->vfs_path);
> 	if (err)
> 		goto out1;
> and having no uses of new_type whatsoever.  Smells like missing
> check _and_ wrong calling conventions...
> 
> Do we ever want to allow anything other thant LAST_NORM in any of
> the users?

I agree with you and Amir. The naming is also a bit confusing. Maybe, I am
confusing myself but path_parentat() seems to return the parent component of the
pathname passed into nd, not the parent of the resolved path, and these do not
have to be the same if last_type isn't normal. I presume that is why they put
that check into vfs.c.

So what do you think is the best place to move 

 	if (unlikely(type != LAST_NORM)) {
 		path_put(path);
 		return -ENOENT;
 	}

to? path_parentat() or filename_parentat()?

And thanks a lot for taking the time to review, Al, I appreciate it :)

Thanks,
Jori.

[PATCH v2] vfs: replace ints with enum component_type for LAST_XXX

[Jori Koolstra]

Several functions in namei.c take an "int *type" parameter, such as
filename_parentat(). To know what values this can take you have to find
the anonymous struct that defines the LAST_XXX values. I would argue
that the readability of the code is improved by making this an explicit
type.

Signed-off-by: Jori Koolstra <jkoolstra@xs4all.nl>

---

v2: move back to LAST_XXX and change int to component_type in
fs/smb/server/vfs.c.

---
 fs/namei.c            | 41 ++++++++++++++++++++++-------------------
 fs/smb/server/vfs.c   |  5 +++--
 include/linux/namei.h |  4 ++--
 3 files changed, 27 insertions(+), 23 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 9e5500dad14f..a880454a6415 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -721,15 +721,15 @@ EXPORT_SYMBOL(path_put);
 
 #define EMBEDDED_LEVELS 2
 struct nameidata {
-	struct path	path;
-	struct qstr	last;
-	struct path	root;
-	struct inode	*inode; /* path.dentry.d_inode */
-	unsigned int	flags, state;
-	unsigned	seq, next_seq, m_seq, r_seq;
-	int		last_type;
-	unsigned	depth;
-	int		total_link_count;
+	struct path		path;
+	struct qstr		last;
+	struct path		root;
+	struct inode		*inode; /* path.dentry.d_inode */
+	unsigned int		flags, state;
+	unsigned		seq, next_seq, m_seq, r_seq;
+	enum component_type	last_type;
+	unsigned		depth;
+	int			total_link_count;
 	struct saved {
 		struct path link;
 		struct delayed_call done;
@@ -2221,7 +2221,7 @@ static struct dentry *follow_dotdot(struct nameidata *nd)
 	return dget(nd->path.dentry);
 }
 
-static const char *handle_dots(struct nameidata *nd, int type)
+static const char *handle_dots(struct nameidata *nd, enum component_type type)
 {
 	if (type == LAST_DOTDOT) {
 		const char *error = NULL;
@@ -2869,7 +2869,7 @@ static int path_parentat(struct nameidata *nd, unsigned flags,
 /* Note: this does not consume "name" */
 static int __filename_parentat(int dfd, struct filename *name,
 			       unsigned int flags, struct path *parent,
-			       struct qstr *last, int *type,
+			       struct qstr *last, enum component_type *type,
 			       const struct path *root)
 {
 	int retval;
@@ -2894,7 +2894,7 @@ static int __filename_parentat(int dfd, struct filename *name,
 
 static int filename_parentat(int dfd, struct filename *name,
 			     unsigned int flags, struct path *parent,
-			     struct qstr *last, int *type)
+			     struct qstr *last, enum component_type *type)
 {
 	return __filename_parentat(dfd, name, flags, parent, last, type, NULL);
 }
@@ -2963,7 +2963,8 @@ static struct dentry *__start_removing_path(int dfd, struct filename *name,
 	struct path parent_path __free(path_put) = {};
 	struct dentry *d;
 	struct qstr last;
-	int type, error;
+	enum component_type type;
+	int error;
 
 	error = filename_parentat(dfd, name, 0, &parent_path, &last, &type);
 	if (error)
@@ -3009,7 +3010,8 @@ struct dentry *kern_path_parent(const char *name, struct path *path)
 	CLASS(filename_kernel, filename)(name);
 	struct dentry *d;
 	struct qstr last;
-	int type, error;
+	enum component_type type;
+	int error;
 
 	error = filename_parentat(AT_FDCWD, filename, 0, &parent_path, &last, &type);
 	if (error)
@@ -3057,7 +3059,7 @@ EXPORT_SYMBOL(kern_path);
  * @root: pointer to struct path of the base directory
  */
 int vfs_path_parent_lookup(struct filename *filename, unsigned int flags,
-			   struct path *parent, struct qstr *last, int *type,
+			   struct path *parent, struct qstr *last, enum component_type *type,
 			   const struct path *root)
 {
 	return  __filename_parentat(AT_FDCWD, filename, flags, parent, last,
@@ -4903,7 +4905,7 @@ static struct dentry *filename_create(int dfd, struct filename *name,
 	bool want_dir = lookup_flags & LOOKUP_DIRECTORY;
 	unsigned int reval_flag = lookup_flags & LOOKUP_REVAL;
 	unsigned int create_flags = LOOKUP_CREATE | LOOKUP_EXCL;
-	int type;
+	enum component_type type;
 	int error;
 
 	error = filename_parentat(dfd, name, reval_flag, path, &last, &type);
@@ -5365,7 +5367,7 @@ int filename_rmdir(int dfd, struct filename *name)
 	struct dentry *dentry;
 	struct path path;
 	struct qstr last;
-	int type;
+	enum component_type type;
 	unsigned int lookup_flags = 0;
 	struct delegated_inode delegated_inode = { };
 retry:
@@ -5383,6 +5385,7 @@ int filename_rmdir(int dfd, struct filename *name)
 	case LAST_ROOT:
 		error = -EBUSY;
 		goto exit2;
+	case LAST_NORM: ; // OK
 	}
 
 	error = mnt_want_write(path.mnt);
@@ -5507,7 +5510,7 @@ int filename_unlinkat(int dfd, struct filename *name)
 	struct dentry *dentry;
 	struct path path;
 	struct qstr last;
-	int type;
+	enum component_type type;
 	struct inode *inode;
 	struct delegated_inode delegated_inode = { };
 	unsigned int lookup_flags = 0;
@@ -6074,7 +6077,7 @@ int filename_renameat2(int olddfd, struct filename *from,
 	struct renamedata rd;
 	struct path old_path, new_path;
 	struct qstr old_last, new_last;
-	int old_type, new_type;
+	enum component_type old_type, new_type;
 	struct delegated_inode delegated_inode = { };
 	unsigned int lookup_flags = 0;
 	bool should_retry = false;
diff --git a/fs/smb/server/vfs.c b/fs/smb/server/vfs.c
index d08973b288e5..b12d481f5ba9 100644
--- a/fs/smb/server/vfs.c
+++ b/fs/smb/server/vfs.c
@@ -56,7 +56,8 @@ static int ksmbd_vfs_path_lookup(struct ksmbd_share_config *share_conf,
 {
 	struct qstr last;
 	const struct path *root_share_path = &share_conf->vfs_path;
-	int err, type;
+	int err;
+	enum component_type type;
 	struct dentry *d;
 
 	if (pathname[0] == '\0') {
@@ -668,7 +669,7 @@ int ksmbd_vfs_rename(struct ksmbd_work *work, const struct path *old_path,
 	struct renamedata rd;
 	struct ksmbd_share_config *share_conf = work->tcon->share_conf;
 	struct ksmbd_file *parent_fp;
-	int new_type;
+	enum component_type new_type;
 	int err, lookup_flags = LOOKUP_NO_SYMLINKS;
 
 	if (ksmbd_override_fsids(work))
diff --git a/include/linux/namei.h b/include/linux/namei.h
index 58600cf234bc..fa3ae87762b7 100644
--- a/include/linux/namei.h
+++ b/include/linux/namei.h
@@ -16,7 +16,7 @@ enum { MAX_NESTED_LINKS = 8 };
 /*
  * Type of the last component on LOOKUP_PARENT
  */
-enum {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT};
+enum component_type {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT};
 
 /* pathwalk mode */
 #define LOOKUP_FOLLOW		BIT(0)	/* follow links at the end */
@@ -70,7 +70,7 @@ static inline void end_removing_path(const struct path *path , struct dentry *de
 	end_creating_path(path, dentry);
 }
 int vfs_path_parent_lookup(struct filename *filename, unsigned int flags,
-			   struct path *parent, struct qstr *last, int *type,
+			   struct path *parent, struct qstr *last, enum component_type *type,
 			   const struct path *root);
 int vfs_path_lookup(struct dentry *, struct vfsmount *, const char *,
 		    unsigned int, struct path *);
-- 
2.53.0

Re: [PATCH v2] vfs: replace ints with enum component_type for LAST_XXX

[Jan Kara]

On Wed 01-04-26 19:43:03, Jori Koolstra wrote:
> Several functions in namei.c take an "int *type" parameter, such as
> filename_parentat(). To know what values this can take you have to find
> the anonymous struct that defines the LAST_XXX values. I would argue
> that the readability of the code is improved by making this an explicit
> type.
> 
> Signed-off-by: Jori Koolstra <jkoolstra@xs4all.nl>

I guess less churn is better :). Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> 
> ---
> 
> v2: move back to LAST_XXX and change int to component_type in
> fs/smb/server/vfs.c.
> 
> ---
>  fs/namei.c            | 41 ++++++++++++++++++++++-------------------
>  fs/smb/server/vfs.c   |  5 +++--
>  include/linux/namei.h |  4 ++--
>  3 files changed, 27 insertions(+), 23 deletions(-)
> 
> diff --git a/fs/namei.c b/fs/namei.c
> index 9e5500dad14f..a880454a6415 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -721,15 +721,15 @@ EXPORT_SYMBOL(path_put);
>  
>  #define EMBEDDED_LEVELS 2
>  struct nameidata {
> -	struct path	path;
> -	struct qstr	last;
> -	struct path	root;
> -	struct inode	*inode; /* path.dentry.d_inode */
> -	unsigned int	flags, state;
> -	unsigned	seq, next_seq, m_seq, r_seq;
> -	int		last_type;
> -	unsigned	depth;
> -	int		total_link_count;
> +	struct path		path;
> +	struct qstr		last;
> +	struct path		root;
> +	struct inode		*inode; /* path.dentry.d_inode */
> +	unsigned int		flags, state;
> +	unsigned		seq, next_seq, m_seq, r_seq;
> +	enum component_type	last_type;
> +	unsigned		depth;
> +	int			total_link_count;
>  	struct saved {
>  		struct path link;
>  		struct delayed_call done;
> @@ -2221,7 +2221,7 @@ static struct dentry *follow_dotdot(struct nameidata *nd)
>  	return dget(nd->path.dentry);
>  }
>  
> -static const char *handle_dots(struct nameidata *nd, int type)
> +static const char *handle_dots(struct nameidata *nd, enum component_type type)
>  {
>  	if (type == LAST_DOTDOT) {
>  		const char *error = NULL;
> @@ -2869,7 +2869,7 @@ static int path_parentat(struct nameidata *nd, unsigned flags,
>  /* Note: this does not consume "name" */
>  static int __filename_parentat(int dfd, struct filename *name,
>  			       unsigned int flags, struct path *parent,
> -			       struct qstr *last, int *type,
> +			       struct qstr *last, enum component_type *type,
>  			       const struct path *root)
>  {
>  	int retval;
> @@ -2894,7 +2894,7 @@ static int __filename_parentat(int dfd, struct filename *name,
>  
>  static int filename_parentat(int dfd, struct filename *name,
>  			     unsigned int flags, struct path *parent,
> -			     struct qstr *last, int *type)
> +			     struct qstr *last, enum component_type *type)
>  {
>  	return __filename_parentat(dfd, name, flags, parent, last, type, NULL);
>  }
> @@ -2963,7 +2963,8 @@ static struct dentry *__start_removing_path(int dfd, struct filename *name,
>  	struct path parent_path __free(path_put) = {};
>  	struct dentry *d;
>  	struct qstr last;
> -	int type, error;
> +	enum component_type type;
> +	int error;
>  
>  	error = filename_parentat(dfd, name, 0, &parent_path, &last, &type);
>  	if (error)
> @@ -3009,7 +3010,8 @@ struct dentry *kern_path_parent(const char *name, struct path *path)
>  	CLASS(filename_kernel, filename)(name);
>  	struct dentry *d;
>  	struct qstr last;
> -	int type, error;
> +	enum component_type type;
> +	int error;
>  
>  	error = filename_parentat(AT_FDCWD, filename, 0, &parent_path, &last, &type);
>  	if (error)
> @@ -3057,7 +3059,7 @@ EXPORT_SYMBOL(kern_path);
>   * @root: pointer to struct path of the base directory
>   */
>  int vfs_path_parent_lookup(struct filename *filename, unsigned int flags,
> -			   struct path *parent, struct qstr *last, int *type,
> +			   struct path *parent, struct qstr *last, enum component_type *type,
>  			   const struct path *root)
>  {
>  	return  __filename_parentat(AT_FDCWD, filename, flags, parent, last,
> @@ -4903,7 +4905,7 @@ static struct dentry *filename_create(int dfd, struct filename *name,
>  	bool want_dir = lookup_flags & LOOKUP_DIRECTORY;
>  	unsigned int reval_flag = lookup_flags & LOOKUP_REVAL;
>  	unsigned int create_flags = LOOKUP_CREATE | LOOKUP_EXCL;
> -	int type;
> +	enum component_type type;
>  	int error;
>  
>  	error = filename_parentat(dfd, name, reval_flag, path, &last, &type);
> @@ -5365,7 +5367,7 @@ int filename_rmdir(int dfd, struct filename *name)
>  	struct dentry *dentry;
>  	struct path path;
>  	struct qstr last;
> -	int type;
> +	enum component_type type;
>  	unsigned int lookup_flags = 0;
>  	struct delegated_inode delegated_inode = { };
>  retry:
> @@ -5383,6 +5385,7 @@ int filename_rmdir(int dfd, struct filename *name)
>  	case LAST_ROOT:
>  		error = -EBUSY;
>  		goto exit2;
> +	case LAST_NORM: ; // OK
>  	}
>  
>  	error = mnt_want_write(path.mnt);
> @@ -5507,7 +5510,7 @@ int filename_unlinkat(int dfd, struct filename *name)
>  	struct dentry *dentry;
>  	struct path path;
>  	struct qstr last;
> -	int type;
> +	enum component_type type;
>  	struct inode *inode;
>  	struct delegated_inode delegated_inode = { };
>  	unsigned int lookup_flags = 0;
> @@ -6074,7 +6077,7 @@ int filename_renameat2(int olddfd, struct filename *from,
>  	struct renamedata rd;
>  	struct path old_path, new_path;
>  	struct qstr old_last, new_last;
> -	int old_type, new_type;
> +	enum component_type old_type, new_type;
>  	struct delegated_inode delegated_inode = { };
>  	unsigned int lookup_flags = 0;
>  	bool should_retry = false;
> diff --git a/fs/smb/server/vfs.c b/fs/smb/server/vfs.c
> index d08973b288e5..b12d481f5ba9 100644
> --- a/fs/smb/server/vfs.c
> +++ b/fs/smb/server/vfs.c
> @@ -56,7 +56,8 @@ static int ksmbd_vfs_path_lookup(struct ksmbd_share_config *share_conf,
>  {
>  	struct qstr last;
>  	const struct path *root_share_path = &share_conf->vfs_path;
> -	int err, type;
> +	int err;
> +	enum component_type type;
>  	struct dentry *d;
>  
>  	if (pathname[0] == '\0') {
> @@ -668,7 +669,7 @@ int ksmbd_vfs_rename(struct ksmbd_work *work, const struct path *old_path,
>  	struct renamedata rd;
>  	struct ksmbd_share_config *share_conf = work->tcon->share_conf;
>  	struct ksmbd_file *parent_fp;
> -	int new_type;
> +	enum component_type new_type;
>  	int err, lookup_flags = LOOKUP_NO_SYMLINKS;
>  
>  	if (ksmbd_override_fsids(work))
> diff --git a/include/linux/namei.h b/include/linux/namei.h
> index 58600cf234bc..fa3ae87762b7 100644
> --- a/include/linux/namei.h
> +++ b/include/linux/namei.h
> @@ -16,7 +16,7 @@ enum { MAX_NESTED_LINKS = 8 };
>  /*
>   * Type of the last component on LOOKUP_PARENT
>   */
> -enum {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT};
> +enum component_type {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT};
>  
>  /* pathwalk mode */
>  #define LOOKUP_FOLLOW		BIT(0)	/* follow links at the end */
> @@ -70,7 +70,7 @@ static inline void end_removing_path(const struct path *path , struct dentry *de
>  	end_creating_path(path, dentry);
>  }
>  int vfs_path_parent_lookup(struct filename *filename, unsigned int flags,
> -			   struct path *parent, struct qstr *last, int *type,
> +			   struct path *parent, struct qstr *last, enum component_type *type,
>  			   const struct path *root);
>  int vfs_path_lookup(struct dentry *, struct vfsmount *, const char *,
>  		    unsigned int, struct path *);
> -- 
> 2.53.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR