Re: [PATCH 7/7] media: usb: uvc: no need to check return value of debugfs_create functions

From: Laurent Pinchart
Date: Thu Sep 17 2020 - 08:35:08 EST


Hi Greg,

On Wed, Aug 19, 2020 at 02:47:19AM +0300, Laurent Pinchart wrote:
> Hi Greg,
>
> Thank you for the patch.
>
> On Tue, Aug 18, 2020 at 03:36:08PM +0200, Greg Kroah-Hartman wrote:
> > When calling debugfs functions, there is no need to ever check the
> > return value. The function can work or not, but the code logic should
> > never do something different based on this.
>
> Is there no value in warning the user that something went wrong ? Silent
> failures are harder to debug.

Could yous share your opinion about this ?

> > Cc: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> > Cc: Mauro Carvalho Chehab <mchehab@xxxxxxxxxx>
> > Cc: linux-media@xxxxxxxxxxxxxxx
> > Cc: linux-kernel@xxxxxxxxxxxxxxx
> > Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> > ---
> > drivers/media/usb/uvc/uvc_debugfs.c | 20 ++++----------------
> > 1 file changed, 4 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/media/usb/uvc/uvc_debugfs.c b/drivers/media/usb/uvc/uvc_debugfs.c
> > index 2b8af4b54117..1a1258d4ffca 100644
> > --- a/drivers/media/usb/uvc/uvc_debugfs.c
> > +++ b/drivers/media/usb/uvc/uvc_debugfs.c
> > @@ -73,7 +73,6 @@ static struct dentry *uvc_debugfs_root_dir;
> > void uvc_debugfs_init_stream(struct uvc_streaming *stream)
> > {
> > struct usb_device *udev = stream->dev->udev;
> > - struct dentry *dent;
> > char dir_name[33];
> >
> > if (uvc_debugfs_root_dir == NULL)
> > @@ -82,22 +81,11 @@ void uvc_debugfs_init_stream(struct uvc_streaming *stream)
> > snprintf(dir_name, sizeof(dir_name), "%u-%u-%u", udev->bus->busnum,
> > udev->devnum, stream->intfnum);
> >
> > - dent = debugfs_create_dir(dir_name, uvc_debugfs_root_dir);
> > - if (IS_ERR_OR_NULL(dent)) {
> > - uvc_printk(KERN_INFO, "Unable to create debugfs %s "
> > - "directory.\n", dir_name);
> > - return;
> > - }
> > -
> > - stream->debugfs_dir = dent;
> > + stream->debugfs_dir = debugfs_create_dir(dir_name,
> > + uvc_debugfs_root_dir);
> >
> > - dent = debugfs_create_file("stats", 0444, stream->debugfs_dir,
> > - stream, &uvc_debugfs_stats_fops);
> > - if (IS_ERR_OR_NULL(dent)) {
> > - uvc_printk(KERN_INFO, "Unable to create debugfs stats file.\n");
> > - uvc_debugfs_cleanup_stream(stream);
> > - return;
> > - }
> > + debugfs_create_file("stats", 0444, stream->debugfs_dir, stream,
> > + &uvc_debugfs_stats_fops);
> > }
> >
> > void uvc_debugfs_cleanup_stream(struct uvc_streaming *stream)

--
Regards,

Laurent Pinchart