On 2012.09.11 09:25, Hans de Goede wrote: >> 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 ...).
Well, actually, my issue with that is precisely that Apple seems to have removed some of the detach functionality we were relying on for HID. But more about this below. > 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 ... Then I guess the question becomes: should we add a new API even if we expect at least one for the platforms we support never to have any use for it? As far as I'm concerned, I don't see detach/detach_and_claim as something that's ever gonna be practical on Windows. Theoretically, it's doable yes, but the drawbacks (size of files to be embedded, inconsistencies with regards to system policies, switch times) are unlikely to make that option viable. So from where I stand, detach looks like a NO_GO on Windows, and we have to decide if we want to add a global API even when we don't envision it being useful on at least one (major) platform. Which brings us back to to the changing of a lightbulb question: How many supported platforms does it take for libusbx to warrant a common API call? > What you're inventing here feels a lot like > the venerable unix ioctl(), Thanks! That's *exactly* what I've been going for. I should have realized it some time ago... Now that it is spelt out, I'm even more in favour of introducing a libusb_ioctl() call, with this exact name. It would make exceedingly explicit for our users to figure out what it's all about. > and in recent years for new kernel API-s > people have very deliberately being steering away from ioctl, because > it has very poor semantics. I don't remember seeing much of that happening on Windows, despite Microsoft never shying away from breaking a working model at every opportunity they have to introduce one they think is better. I also would add that if ioctl was that broken, it's unlikely it would have stood the test of time, and that is something that the new kernel APIs might still need to be tested against. For instance, what mechanisms do they use to account for evolution, eg. when there's a requirement to add a new parameter to a call? I already mentioned the _EX/_V2 approach on Windows, which I have reservations about, and I would say that, as long as you're targeting C, you're going to be pretty limited... Doesn't mean that I'm not curious finding out how they skirt around good old ioctl, but if the idea is to just introduce plain API calls, I'd advise them to have a look at the evolution of MS APIs to get an idea of what it entails, and especially the infamous _EX_V2() creep. > 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). I strongly doubt we won't be able to work around varargs, if that's what's would ideally need for an OS call. IIRC, Michael was able to do just that when he added support for MSVC6, and the only varargs we've ever had some use for has been for logging. Unlikely that this will change. Relying on a header that specifies the size of what's to follow, as well as casts or zero sized arrays (which can be a bit of pain when dealing with MS compilers btw) seems to be common practice. > Another big advantage of having separate functions for each platform > specific op, besides simplicity and type checking I'd say if we want strong type checking, we should probably not be writing a library in C... But the biggest drawback I see with separate OS specific function is that the OS holds a lot less promise of compatibility from one version to the next than what we get from the USB specs, and this is really where I come from with regards to my proposal: We've already see Apple break OS detach capability for HID, we've seen Linux switch away from devfs for udev, and move away from usbfs, we've seen Windows switch from Config Manager to SetupAPI (and "forget" to provide useful parent/child API calls in the process), so I can say with much certainty that some of the OS calls we'd be likely to introduce standalone API for *will* become obsolete one way or another, due to OS evolution. In the best case scenario then, we'll be left with a useless empty shell, that needs to be carried until the next major version. But in the worst case scenario, we may have to implement workarounds or provide extra functionality, as public APIs are pretty much a promise to the user that, at least on some platforms, the call will do what it states. Seeing strong precedents on all the major OSes for ditching functionality that might be very relevant to OS-specific call we may have introduced in libusbx, I can't help but want to advocate for some kind of safety net and avoidance for _v2/_ex calls. Thus my proposal, with a single libusb_ioctl, is to make sure that we explicitly state that those OS IOCTLs are provided AS IS, with no guarantee of being carried in the same form, or at all, from one version of to the next, as well as ensuring that should we need to alter a call, we can do so without obvious breakage. IMO, this right to err or to alter is a lot more difficult to achieve with standalone and very public API calls. > 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. Which I personally see better as it'll ensure we have all the OS calls documented in one single place: libusb_ioctl. If we go with single APIs, they can be spread all over the place. But I tend to find this whole point a bit moot, as I think that, as far as the doc is concerned, there's not that much difference between the 2 approaches. > And writing docs is not only good for > the docs, but also makes you think more about the API design. IMO, that's a fairly weak point. There's one level of indirection between ioctl and standalone API and that's pretty much it. Plus, as stated above, ioctl gives you more leeway to get it right on subsequent iterations (which, again, I don't see more chance of implementing wrong either way), whereas with an API call, if you realize that you could have used that oh-so-convenient extra parameter you didn't realize you'd need until after users started to show real-life scenarios, or the OS manufacturer started to do change APIs differently, it will become awfully apparent... > 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@ So, now I need to figure out a libusb.h.in -> libusb.h for MSVC (DDK and VS2010 have no autotools)... I'm still listening, but I'm already seeing an extra burden, and additional points of failure for Windows, which I could really do without. > 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're already doing some of that in xusb for detach kernel. https://github.com/libusbx/libusbx/blob/master/examples/xusb.c#L840 I also suspect we would want to go with detach_and_claim in xusb once we have it. > 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. Well, if we go with that I'd see an either^or there: Either we go with requiring #ifdefs and don't have to return NOT_SUPPORTED from the function only being listed in the API for the platforms it actually applies, or no #ifdef, and the app is required to handle NOT_SUPPORTED being returned. Now, seeing that: - I wouldn't mind getting some idea where we want to go with regards to OS specific calls before we introduce the detach_and_claim() - I don't have a major issue introducing the current patch as proposed, since we already have detach for all platforms, and an exception to a future and currently imprecise rule for OS specific call isn't going to harm us much - I kind of agree that these would be nice to have in 1.0.13 rather than to 1.0.14 - Time-wise, I could use some delaying of the RC and release by a few days (no more than a week though), especially as, as expected, some last minutes patches are being rushed in (and I'm also guilty as charged on that) ...I'm planning to wait a few more days to go to RC (and release), till we can come to some form of agreement with regards to what we want to do with regards to this specific patch. And I'm also hoping we can get a better idea of how we want to handle OS specific calls as a result. Regards, /Pete ------------------------------------------------------------------------------ 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
