[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: [Nbd] nbd: Add debugfs entries



On Wed, Aug 19, 2015 at 08:33:49AM +0200, Markus Pargmann wrote:
> Hi,
> 
> On Tue, Aug 18, 2015 at 10:46:21PM +0300, Dan Carpenter wrote:
> > Hello Markus Pargmann,
> > 
> > The patch 30d53d9c11b6: "nbd: Add debugfs entries" from Aug 17, 2015,
> > leads to the following static checker warning:
> > 
> > 	drivers/block/nbd.c:883 nbd_dev_dbg_init()
> > 	warn: passing zero to 'PTR_ERR'
> > 
> > drivers/block/nbd.c
> >    875  static int nbd_dev_dbg_init(struct nbd_device *nbd)
> >    876  {
> >    877          struct dentry *dir;
> >    878          struct dentry *f;
> >    879  
> >    880          dir = debugfs_create_dir(nbd_name(nbd), nbd_dbg_dir);
> >    881          if (IS_ERR_OR_NULL(dir)) {
> >    882                  dev_err(nbd_to_dev(nbd), "Failed to create debugfs dir for '%s' (%ld)\n",
> >    883                          nbd_name(nbd), PTR_ERR(dir));
> >    884                  return PTR_ERR(dir);
> > 
> > The static checker warning is that if "dir" is NULL then we return
> > success.

I just tried to produce the same warnings with smatch, sparse and
coccicheck but none of them gave me this warning.
What are you using?

Best regards,

Markus

> > 
> > Debugfs is designed so that we don't need to check for errors.  If
> > debugfs_create_dir() returns an error pointer that means it isn't
> > configured in.  The other debugfs functions will be stubbed out so
> > passing an error pointer to them is harmless.  With this code we print a
> > lot of warning messages for people who don't enable debugfs in their
> > .config.  If debugfs is configured but debugfs_create_dir() fails, it
> > returns NULL.  The other debugfs functions are designed to not care, if
> > the dir is NULL then they just put the files in the base directory
> > instead.
> > 
> > My suggestion is that you use debugfs as designed (no error handling)
> > but the other option would be to fix these error messages and return
> > values so even though it doesn't work as designed, it at least works.
> 
> Thanks for spotting this, seems I forgot to run the static checker :-/.
> 
> It works at the moment. If debugfs is disabled empty functions are used.
> 
> I will probably fix it by checking the dir create for NULL. It wouldn't
> be nice to have the files everywhere in debugfs in case of an error. But
> I can remove the rest of the error handling here which then should
> remove these warnings.
> 
> Thanks,
> 
> Markus
> 
> > 
> >    885          }
> >    886          nbd->dbg_dir = dir;
> > 
> > drivers/block/nbd.c:884 nbd_dev_dbg_init() warn: passing zero to 'PTR_ERR'
> > drivers/block/nbd.c:891 nbd_dev_dbg_init() warn: passing zero to 'PTR_ERR'
> > drivers/block/nbd.c:892 nbd_dev_dbg_init() warn: passing zero to 'PTR_ERR'
> > drivers/block/nbd.c:898 nbd_dev_dbg_init() warn: passing zero to 'PTR_ERR'
> > drivers/block/nbd.c:899 nbd_dev_dbg_init() warn: passing zero to 'PTR_ERR'
> > drivers/block/nbd.c:905 nbd_dev_dbg_init() warn: passing zero to 'PTR_ERR'
> > drivers/block/nbd.c:906 nbd_dev_dbg_init() warn: passing zero to 'PTR_ERR'
> > drivers/block/nbd.c:912 nbd_dev_dbg_init() warn: passing zero to 'PTR_ERR'
> > drivers/block/nbd.c:913 nbd_dev_dbg_init() warn: passing zero to 'PTR_ERR'
> > drivers/block/nbd.c:919 nbd_dev_dbg_init() warn: passing zero to 'PTR_ERR'
> > drivers/block/nbd.c:920 nbd_dev_dbg_init() warn: passing zero to 'PTR_ERR'
> > 
> > regards,
> > dan carpenter
> > 
> 
> -- 
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |



> ------------------------------------------------------------------------------

> _______________________________________________
> Nbd-general mailing list
> Nbd-general@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/nbd-general


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Attachment: signature.asc
Description: Digital signature


Reply to: