* [PATCH v1] 9p: Fix mkdir to return NULL on success
@ 2026-05-11 6:18 Hongling Zeng
2026-05-19 12:21 ` Dominique Martinet
2026-05-19 17:35 ` David Laight
0 siblings, 2 replies; 6+ messages in thread
From: Hongling Zeng @ 2026-05-11 6:18 UTC (permalink / raw)
To: ericvh, lucho, asmadeus, linux_oss
Cc: v9fs, linux-kernel, zhongling0719, 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 *")
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.
---
---
fs/9p/vfs_inode.c | 5 ++---
fs/9p/vfs_inode_dotl.c | 4 ++--
2 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
index d1508b1fe109..d2d18c796b58 100644
--- a/fs/9p/vfs_inode.c
+++ b/fs/9p/vfs_inode.c
@@ -672,13 +672,12 @@ 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;
+ int err = 0;
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);
@@ -692,7 +691,7 @@ static struct dentry *v9fs_vfs_mkdir(struct mnt_idmap *idmap, struct inode *dir,
if (fid)
p9_fid_put(fid);
- return ERR_PTR(err);
+ return err ? ERR_PTR(err) : 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] 6+ messages in thread
* Re: [PATCH v1] 9p: Fix mkdir to return NULL on success
2026-05-11 6:18 [PATCH v1] 9p: Fix mkdir to return NULL on success Hongling Zeng
@ 2026-05-19 12:21 ` Dominique Martinet
2026-05-20 1:42 ` Hongling Zeng
2026-05-19 17:35 ` David Laight
1 sibling, 1 reply; 6+ messages in thread
From: Dominique Martinet @ 2026-05-19 12:21 UTC (permalink / raw)
To: Hongling Zeng; +Cc: ericvh, lucho, linux_oss, v9fs, linux-kernel, zhongling0719
Hongling Zeng wrote on Mon, May 11, 2026 at 02:18:30PM +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 *")
> Acked-by: Christian Schoenebeck <linux_oss@crudebyte.com>
> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
Thanks, I've picked this up with s/Fix/make/ in the commit subject, as I
(subjectively) don't consider this a fix given it's only a style change
--
Dominique
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1] 9p: Fix mkdir to return NULL on success
2026-05-11 6:18 [PATCH v1] 9p: Fix mkdir to return NULL on success Hongling Zeng
2026-05-19 12:21 ` Dominique Martinet
@ 2026-05-19 17:35 ` David Laight
2026-05-20 1:24 ` Hongling Zeng
1 sibling, 1 reply; 6+ messages in thread
From: David Laight @ 2026-05-19 17:35 UTC (permalink / raw)
To: Hongling Zeng
Cc: ericvh, lucho, asmadeus, linux_oss, v9fs, linux-kernel,
zhongling0719
On Mon, 11 May 2026 14:18:30 +0800
Hongling Zeng <zenghongling@kylinos.cn> wrote:
> 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 *")
> 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.
> ---
> ---
> fs/9p/vfs_inode.c | 5 ++---
> fs/9p/vfs_inode_dotl.c | 4 ++--
> 2 files changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
> index d1508b1fe109..d2d18c796b58 100644
> --- a/fs/9p/vfs_inode.c
> +++ b/fs/9p/vfs_inode.c
> @@ -672,13 +672,12 @@ 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;
> + int err = 0;
> 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);
> @@ -692,7 +691,7 @@ static struct dentry *v9fs_vfs_mkdir(struct mnt_idmap *idmap, struct inode *dir,
>
> if (fid)
> p9_fid_put(fid);
> - return ERR_PTR(err);
> + return err ? ERR_PTR(err) : NULL;
> }
That function is odd, it ends:
fid = v9fs_create(v9ses, dir, dentry, NULL, perm, P9_OREAD);
if (IS_ERR(fid)) {
err = PTR_ERR(fid);
fid = NULL;
A 'return' here would solve the mess later on.
} else {
inc_nlink(dir);
v9fs_invalidate_inode_attr(dir);
}
Can v9fs_create() even return NULL?
In any case p9_fid_put() already contains an (inline) check for both
NULL and IS_ERR() so the 'if (fid)' and 'fid == NULL' (above) are pointless.
if (fid)
p9_fid_put(fid);
return ERR_PTR(err);
-- David
>
> /**
> 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] 6+ messages in thread
* Re: [PATCH v1] 9p: Fix mkdir to return NULL on success
2026-05-19 17:35 ` David Laight
@ 2026-05-20 1:24 ` Hongling Zeng
0 siblings, 0 replies; 6+ messages in thread
From: Hongling Zeng @ 2026-05-20 1:24 UTC (permalink / raw)
To: David Laight, Hongling Zeng
Cc: ericvh, lucho, asmadeus, linux_oss, v9fs, linux-kernel
Thank you for the review. You're right about the unnecessary err variable
and NULL check - v9fs_create() never returns NULL and p9_fid_put()
handles
both NULL and IS_ERR cases internally.
I'll prepare a v2 patch with the simplified implementation as suggested.
Regards,
Hongling Zeng
在 2026年05月20日 01:35, David Laight 写道:
> On Mon, 11 May 2026 14:18:30 +0800
> Hongling Zeng <zenghongling@kylinos.cn> wrote:
>
>> 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 *")
>> 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.
>> ---
>> ---
>> fs/9p/vfs_inode.c | 5 ++---
>> fs/9p/vfs_inode_dotl.c | 4 ++--
>> 2 files changed, 4 insertions(+), 5 deletions(-)
>>
>> diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
>> index d1508b1fe109..d2d18c796b58 100644
>> --- a/fs/9p/vfs_inode.c
>> +++ b/fs/9p/vfs_inode.c
>> @@ -672,13 +672,12 @@ 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;
>> + int err = 0;
>> 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);
>> @@ -692,7 +691,7 @@ static struct dentry *v9fs_vfs_mkdir(struct mnt_idmap *idmap, struct inode *dir,
>>
>> if (fid)
>> p9_fid_put(fid);
>> - return ERR_PTR(err);
>> + return err ? ERR_PTR(err) : NULL;
>> }
> That function is odd, it ends:
> fid = v9fs_create(v9ses, dir, dentry, NULL, perm, P9_OREAD);
> if (IS_ERR(fid)) {
> err = PTR_ERR(fid);
> fid = NULL;
>
> A 'return' here would solve the mess later on.
>
> } else {
> inc_nlink(dir);
> v9fs_invalidate_inode_attr(dir);
> }
>
> Can v9fs_create() even return NULL?
> In any case p9_fid_put() already contains an (inline) check for both
> NULL and IS_ERR() so the 'if (fid)' and 'fid == NULL' (above) are pointless.
>
> if (fid)
> p9_fid_put(fid);
> return ERR_PTR(err);
>
> -- David
>
>>
>> /**
>> 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] 6+ messages in thread
* Re: [PATCH v1] 9p: Fix mkdir to return NULL on success
2026-05-19 12:21 ` Dominique Martinet
@ 2026-05-20 1:42 ` Hongling Zeng
2026-05-20 1:59 ` Dominique Martinet
0 siblings, 1 reply; 6+ messages in thread
From: Hongling Zeng @ 2026-05-20 1:42 UTC (permalink / raw)
To: Dominique Martinet, Hongling Zeng
Cc: ericvh, lucho, linux_oss, v9fs, linux-kernel
Hi Dominique,
Thanks for picking this up!
I received David feedback suggesting to simplify v9fs_vfs_mkdir()
by removing the 'err' variable and using ERR_CAST(). Since v1 is
already merged, would you still like a v2, or shall I submit it
separately?
Best regards,
Hongling
在 2026年05月19日 20:21, Dominique Martinet 写道:
> Hongling Zeng wrote on Mon, May 11, 2026 at 02:18:30PM +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 *")
>> Acked-by: Christian Schoenebeck <linux_oss@crudebyte.com>
>> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
> Thanks, I've picked this up with s/Fix/make/ in the commit subject, as I
> (subjectively) don't consider this a fix given it's only a style change
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1] 9p: Fix mkdir to return NULL on success
2026-05-20 1:42 ` Hongling Zeng
@ 2026-05-20 1:59 ` Dominique Martinet
0 siblings, 0 replies; 6+ messages in thread
From: Dominique Martinet @ 2026-05-20 1:59 UTC (permalink / raw)
To: Hongling Zeng; +Cc: Hongling Zeng, ericvh, lucho, linux_oss, v9fs, linux-kernel
Hongling Zeng wrote on Wed, May 20, 2026 at 09:42:53AM +0800:
> I received David feedback suggesting to simplify v9fs_vfs_mkdir()
> by removing the 'err' variable and using ERR_CAST(). Since v1 is
> already merged, would you still like a v2, or shall I submit it
> separately?
It's only got as far as my tree, please send a new version
--
Dominique Martinet | Asmadeus
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-05-20 1:59 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-11 6:18 [PATCH v1] 9p: Fix mkdir to return NULL on success Hongling Zeng
2026-05-19 12:21 ` Dominique Martinet
2026-05-20 1:42 ` Hongling Zeng
2026-05-20 1:59 ` Dominique Martinet
2026-05-19 17:35 ` David Laight
2026-05-20 1:24 ` Hongling Zeng
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.