Hi,

On 09/11/2012 12:51 AM, Pete Batard wrote:
> On 2012.09.10 08:56, Hans de Goede wrote:
>> Note this set does not yet hook up the new kernel ioctl, I would like to
>> wait with that until it hits Linus' tree
>
> and
>
>> I know it is a bit late, but if possible I would like to get this in for
>> v1.0.13
>
> So this is really 1/3 and 2/3, with 3/3 ("A follow up patch will add use
> of the new ioctl") to be produced later, right?

Right.

> I don't have an issue adding these 2 for 1.0.13 if others want it, but
> somehow I feel this is exactly the kind of OS specific calls we want to
> avoid adding an extra public API for (since, if the kernel ever
> implements something that obsoletes it, we'll have to publicly carry
> that call forever). Instead, I see it better suited into a new single &
> generic libusb_perform_os_specific_op() or something, that would take OS
> specific actions/parameters, but through an API generic that all OSes
> can have some use for. I.e. something along the lines of what I proposed
> previously (though thinking about it some more, I think an union of
> single op's would be better suited than a struct of all of them).
>
> Do we expect any other platforms, besides Linux to ever need
> libusb_detach_kernel_driver_and_claim()?

Yes actually, I do :) others platforms already have a claim implementation,
and I would hope / expect that one day we'll support detaching on more
platforms, as we do need it on more platforms. I know Mac OS X has a detach
like functionality (which I've to admit usually does not work, because
drivers can refuse to be detached, and most do ...).

> Granted, we already have libusb_detach_kernel_driver(), so yeah, it's a
> special case, but from the comments, the call looks awfully OS specific
> to me

As said before, others have claim, others may get detach and I expect them
to hit the same race-like issues too once they get detach ...

> Thus, I would prefer making it less conspicuous, and at the same time
> try to sort a once and for all approach for OS specific calls.

I've not looked into your proposal much (-ENOTIME), but that was talking
about properties, where as now you're talking about operations... And I
must say I'm against having some sort of generic
libusb_perform_os_specific_op() function, that will just turn out to
be some function call multiplexer, with a vararg argument list so
no type checking, etc. What you're inventing here feels a lot like
the venerable unix ioctl(), and in recent years for new kernel API-s
people have very deliberately being steering away from ioctl, because
it has very poor semantics.

More over this just adds a needless abstraction layer between the app and
the actual call which can only introduce bugs (esp. with varargs or another
way to pass not known in advance amount of arguments with unknown types).

Another big advantage of having separate functions for each platform
specific op, besides simplicity and type checking, is documentation, just
the presence in libusb.h alone is worthwhile documentation, and if we
make each platform specific op a separate function, we also force ourselves
to write a doxygen comment for it. And writing docs is not only good for
the docs, but also makes you think more about the API design.

So counter proposal:
1) We add something like the following to libusb.h:

#define LIBUSB_PLATFORM_LINUX   1
#define LIBUSB_PLATFORM_WINDOWS 2
#define LIBUSB_PLATFORM_DARWIN  3
#define LIBUSB_PLATFORM_BSD     4

#define LIBUSB_PLATFORM @LIBUSB_PLATFORM@

And move libusb.h to libusb.h.in and let configure produce libusb.h during
the build filling in @LIBUSB_PLATFORM@

2) If we add something which we expect not to be able to implement on all
platforms, we only include the function in the API on select platforms,
and clearly document the list of platforms where it is available
(so a whitelist). Apps which want to use such functions then must do
so in the following manner:

#if LIBUSB_PLATFORM == LIBUSB_PLATFORM_LINUX || LIBUSB_PLATFORM == 
LIBUSB_PLATFORM_BSD
        libusb_special_call_foo()
#endif

We could even retro-actively apply this to existing platform specific
functions, at least at the documentation level, we should still build
in empty stubs returning LIBUSB_ERROR_NOT_SUPPORTED for ABI compatibility.

Regards,

Hans

------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
libusbx-devel mailing list
libusbx-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libusbx-devel

Reply via email to