Re: [PATCH v1] drivers: block: Updates return value check

[Atul Kumar Pant <atulpant.linux@gmail.com>]

On Sun, Aug 06, 2023 at 03:36:18PM +0200, Greg KH wrote:
> On Sun, Aug 06, 2023 at 05:53:51PM +0530, Atul Kumar Pant wrote:
> > Updating the check of return value from debugfs_create_dir
> > to use IS_ERR.
> 
> Why?
> 
> > 
> > Signed-off-by: Atul Kumar Pant <atulpant.linux@gmail.com>
> > ---
> >  drivers/block/nbd.c     | 4 ++--
> >  drivers/block/pktcdvd.c | 2 +-
> >  2 files changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
> > index 9c35c958f2c8..65ecde3e2a5b 100644
> > --- a/drivers/block/nbd.c
> > +++ b/drivers/block/nbd.c
> > @@ -1666,7 +1666,7 @@ static int nbd_dev_dbg_init(struct nbd_device *nbd)
> >  		return -EIO;
> >  
> >  	dir = debugfs_create_dir(nbd_name(nbd), nbd_dbg_dir);
> > -	if (!dir) {
> > +	if (IS_ERR(dir)) {
> >  		dev_err(nbd_to_dev(nbd), "Failed to create debugfs dir for '%s'\n",
> >  			nbd_name(nbd));
> >  		return -EIO;
> 
> This isn't correct, sorry.  Please do not make this change.
> 
> > @@ -1692,7 +1692,7 @@ static int nbd_dbg_init(void)
> >  	struct dentry *dbg_dir;
> >  
> >  	dbg_dir = debugfs_create_dir("nbd", NULL);
> > -	if (!dbg_dir)
> > +	if (IS_ERR(dbg_dir))
> >  		return -EIO;
> 
> Again, not corrct.
> 
> >  	nbd_dbg_dir = dbg_dir;
> > diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c
> > index d5d7884cedd4..69e5a100b3cf 100644
> > --- a/drivers/block/pktcdvd.c
> > +++ b/drivers/block/pktcdvd.c
> > @@ -451,7 +451,7 @@ static void pkt_debugfs_dev_new(struct pktcdvd_device *pd)
> >  	if (!pkt_debugfs_root)
> >  		return;
> >  	pd->dfs_d_root = debugfs_create_dir(pd->name, pkt_debugfs_root);
> > -	if (!pd->dfs_d_root)
> > +	if (IS_ERR(pd->dfs_d_root))
> >  		return;
> 
> Also not correct.
> 
> Why check the return value at all?  As this check has always been wrong,
> why are you wanting to keep it?

    I'll check the code again. I was not aware that this check is wrong,
    so just tried to fix this based on return value of
    debugfs_create_dir.

> 
> Also, you never responded to our previous review comments, why not?  To
> ignore people is not generally considered a good idea :(

    I might have missed seeing your comments hence I did not reply back.
    Please accept my sincere apologies for this.
    I have one confusion though, regarding the comments that you are
    referring to. Are you mentioning about this patch? Re: [PATCH v5] selftests: rtc: Improve rtctest error handling
    Here I got the following response from your email bot -
    Patch submitter, please ignore Markus's suggestion; you do not need to follow it at all.

    Maybe I misunderstood this comment and hence did not reply/do
    anything in response to Markus's comments.
    If you were referring to some other patch then if possible, can you please tell me the
    suject of the patch? I will reply to your comments and will make the
    fixes accordingly.

> 
> greg k-h