Re: [PATCH v2] 9p: avoid returning ERR_PTR(0) from mkdir operations

Hongling Zeng <[email protected]> Wed, 27 May 2026 11:52:02 +0800
Newsgroups dev.linux.lists.v9fs,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
   Hi David,

   Gentle ping for review on this patch v2. Since you originally suggested
   this approach, I wanted to make sure the final implementation matches 
your
   intent before it gets merged.

   Thanks,
   Hongling

在 2026年05月20日 10:26, Hongling Zeng 写道:
> When mkdir succeeds, v9fs_vfs_mkdir_dotl() and v9fs_vfs_mkdir() return
> ERR_PTR(0) which is incorrect. They should return NULL instead for
> success and ERR_PTR() only with negative error codes for failure.
>
> Return NULL instead of passing to ERR_PTR while err is zero
> Fixes smatch warnings:
>    fs/9p/vfs_inode_dotl.c:420 v9fs_vfs_mkdir_dotl() warn: passing zero to 'ERR_PTR'
>    fs/9p/vfs_inode.c:695 v9fs_vfs_mkdir() warn: passing zero to 'ERR_PTR'
>
> This change does not alter the runtime behavior since ERR_PTR(0) and NULL
> are equivalent. However, it improves code readability and silences static
> analyzer warnings.
>
> Fixes: 88d5baf69082 ("Change inode_operations.mkdir to return struct dentry *")
> Suggested-by: David Laight <[email protected]>
> Acked-by: Christian Schoenebeck <[email protected]>
> Signed-off-by: Hongling Zeng <[email protected]>
>
> ---
> change inv1:
> - Add clarification in commit message that this is not a functional
>    change, as suggested by Christian Schoenebeck.
> - Add acked-by.
> ---
> Changes in v2:
> - Simplified v9fs_vfs_mkdir() implementation based on reviewer feedback
> - Remove unnecessary 'err' variable and use ERR_CAST()
> - Add suggested-by
> ---
>   fs/9p/vfs_inode.c      | 19 ++++++-------------
>   fs/9p/vfs_inode_dotl.c |  4 ++--
>   2 files changed, 8 insertions(+), 15 deletions(-)
>
> diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
> index d1508b1fe109..d2692bc20b08 100644
> --- a/fs/9p/vfs_inode.c
> +++ b/fs/9p/vfs_inode.c
> @@ -672,27 +672,20 @@ v9fs_vfs_create(struct mnt_idmap *idmap, struct inode *dir,
>   static struct dentry *v9fs_vfs_mkdir(struct mnt_idmap *idmap, struct inode *dir,
>   				     struct dentry *dentry, umode_t mode)
>   {
> -	int err;
>   	u32 perm;
>   	struct p9_fid *fid;
>   	struct v9fs_session_info *v9ses;
>   
>   	p9_debug(P9_DEBUG_VFS, "name %pd\n", dentry);
> -	err = 0;
>   	v9ses = v9fs_inode2v9ses(dir);
>   	perm = unixmode2p9mode(v9ses, mode | S_IFDIR);
>   	fid = v9fs_create(v9ses, dir, dentry, NULL, perm, P9_OREAD);
> -	if (IS_ERR(fid)) {
> -		err = PTR_ERR(fid);
> -		fid = NULL;
> -	} else {
> -		inc_nlink(dir);
> -		v9fs_invalidate_inode_attr(dir);
> -	}
> -
> -	if (fid)
> -		p9_fid_put(fid);
> -	return ERR_PTR(err);
> +	if (IS_ERR(fid))
> +		return ERR_CAST(fid);
> +	inc_nlink(dir);
> +	v9fs_invalidate_inode_attr(dir);
> +	p9_fid_put(fid);
> +	return NULL;
>   }
>   
>   /**
> diff --git a/fs/9p/vfs_inode_dotl.c b/fs/9p/vfs_inode_dotl.c
> index 71796a89bcf4..83a52a85ce08 100644
> --- a/fs/9p/vfs_inode_dotl.c
> +++ b/fs/9p/vfs_inode_dotl.c
> @@ -349,7 +349,7 @@ static struct dentry *v9fs_vfs_mkdir_dotl(struct mnt_idmap *idmap,
>   					  struct inode *dir, struct dentry *dentry,
>   					  umode_t omode)
>   {
> -	int err;
> +	int err = 0;
>   	struct v9fs_session_info *v9ses;
>   	struct p9_fid *fid = NULL, *dfid = NULL;
>   	kgid_t gid;
> @@ -412,7 +412,7 @@ static struct dentry *v9fs_vfs_mkdir_dotl(struct mnt_idmap *idmap,
>   	p9_fid_put(fid);
>   	v9fs_put_acl(dacl, pacl);
>   	p9_fid_put(dfid);
> -	return ERR_PTR(err);
> +	return err ? ERR_PTR(err) : NULL;
>   }
>   
>   static int