media: uvcvido: Include streaming interface number in debugfs dir name
Commit Message
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>"
Signed-off-by: Torleiv Sundre <torleiv@huddly.com>
---
drivers/media/usb/uvc/uvc_debugfs.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
Comments
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 <torleiv@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.pinchart@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)) {
@@ -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);
dent = debugfs_create_dir(dir_name, uvc_debugfs_root_dir);
if (IS_ERR_OR_NULL(dent)) {