Re: [PATCH v1] drivers: block: Updates return value check
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
Reply to: