[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>

---

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

[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

[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?