On 2012.09.12 10:51, Hans de Goede wrote:
>> 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?
>
> Yes, I want libusbx to enable app writers to do whatever they envision
> and their chosen platform makes possible, rather then offer some sort of
> lowest common denominator approach.

Should have phrased my question better. This was about deciding when a 
call should become a global API one vs keeping it platform specific 
(either through #ifdef or ioctl). None of us wants to remove useful 
features, even if they apply to one platform.

> Hmm, actually having a libusb_ioctl could work out nicely for platform
> specific stuff. It feels a bit low-level-ish, where as in general we
> try to offer a higher-level API. But then again these bits tend to be
> pretty low-level stuff.

That's how I see them too right now, at least from the ones we 
identified so far. I expect future calls to more or less fit that model 
too, but that's only a guess.

> Sometimes a V2 ioctl gets introduced under Linux.

No problem with that, and I expect us to have to do that as well 
eventually. But at least, it's a parameter rather than an API call.

> I like what the v4l2
> Linux subsystem does there, as you probably know, the way Linux ioctls
> work is they take:
>
> Argument 1: a fd, would be a libusb_device_handle in our case
> Argument 2: an int indicating which operation to do
> Argument 3: a long giving the arguments to the operation, generally used to
>   pass a void *, so we should use a void *, as sizeof(long) != sizeof
> (void *)
>   on some non Linux platforms.

Yeah, avoid the long. We could use a uint_ptr (part of stdint.h and 
aimed at being cast to a pointer, regardless of 32/64). I think I'd tend 
to prefer using a void*, but either way is fine.

MS does things a bit differently [1] (even if you ignore the 
OVERLAPPED), with different buffers for in and out, but I see that as a 
bit of an overkill when 3 arguments as proposed above should do just fine.

> So we could add the following function to libusb:
> int libusb_ioctl(libusb_device_handle *dev, int request, void *arguments);

Works for me. Once we have a void*, we should have enough flexibility to 
accommodate whatever.

> Note that some tricks are needed when adding pointers,
> to make sure that the struct size stays the same when
> adding extra fields,

This is where I'd go with a v2 for the request define, actually. What I 
would prefer to avoid are _v2() API calls, but v2 ioctl requests and 
structs are fine by me. I may sound like a fine line (and maybe it is), 
but I just don't think some_api_call_v2() makes the public API look well 
thought through, even if we have little to do about their cause, 
compared to #define SOME_IOCTL_OP_V2 / struct some_ioctl_struct_v2, 
would remain located around a single ioctl call.

> So the ioctl method requires slightly more lines, but I believe it is
> acceptable.

You seem to go further than what I had in mind.

Basically, I was thinking about keeping the ability to alter ioctls when 
they are young and not that widely in use, as we start to get some user 
feedback.
Once these ioctls start to be commonly used, and despite our "NO 
WARRANTY" notice, we of course want to thread very carefully in terms of 
modifications, and use _v2s where it can avoid inconveniencing too many 
users.

> Right, I'm fine with removing ioctl's at our leisure, and simply make
> libusb_ioctl return LIBUSB_ERROR_NOT_SUPPORTED for unknown requests, but we
> must make sure we never re-use request numbers during an ABI stable series.

Sensible. I'd want that too.

> I propose that we first make a decision on how to move forward with
> platform specific calls. If we go for libusb_ioctl, then I would like to
> start with ioctls for:
>
> kernel_driver_active
> detach_kernel_driver
> attach_kernel_driver
> detach_kernel_driver_and_claim

Sounds fair to me. And since we use detach in xusb, it'll give us the 
ability to test/demonstrate the new API with a sample. Plus the calls 
you list it should give us a good idea of what we'll be in for.

> This however is too much work for 1.0.13 IMHO, as you've guessed I'm
> leaning towards going with the ioctl implementation.

Same here.

Ludovic seemed to have a preference for adding straight API calls, 
which, despite my preference, I can understand if we don't expect to 
have to introduce that many calls (KISS). I'm counting 4-6 new API calls 
likely to be introduced in the semi-immediate future if we go that route 
though, with more to come...

> In that case I vote for
> delaying
> the libusb_detach_kernel_driver_and_claim patches to 1.0.14, and
> implement that
> functionality through an ioctl right away.

If we go with ioctl, I'd prefer delaying their introduction to 1.0.14 too.

> If we decide to go the ioctl way I'll make some time to work on this in
> a couple of weeks, or you can beat me to it :)

Extremely unlikely for me at this stage. Plus I'm happy with what you 
seem to have in mind, even if you decide to simplify/alter it a bit. So, 
provided nobody objects, please knock yourself out! :)

Regards,

/Pete

[1] 
http://msdn.microsoft.com/en-us/library/windows/desktop/aa363216%28v=vs.85%29.aspx


------------------------------------------------------------------------------
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