All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.