| Message ID | 20260520074634.53656-1-zenghongling@kylinos.cn |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ext4: Fix ERR_PTR(0) in ext4_mkdir() | expand |
On Wed 20-05-26 15:46:34, Hongling Zeng wrote: > When mkdir succeeds, ext4_mkdir() returns ERR_PTR(0) which is incorrect. > It should return NULL instead for success and ERR_PTR() only with > negative error codes for failure. > > Fixes: 88d5baf69082 ("Change inode_operations.mkdir to return struct dentry *") > Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn> You're right this is a bit sloppy programming although there's no actual functional difference at this point. So feel free to add: Reviewed-by: Jan Kara <jack@suse.cz> Honza > --- > fs/ext4/namei.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c > index 4a47fbd8dd30..8cadaeb15b2b 100644 > --- a/fs/ext4/namei.c > +++ b/fs/ext4/namei.c > @@ -3054,7 +3054,7 @@ static struct dentry *ext4_mkdir(struct mnt_idmap *idmap, struct inode *dir, > out_retry: > if (err == -ENOSPC && ext4_should_retry_alloc(dir->i_sb, &retries)) > goto retry; > - return ERR_PTR(err); > + return err ? ERR_PTR(err) : NULL; > } > > /* > -- > 2.25.1 >
On 5/20/2026 3:46 PM, Hongling Zeng wrote: > When mkdir succeeds, ext4_mkdir() returns ERR_PTR(0) which is incorrect. > It should return NULL instead for success and ERR_PTR() only with > negative error codes for failure. This point is indeed very easy to overlook. However, why not modify other file systems as well? Commit 88d5baf69082 made changes not only to ext4. Thanks, Yi. > > Fixes: 88d5baf69082 ("Change inode_operations.mkdir to return struct dentry *") > Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn> > --- > fs/ext4/namei.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c > index 4a47fbd8dd30..8cadaeb15b2b 100644 > --- a/fs/ext4/namei.c > +++ b/fs/ext4/namei.c > @@ -3054,7 +3054,7 @@ static struct dentry *ext4_mkdir(struct mnt_idmap *idmap, struct inode *dir, > out_retry: > if (err == -ENOSPC && ext4_should_retry_alloc(dir->i_sb, &retries)) > goto retry; > - return ERR_PTR(err); > + return err ? ERR_PTR(err) : NULL; > } > > /*
Hi , Good point! I've been systematically fixing this across filesystems. Several fixes have already been merged (9p, jfs, orangefs,cachefiles....) Still working on a few more filesystems. Thanks for the suggestion! Best regards, Hongling 在 2026年05月20日 17:19, Zhang Yi 写道: > On 5/20/2026 3:46 PM, Hongling Zeng wrote: >> When mkdir succeeds, ext4_mkdir() returns ERR_PTR(0) which is incorrect. >> It should return NULL instead for success and ERR_PTR() only with >> negative error codes for failure. > This point is indeed very easy to overlook. However, why not modify > other file systems as well? Commit 88d5baf69082 made changes not only > to ext4. > > Thanks, > Yi. > >> Fixes: 88d5baf69082 ("Change inode_operations.mkdir to return struct dentry *") >> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn> >> --- >> fs/ext4/namei.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c >> index 4a47fbd8dd30..8cadaeb15b2b 100644 >> --- a/fs/ext4/namei.c >> +++ b/fs/ext4/namei.c >> @@ -3054,7 +3054,7 @@ static struct dentry *ext4_mkdir(struct mnt_idmap *idmap, struct inode *dir, >> out_retry: >> if (err == -ENOSPC && ext4_should_retry_alloc(dir->i_sb, &retries)) >> goto retry; >> - return ERR_PTR(err); >> + return err ? ERR_PTR(err) : NULL; >> } >> >> /*
On 2026/5/20 15:46, Hongling Zeng wrote: > When mkdir succeeds, ext4_mkdir() returns ERR_PTR(0) which is incorrect. > It should return NULL instead for success and ERR_PTR() only with > negative error codes for failure. > > Fixes: 88d5baf69082 ("Change inode_operations.mkdir to return struct dentry *") > Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn> Looks good. Reviewed-by: Baokun Li <libaokun@linux.alibaba.com> > --- > fs/ext4/namei.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c > index 4a47fbd8dd30..8cadaeb15b2b 100644 > --- a/fs/ext4/namei.c > +++ b/fs/ext4/namei.c > @@ -3054,7 +3054,7 @@ static struct dentry *ext4_mkdir(struct mnt_idmap *idmap, struct inode *dir, > out_retry: > if (err == -ENOSPC && ext4_should_retry_alloc(dir->i_sb, &retries)) > goto retry; > - return ERR_PTR(err); > + return err ? ERR_PTR(err) : NULL; > } > > /*
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c index 4a47fbd8dd30..8cadaeb15b2b 100644 --- a/fs/ext4/namei.c +++ b/fs/ext4/namei.c @@ -3054,7 +3054,7 @@ static struct dentry *ext4_mkdir(struct mnt_idmap *idmap, struct inode *dir, out_retry: if (err == -ENOSPC && ext4_should_retry_alloc(dir->i_sb, &retries)) goto retry; - return ERR_PTR(err); + return err ? ERR_PTR(err) : NULL; } /*
When mkdir succeeds, ext4_mkdir() returns ERR_PTR(0) which is incorrect. It should return NULL instead for success and ERR_PTR() only with negative error codes for failure. Fixes: 88d5baf69082 ("Change inode_operations.mkdir to return struct dentry *") Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn> --- fs/ext4/namei.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)