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