On 2013.05.18 10:41, Hans de Goede wrote:
> I disagree, just because we cannot have the perfect API, is not a very
> valid reason to not add support for something, as long as we can do
> so with a good enough API. To quote Torvalds:
> "perfect is the enemy of good".

Except we're not trying for perfect here. We're trying to avoid having 
to duplicate/maintain the same thing twice.

Or maybe you misunderstood what I was saying. What I am proposing is not 
introducing BOS/ep_comp in current, but switch to the next API _now_, 
and introduce it there.

You proposal would mean maintaining the same API twice. I can't really 
call that a "good" solution especially when the better and easier 
solution is to avoid duplicates.

> You make a very valid point here, and since we certainly don't want to
> be changing the ABI every time we add support for a new BOS descriptor
> type, we should design our API in such a way that we don't need too

Yeah. That was something I alluded to about a month ago when I posted 
about the BOS/ep_comp implementation I saw in libusb.

Unrolling the BOS was actually one of the few points where I was trying 
to compromise with the libusb API.

> here is a proposal for that:
>
> struct libusb_bos_dev_capability_descriptor {
>      uint8_t bLength;
>      uint8_t bDescriptorType;
>      uint8_t bDevCapabilityType;
>      uint8_t dev_capability_data[0];
> };
>
> struct libusb_bos_descriptor {
>      uint8_t bLength;
>      uint8_t bDescriptorType;
>      uint16_t wTotalLength;
>      uint8_t bNumDeviceCaps;
>      struct libusb_bos_dev_capability_descriptor *dev_capability[0];
> };
>
> This will allow libusb_get_bos_descriptor to simply always return all
> descriptors.

and be closer to the specs, which is always a good idea.

> For known types like the USB 2.0 Extension descriptor we will then
> offer functions like:
> int libusb_get_usb_2_0_extension_descriptor(struct
> libusb_bos_dev_capability_descriptor *bos_dev_cap, struct
> libusb_usb_2_0_extension_descriptor *usb_2_0_extension);

not exactly sure I like the multiplication of get_bos_whatever (with all 
its limitations, the one good thing the previous proposal has is easy 
access to any of the BOS attributes we unrolled), but I'm not seeing any 
great alternatives that could be future-proof.

The one thing we may want to consider is that some of these 
sub-descriptors may end up having a version number deciding their length 
and/or format (I don't think any of the current ones do, but we're 
trying to future proof the thing), which I believe is what you allude to 
below:

> Or, if they have nested descriptors:
> int libusb_get_foo_descriptor(struct
> libusb_bos_dev_capability_descriptor *bos_dev_cap, struct
> libusb_foo_descriptor **foo);
> void libusb_free_foo_descriptor(struct libusb_foo_descriptor *foo);

and which requires to balance each call with a free as you point out.
But then again, get_desc / free_desc is what we have pretty much for all 
other descriptors already, so it won't be that different from our 
existing model.

> Note the difference in how we handle static-sized versus nested types
> is following the existing pattern of how we do this with device descriptors
> vs config descriptors.

If it were up to me, I'd consider that no descriptor is going to be 
static ever. You never know what surprise the future specs might hold.

> This also fixes the potential issue you mentioned in this comment:
>
>          /* The following assumes that only one descriptor of the following
>           * types will ever be provided by the device for each
> endpoint... */

A limitation of the libusb implementation (not sure if this was in the 
original patch), which I wasn't thrilled about, hence the comment.

> The question really boils down to:
> 1) Not having ss ep desc support in our current ABI version
> 2) Doing an ABI breaking release to add it in an API wise better manner
>     and then quickly having todo an other ABI breaking release to fix
>     all our other API issues
> 3) Having less then ideal, but still very usable ss ep desc support
>     in a manner compatible with our ABI
>
> 1) Is not really desirable given how long patches for this have been
> floating around

Unless we let 1.0 go the way of the dodo, like 0.1 has, and shift our 
development to 2.0/1.1.

> 2) This will mean that rather then maintaining one less then ideal API
> call separately from what it will look in the next ABI breaking release,
> we end up maintaining 3 (*) complete different ABI branches, and
> maintaining an entire branch is a lot more work then maintaining one
> simple function in 2 different forms.

I really don't understand why you think we'd have to maintain 1.0 if we 
bumped API/ABI version tomorrow. Does anyone maintain 0.1 (besides 
compat, which is peanuts and doesn't need a full time maintainer)?

The way I see it, when we bump, we drop _ALL_ work on the previous 
version. That doesn't mean people can no longer use it. It just means 
that if they want fixes, they should upgrade. And there we go back into 
the whole "but it's not fathomable to tell developers that they should 
really upgrade to the latest dependencies if they want the latest stuff 
there", which I genuinely cannot understand.

> 3) So by elimination of 1 and 2, 3 gets my vote

You're eliminating 1 & 2 a bit too hastily if you ask me, on the false 
pretence that switching version takes time (Hello? Remember when we 
forked about one year ago, and started to release very quickly after 
that? How would version switching be any more complicated?), and that we 
somehow _have_ to devote resources to maintain multiple versions at once.

Judging by the number of requests we have seen on libusb and libusbx, 
and unlike hp, nobody appears to be missing ep_comp. I therefore fail to 
see an issue with telling people who want to use a new feature from an 
upgraded USB hardware stack to also use an upgraded USB library, and 
apply very minor modifications to their code as a result.

But more on that in the next mail.

Regards,

/Pete

------------------------------------------------------------------------------
AlienVault Unified Security Management (USM) platform delivers complete
security visibility with the essential security capabilities. Easily and
efficiently configure, manage, and operate all of your security controls
from a single console and one unified framework. Download a free trial.
http://p.sf.net/sfu/alienvault_d2d
_______________________________________________
libusbx-devel mailing list
libusbx-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libusbx-devel

Reply via email to