When mkdir succeeds, ceph_mkdir() sets ret to ERR_PTR(0) which is
incorrect. It should return NULL instead for success.

Fixes: 88d5baf69082 ("Change inode_operations.mkdir to return struct dentry *")
Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
---
 fs/ceph/dir.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
index bac9cfb6b982..ab7f7b7016c6 100644
--- a/fs/ceph/dir.c
+++ b/fs/ceph/dir.c
@@ -1167,7 +1167,7 @@ static struct dentry *ceph_mkdir(struct mnt_idmap *idmap, struct inode *dir,
 	    !req->r_reply_info.head->is_target &&
 	    !req->r_reply_info.head->is_dentry)
 		err = ceph_handle_notrace_create(dir, dentry);
-	ret = ERR_PTR(err);
+	ret = err ? ERR_PTR(err) : NULL;
 out_req:
 	if (!IS_ERR(ret) && req->r_dentry != dentry)
 		/* Some other dentry was spliced in */
-- 
2.25.1

On Wed, 2026-05-20 at 17:54 +0800, Hongling Zeng wrote:
> When mkdir succeeds, ceph_mkdir() sets ret to ERR_PTR(0) which is
> incorrect. It should return NULL instead for success.
> 
> Fixes: 88d5baf69082 ("Change inode_operations.mkdir to return struct dentry *")
> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
> ---
>  fs/ceph/dir.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
> index bac9cfb6b982..ab7f7b7016c6 100644
> --- a/fs/ceph/dir.c
> +++ b/fs/ceph/dir.c
> @@ -1167,7 +1167,7 @@ static struct dentry *ceph_mkdir(struct mnt_idmap *idmap, struct inode *dir,
>  	    !req->r_reply_info.head->is_target &&
>  	    !req->r_reply_info.head->is_dentry)
>  		err = ceph_handle_notrace_create(dir, dentry);
> -	ret = ERR_PTR(err);
> +	ret = err ? ERR_PTR(err) : NULL;
>  out_req:
>  	if (!IS_ERR(ret) && req->r_dentry != dentry)
>  		/* Some other dentry was spliced in */

I think that this modification doesn't make sense. The ERR_PTR(0) returns NULL,
as far as I can understand.

  static inline void * __must_check ERR_PTR(long error)
  {
      return (void *) error;
  }

ERR_PTR(0) evaluates to (void *)0, which is exactly NULL.

Thanks,
Slava.

On Wed, 20 May 2026 18:49:18 +0000
Viacheslav Dubeyko <Slava.Dubeyko@ibm.com> wrote:

> On Wed, 2026-05-20 at 17:54 +0800, Hongling Zeng wrote:
> > When mkdir succeeds, ceph_mkdir() sets ret to ERR_PTR(0) which is
> > incorrect. It should return NULL instead for success.
> > 
> > Fixes: 88d5baf69082 ("Change inode_operations.mkdir to return struct dentry *")
> > Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
> > ---
> >  fs/ceph/dir.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
> > index bac9cfb6b982..ab7f7b7016c6 100644
> > --- a/fs/ceph/dir.c
> > +++ b/fs/ceph/dir.c
> > @@ -1167,7 +1167,7 @@ static struct dentry *ceph_mkdir(struct mnt_idmap *idmap, struct inode *dir,
> >  	    !req->r_reply_info.head->is_target &&
> >  	    !req->r_reply_info.head->is_dentry)
> >  		err = ceph_handle_notrace_create(dir, dentry);
> > -	ret = ERR_PTR(err);
> > +	ret = err ? ERR_PTR(err) : NULL;
> >  out_req:
> >  	if (!IS_ERR(ret) && req->r_dentry != dentry)
> >  		/* Some other dentry was spliced in */  
> 
> I think that this modification doesn't make sense. The ERR_PTR(0) returns NULL,
> as far as I can understand.
> 
>   static inline void * __must_check ERR_PTR(long error)
>   {
>       return (void *) error;
>   }
> 
> ERR_PTR(0) evaluates to (void *)0, which is exactly NULL.

In practise yes, technically no.
NULL is an 'integer constant expression with value 0' cast to 'void *'.
ERR_PTR(0) will be the 'all zero' bit pattern, NULL is an implementation
defined bit pattern.
So '(void *)(1 - 1)' is NULL but '(void *)(x = 0)' isn't.

But I think sparse() is trying to stop you converting a NULL
pointer into 'success' when failure to 'allocate/find' something
would normally be an error.

-- David

> 
> Thanks,
> Slava.

On Wed, 2026-05-20 at 23:41 +0100, David Laight wrote:
> On Wed, 20 May 2026 18:49:18 +0000
> Viacheslav Dubeyko <Slava.Dubeyko@ibm.com> wrote:
> 
> > On Wed, 2026-05-20 at 17:54 +0800, Hongling Zeng wrote:
> > > When mkdir succeeds, ceph_mkdir() sets ret to ERR_PTR(0) which is
> > > incorrect. It should return NULL instead for success.
> > > 
> > > Fixes: 88d5baf69082 ("Change inode_operations.mkdir to return struct dentry *")
> > > Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
> > > ---
> > >  fs/ceph/dir.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
> > > index bac9cfb6b982..ab7f7b7016c6 100644
> > > --- a/fs/ceph/dir.c
> > > +++ b/fs/ceph/dir.c
> > > @@ -1167,7 +1167,7 @@ static struct dentry *ceph_mkdir(struct mnt_idmap *idmap, struct inode *dir,
> > >  	    !req->r_reply_info.head->is_target &&
> > >  	    !req->r_reply_info.head->is_dentry)
> > >  		err = ceph_handle_notrace_create(dir, dentry);
> > > -	ret = ERR_PTR(err);
> > > +	ret = err ? ERR_PTR(err) : NULL;
> > >  out_req:
> > >  	if (!IS_ERR(ret) && req->r_dentry != dentry)
> > >  		/* Some other dentry was spliced in */  
> > 
> > I think that this modification doesn't make sense. The ERR_PTR(0) returns NULL,
> > as far as I can understand.
> > 
> >   static inline void * __must_check ERR_PTR(long error)
> >   {
> >       return (void *) error;
> >   }
> > 
> > ERR_PTR(0) evaluates to (void *)0, which is exactly NULL.
> 
> In practise yes, technically no.
> NULL is an 'integer constant expression with value 0' cast to 'void *'.
> ERR_PTR(0) will be the 'all zero' bit pattern, NULL is an implementation
> defined bit pattern.
> So '(void *)(1 - 1)' is NULL but '(void *)(x = 0)' isn't.
> 
> But I think sparse() is trying to stop you converting a NULL
> pointer into 'success' when failure to 'allocate/find' something
> would normally be an error.
> 
> 

So, do you think that this patch makes sense? I could be wrong with my
conclusion. :)

Thanks,
Slava.