Hi Torleiv,

Thank you for the patch.

In the subject line uvcvido should be written uvcvideo.

On Sun, Apr 28, 2019 at 07:21:13AM +0200, Torleiv Sundre wrote:
> uvcvideo creates a debugfs directory based on the device bus number and
> device number. If a device contains more than one uvc function, the
> creation of the second and following debugfs directories will fail and
> print an info message like this:
>   "uvcvideo: Unable to create debugfs 3-2 directory."
> 
> This patch includes the uvc streaming interface number in the debugfs
> directory name, to make sure it is unique. The directory name format is
> changed from "<busnum>-<devnum>" to "<busnum>-<devnum>-<intfnum>"

Good idea. Please see below for a few comments.

> Signed-off-by: Torleiv Sundre <torl...@huddly.com>
> ---
>  drivers/media/usb/uvc/uvc_debugfs.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_debugfs.c 
> b/drivers/media/usb/uvc/uvc_debugfs.c
> index 77e7c2419b9b..6e244acb80ab 100644
> --- a/drivers/media/usb/uvc/uvc_debugfs.c
> +++ b/drivers/media/usb/uvc/uvc_debugfs.c
> @@ -84,7 +84,8 @@ void uvc_debugfs_init_stream(struct uvc_streaming *stream)
>       if (uvc_debugfs_root_dir == NULL)
>               return;
>  
> -     sprintf(dir_name, "%u-%u", udev->bus->busnum, udev->devnum);
> +     sprintf(dir_name, "%u-%u-%d", udev->bus->busnum, udev->devnum,
> +             stream->intfnum);

intfnum should never be negative, I would thus use %u instead of %d.
Furthermore, the dir_name buffer is 32 bytes long, so if something
really wrongs happen and the three variables end up being very large,
sprintf could overflow (10 chars for each value, 2 for the dashes, 1
for the ending \0). I propose extending dir_name to 33 bytes, and using
snprintf() instead of sprintf() to be on the safe side.

 {
        struct usb_device *udev = stream->dev->udev;
        struct dentry *dent;
-       char dir_name[32];
+       char dir_name[33];

        if (uvc_debugfs_root_dir == NULL)
                return;

-       sprintf(dir_name, "%u-%u", udev->bus->busnum, udev->devnum);
+       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)) {

With those changes,

Reviewed-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com>

If you're fine with the changes there's no need to resubmit, I'll apply
the modified patch.

>  
>       dent = debugfs_create_dir(dir_name, uvc_debugfs_root_dir);
>       if (IS_ERR_OR_NULL(dent)) {

-- 
Regards,

Laurent Pinchart

Reply via email to