[PATCH 1/1] uapi: cdc.h: cleanly provide for more interfaces and countries

[PATCH 1/1] uapi: cdc.h: cleanly provide for more interfaces and countries

[Oliver Neukum]

The spec requires at least one interface respectively country.
It allows multiple ones. This needs to be clearly said in the UAPI.

Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
 include/uapi/linux/usb/cdc.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/usb/cdc.h b/include/uapi/linux/usb/cdc.h
index 1924cf665448..5fcbce0be133 100644
--- a/include/uapi/linux/usb/cdc.h
+++ b/include/uapi/linux/usb/cdc.h
@@ -105,7 +105,7 @@ struct usb_cdc_union_desc {
 
 	__u8	bMasterInterface0;
 	__u8	bSlaveInterface0;
-	/* ... and there could be other slave interfaces */
+	__u8	bSlaveInterfaces[];
 } __attribute__ ((packed));
 
 /* "Country Selection Functional Descriptor" from CDC spec 5.2.3.9 */
@@ -116,7 +116,7 @@ struct usb_cdc_country_functional_desc {
 
 	__u8	iCountryCodeRelDate;
 	__le16	wCountyCode0;
-	/* ... and there can be a lot of country codes */
+	__le16	wCountyCodes[];
 } __attribute__ ((packed));
 
 /* "Network Channel Terminal Functional Descriptor" from CDC spec 5.2.3.11 */
-- 
2.51.1

Re: [PATCH 1/1] uapi: cdc.h: cleanly provide for more interfaces and countries

[Greg KH]

On Tue, Oct 28, 2025 at 01:32:22PM +0100, Oliver Neukum wrote:
> The spec requires at least one interface respectively country.
> It allows multiple ones. This needs to be clearly said in the UAPI.
> 
> Signed-off-by: Oliver Neukum <oneukum@suse.com>
> ---
>  include/uapi/linux/usb/cdc.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/uapi/linux/usb/cdc.h b/include/uapi/linux/usb/cdc.h
> index 1924cf665448..5fcbce0be133 100644
> --- a/include/uapi/linux/usb/cdc.h
> +++ b/include/uapi/linux/usb/cdc.h
> @@ -105,7 +105,7 @@ struct usb_cdc_union_desc {
>  
>  	__u8	bMasterInterface0;
>  	__u8	bSlaveInterface0;
> -	/* ... and there could be other slave interfaces */
> +	__u8	bSlaveInterfaces[];

Can this be combined with bSlaveInterface0?  Feels odd to have 0 and
then "more".

Also, what determines how many, the overall length?

>  } __attribute__ ((packed));
>  
>  /* "Country Selection Functional Descriptor" from CDC spec 5.2.3.9 */
> @@ -116,7 +116,7 @@ struct usb_cdc_country_functional_desc {
>  
>  	__u8	iCountryCodeRelDate;
>  	__le16	wCountyCode0;
> -	/* ... and there can be a lot of country codes */
> +	__le16	wCountyCodes[];

same here.

thanks,

greg k-h

Re: [PATCH 1/1] uapi: cdc.h: cleanly provide for more interfaces and countries

[Oliver Neukum]

Hi,

On 28.10.25 14:32, Greg KH wrote:
> On Tue, Oct 28, 2025 at 01:32:22PM +0100, Oliver Neukum wrote:
>> The spec requires at least one interface respectively country.
>> It allows multiple ones. This needs to be clearly said in the UAPI.
>>
>> Signed-off-by: Oliver Neukum <oneukum@suse.com>
>> ---
>>   include/uapi/linux/usb/cdc.h | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/uapi/linux/usb/cdc.h b/include/uapi/linux/usb/cdc.h
>> index 1924cf665448..5fcbce0be133 100644
>> --- a/include/uapi/linux/usb/cdc.h
>> +++ b/include/uapi/linux/usb/cdc.h
>> @@ -105,7 +105,7 @@ struct usb_cdc_union_desc {
>>   
>>   	__u8	bMasterInterface0;
>>   	__u8	bSlaveInterface0;
>> -	/* ... and there could be other slave interfaces */
>> +	__u8	bSlaveInterfaces[];
> 
> Can this be combined with bSlaveInterface0?  Feels odd to have 0 and
> then "more".

I am afraid the C language does not allow you to specify
that an array must have a minimum length other than zero.
In this case bSlaveInterface0 must be present.
I don't think using only an array would be the right choice.
> Also, what determines how many, the overall length?

bLength - 4 is the number of slave interfaces, which
must at least be 1.
We cannot use counted_by in UAPI, can we?

	Regards
		Oliver

Re: [PATCH 1/1] uapi: cdc.h: cleanly provide for more interfaces and countries

[Greg KH]

On Tue, Oct 28, 2025 at 03:34:40PM +0100, Oliver Neukum wrote:
> Hi,
> 
> On 28.10.25 14:32, Greg KH wrote:
> > On Tue, Oct 28, 2025 at 01:32:22PM +0100, Oliver Neukum wrote:
> > > The spec requires at least one interface respectively country.
> > > It allows multiple ones. This needs to be clearly said in the UAPI.
> > > 
> > > Signed-off-by: Oliver Neukum <oneukum@suse.com>
> > > ---
> > >   include/uapi/linux/usb/cdc.h | 4 ++--
> > >   1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/include/uapi/linux/usb/cdc.h b/include/uapi/linux/usb/cdc.h
> > > index 1924cf665448..5fcbce0be133 100644
> > > --- a/include/uapi/linux/usb/cdc.h
> > > +++ b/include/uapi/linux/usb/cdc.h
> > > @@ -105,7 +105,7 @@ struct usb_cdc_union_desc {
> > >   	__u8	bMasterInterface0;
> > >   	__u8	bSlaveInterface0;
> > > -	/* ... and there could be other slave interfaces */
> > > +	__u8	bSlaveInterfaces[];
> > 
> > Can this be combined with bSlaveInterface0?  Feels odd to have 0 and
> > then "more".
> 
> I am afraid the C language does not allow you to specify
> that an array must have a minimum length other than zero.
> In this case bSlaveInterface0 must be present.
> I don't think using only an array would be the right choice.

bSlaveInterface[1] ?

I'll add Gustavo here, he's done lots of work in this area.

> > Also, what determines how many, the overall length?
> 
> bLength - 4 is the number of slave interfaces, which
> must at least be 1.
> We cannot use counted_by in UAPI, can we?

I don't know, Gustavo?

thanks,

greg k-h

Re: [PATCH 1/1] uapi: cdc.h: cleanly provide for more interfaces and countries

[Gustavo A. R. Silva]

On 10/28/25 14:40, Greg KH wrote:
> On Tue, Oct 28, 2025 at 03:34:40PM +0100, Oliver Neukum wrote:
>> Hi,
>>
>> On 28.10.25 14:32, Greg KH wrote:
>>> On Tue, Oct 28, 2025 at 01:32:22PM +0100, Oliver Neukum wrote:
>>>> The spec requires at least one interface respectively country.
>>>> It allows multiple ones. This needs to be clearly said in the UAPI.
>>>>
>>>> Signed-off-by: Oliver Neukum <oneukum@suse.com>
>>>> ---
>>>>    include/uapi/linux/usb/cdc.h | 4 ++--
>>>>    1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/include/uapi/linux/usb/cdc.h b/include/uapi/linux/usb/cdc.h
>>>> index 1924cf665448..5fcbce0be133 100644
>>>> --- a/include/uapi/linux/usb/cdc.h
>>>> +++ b/include/uapi/linux/usb/cdc.h
>>>> @@ -105,7 +105,7 @@ struct usb_cdc_union_desc {
>>>>    	__u8	bMasterInterface0;
>>>>    	__u8	bSlaveInterface0;
>>>> -	/* ... and there could be other slave interfaces */
>>>> +	__u8	bSlaveInterfaces[];
>>>
>>> Can this be combined with bSlaveInterface0?  Feels odd to have 0 and
>>> then "more".
>>
>> I am afraid the C language does not allow you to specify
>> that an array must have a minimum length other than zero.
>> In this case bSlaveInterface0 must be present.
>> I don't think using only an array would be the right choice.

You can do something like this:

@@ -104,8 +104,10 @@ struct usb_cdc_union_desc {
         __u8    bDescriptorSubType;

         __u8    bMasterInterface0;
-       __u8    bSlaveInterface0;
-       /* ... and there could be other slave interfaces */
+       union {
+               __u8    bSlaveInterface0;
+               DECLARE_FLEX_ARRAY(__u8, bSlaveInterfaces);
+       };
  } __attribute__ ((packed));

Just adjust any related code to account for the fact that the
"other slave interfaces" will start at bSlaveInterfaces[1].

Something like the following is preferable, but this syntax
for flexible-array members in unions is only supported starting
with GCC 15:

--- a/include/uapi/linux/usb/cdc.h
+++ b/include/uapi/linux/usb/cdc.h
@@ -104,8 +104,10 @@ struct usb_cdc_union_desc {
         __u8    bDescriptorSubType;

         __u8    bMasterInterface0;
-       __u8    bSlaveInterface0;
-       /* ... and there could be other slave interfaces */
+       union {
+               __u8    bSlaveInterface0;
+               __u8 bSlaveInterfaces[];
+       };
  } __attribute__ ((packed));

So, we'll have to wait until that becomes the minimum supported
version before we can use the above code.

> 
> bSlaveInterface[1] ?

No, please. One-element arrays when used as flexible arrays are
deprecated. It took us years to transform those into flexible-array
members.

> 
> I'll add Gustavo here, he's done lots of work in this area.
> 
>>> Also, what determines how many, the overall length?
>>
>> bLength - 4 is the number of slave interfaces, which
>> must at least be 1.
>> We cannot use counted_by in UAPI, can we?

Yes. We _can_ (and do) use __counted_by in UAPI.

The problem is that currently GCC doesn't support expressions
in __counted_by. So, something like the following is not possible:

__counted_by(bLength - 4)

-Gustavo

> 
> I don't know, Gustavo?
> 
> thanks,
> 
> greg k-h