Re: [PATCH] usb: video: fix descriptor structures macros

[Sylvain Pelissier <sylvain.pelissier@gmail.com>]

On Thu, 28 Jan 2021 at 09:22, Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Thu, Jan 28, 2021 at 09:12:59AM +0100, Sylvain Pelissier wrote:
> > The macros defining structures for descriptors use token
> > pasting incorrectly. For example, the macro
> > UVC_EXTENSION_UNIT_DESCRIPTOR(1, 2) defines a structure named
> > uvc_extension_unit_descriptor_p_2 which should be
> > uvc_extension_unit_descriptor_1_2 instead.
> >
> > Signed-off-by: Sylvain Pelissier <sylvain.pelissier@gmail.com>
> > ---
> >  include/uapi/linux/usb/video.h | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/uapi/linux/usb/video.h b/include/uapi/linux/usb/video.h
> > index d854cb19c42c..1eb982df87a0 100644
> > --- a/include/uapi/linux/usb/video.h
> > +++ b/include/uapi/linux/usb/video.h
> > @@ -324,7 +324,7 @@ struct uvc_extension_unit_descriptor {
> >  #define UVC_DT_EXTENSION_UNIT_SIZE(p, n)             (24+(p)+(n))
> >
> >  #define UVC_EXTENSION_UNIT_DESCRIPTOR(p, n) \
> > -     uvc_extension_unit_descriptor_##p_##n
> > +     uvc_extension_unit_descriptor_##p##_##n
> >
> >  #define DECLARE_UVC_EXTENSION_UNIT_DESCRIPTOR(p, n)  \
> >  struct UVC_EXTENSION_UNIT_DESCRIPTOR(p, n) {         \
> > @@ -371,7 +371,7 @@ struct uvc_input_header_descriptor {
> >  #define UVC_DT_INPUT_HEADER_SIZE(n, p)                       (13+(n*p))
> >
> >  #define UVC_INPUT_HEADER_DESCRIPTOR(n, p) \
> > -     uvc_input_header_descriptor_##n_##p
> > +     uvc_input_header_descriptor_##n##_##p
> >
> >  #define DECLARE_UVC_INPUT_HEADER_DESCRIPTOR(n, p)    \
> >  struct UVC_INPUT_HEADER_DESCRIPTOR(n, p) {           \
> > @@ -406,7 +406,7 @@ struct uvc_output_header_descriptor {
> >  #define UVC_DT_OUTPUT_HEADER_SIZE(n, p)                      (9+(n*p))
> >
> >  #define UVC_OUTPUT_HEADER_DESCRIPTOR(n, p) \
> > -     uvc_output_header_descriptor_##n_##p
> > +     uvc_output_header_descriptor_##n##_##p
> >
> >  #define DECLARE_UVC_OUTPUT_HEADER_DESCRIPTOR(n, p)   \
> >  struct UVC_OUTPUT_HEADER_DESCRIPTOR(n, p) {          \
> > --
> > 2.25.1
> >
>
> And no one noticed this?  If not, then why do we have these defines at
> all, as obviously they are not used :)

The problem is not detected if you have descriptors with the second
argument different each time since the structure name will change. It
is maybe why it was not noticed. Nevertheless, the extension unit, the
input header and the output header descriptors are part of the USB
video class specification and are often used by devices. We have
detected the problem while implementing a usb gadget having such
descriptors.

>
> thanks,
>
> greg k-h