Linux 9p file system development
 help / color / mirror / Atom feed
From: Hongling Zeng <zhongling0719@126.com>
To: ericvh@kernel.org, lucho@ionkov.net, asmadeus@codewreck.org,
	 linux_oss@crudebyte.com,
	David Laight <david.laight.linux@gmail.com>
Cc: Hongling Zeng <zenghongling@kylinos.cn>,
	v9fs@lists.linux.dev,  linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] 9p: avoid returning ERR_PTR(0) from mkdir operations
Date: Wed, 27 May 2026 11:52:02 +0800	[thread overview]
Message-ID: <6A166A62.6030802@126.com> (raw)
In-Reply-To: <20260520022650.14217-1-zenghongling@kylinos.cn>

   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 <david.laight.linux@gmail.com>
> Acked-by: Christian Schoenebeck <linux_oss@crudebyte.com>
> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
>
> ---
> 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


  reply	other threads:[~2026-05-27  3:52 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-20  2:26 [PATCH v2] 9p: avoid returning ERR_PTR(0) from mkdir operations Hongling Zeng
2026-05-27  3:52 ` Hongling Zeng [this message]
2026-05-29  1:55 ` Dominique Martinet

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6A166A62.6030802@126.com \
    --to=zhongling0719@126.com \
    --cc=asmadeus@codewreck.org \
    --cc=david.laight.linux@gmail.com \
    --cc=ericvh@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux_oss@crudebyte.com \
    --cc=lucho@ionkov.net \
    --cc=v9fs@lists.linux.dev \
    --cc=zenghongling@kylinos.cn \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox