On Tue, Mar 29, 2022 at 07:00:49PM +0200, Fabio M. De Francesco wrote: > On martedì 22 marzo 2022 09:38:58 CEST Fabio M. De Francesco wrote: > > debugfs_create_dir() returns a pointers to a dentry objects. On failures > > it returns errors. Currently the values returned to visornic_probe() > > seem to be tested for being equal to NULL in case of failures. > > > > Properly test with "if (IS_ERR())" and then assign the correct error > > value to the "err" variable. > > > > Signed-off-by: Fabio M. De Francesco <fmdefrancesco@xxxxxxxxx> > > --- > > drivers/staging/unisys/visornic/visornic_main.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/staging/unisys/visornic/visornic_main.c b/drivers/staging/unisys/visornic/visornic_main.c > > index 643432458105..58d03f3d3173 100644 > > --- a/drivers/staging/unisys/visornic/visornic_main.c > > +++ b/drivers/staging/unisys/visornic/visornic_main.c > > @@ -1922,11 +1922,11 @@ static int visornic_probe(struct visor_device *dev) > > /* create debug/sysfs directories */ > > devdata->eth_debugfs_dir = debugfs_create_dir(netdev->name, > > visornic_debugfs_dir); > > - if (!devdata->eth_debugfs_dir) { > > + if (IS_ERR(devdata->eth_debugfs_dir)) { > > dev_err(&dev->device, > > "%s debugfs_create_dir %s failed\n", > > __func__, netdev->name); > > - err = -ENOMEM; > > + err = PTR_ERR(devdata->eth_debugfs_dir); > > goto cleanup_register_netdev; > > } > > > > -- > > 2.34.1 > > > Hi Greg, Dan, > > Now I have time to rework this patch but, if I'm not misunderstanding, you > asked for two contrasting works to do here... > > Dan wrote that "[in] this case you can delete the whole devdata->eth_debugfs_dir > and the related code.". > > Greg wrote that "We really shouldn't be checking this value at all. There's > no reason to check the return value of a debugfs_* call. Can you fix up the code to > do that instead?". > > I'm confused because they look like two incompatible requests. What should I do? Greg is saying delete the tests and the error handling. But I am saying delete the tests, the error handling, the devdata->eth_debugfs_dir related code and the call to debugfs_create_dir(). There is no conflict. Delete it all. regards, dan carpenter