Linux 9p file system development
 help / color / mirror / Atom feed
* [PATCH v2] 9p: avoid returning ERR_PTR(0) from mkdir operations
@ 2026-05-20  2:26 Hongling Zeng
  2026-05-27  3:52 ` Hongling Zeng
  2026-05-29  1:55 ` Dominique Martinet
  0 siblings, 2 replies; 3+ messages in thread
From: Hongling Zeng @ 2026-05-20  2:26 UTC (permalink / raw)
  To: ericvh, lucho, asmadeus, linux_oss
  Cc: v9fs, linux-kernel, zhongling0719, Hongling Zeng, David Laight

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


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] 9p: avoid returning ERR_PTR(0) from mkdir operations
  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
  2026-05-29  1:55 ` Dominique Martinet
  1 sibling, 0 replies; 3+ messages in thread
From: Hongling Zeng @ 2026-05-27  3:52 UTC (permalink / raw)
  To: ericvh, lucho, asmadeus, linux_oss, David Laight
  Cc: Hongling Zeng, v9fs, linux-kernel

   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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] 9p: avoid returning ERR_PTR(0) from mkdir operations
  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
@ 2026-05-29  1:55 ` Dominique Martinet
  1 sibling, 0 replies; 3+ messages in thread
From: Dominique Martinet @ 2026-05-29  1:55 UTC (permalink / raw)
  To: Hongling Zeng
  Cc: ericvh, lucho, linux_oss, v9fs, linux-kernel, zhongling0719,
	David Laight

Hongling Zeng wrote on Wed, May 20, 2026 at 10:26:50AM +0800:
> 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>

Thanks for the v2, this looks correct to me.

The commit message should mention that v9fs_create() never returns NULL
and v9fs_vfs_mkdir() can be simplified accordingly, but I'll fix that on
my end

-- 
Dominique

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-05-29  1:55 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
2026-05-29  1:55 ` Dominique Martinet

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox