Hi, On 05/17/2013 03:04 AM, Pete Batard wrote: > Looks OK to me. Just a couple minor issues: > > > o io.c:1952:4: warning: Value stored to 'ret' is never read > ret = LIBUSB_ERROR_OTHER; > ^ ~~~~~~~~~~~~~~~~~~ > Looking at the source, we probably want r = LIBUSB_ERROR_OTHER; there.
Agreed and fixed in my darwin-integration branch > o core.c(751): warning C4100: 'ctx' : unreferenced formal parameter, > which comes from the removal applied in [1]. > > Ideally, we'd probably want to remove the ctx param from the call, since > it was used by the now removed libusb_get_device_list(), but that'd mean > changing the API... Right, which as we learned with the MaxPower is a complete no no, and get_port_path() is already being used in the wild, so we must keep it around in the 1.x.y series for ever (or at least for a very long time). > With no other topology calls taking a context (and > this parameter avoidable in the first place - see below), it definitely > looks like an oddball there. Agreed. > > The lets-not-touch-the-API-at-all option include adding an UNUSED(ctx) > (UNUSED is defined in libusbi.h) or do something like print a warning on > overflow or something, since usb_warn takes a ctx. Printing a warning on overflow is a good idea, but we can use dev->ctx for that just as well, so it does not fix the oddball API issue. > Right now, I'd tend to lean more towards introducing an API change, > especially as the ctx made no sense in the first place: we could have > used dev->ctx all the same, and the blame for not doing that is entirely > with me, since I introduced that code. The way I'd see then would be to > flag get_port_path() as deprecated, after adding an UNUSED() call there, > and introduce something like: > > int API_EXPORTED libusb_get_port_numbers(libusb_device *dev, uint8_t* > port_numbers, uint8_t port_numbers_len) > > that would do the same thing as get_port_path() but without the context. > I think having both get_port_number() and get_port_numbers() would also > be more intuitive for our users. I rather dislike the extra ctx parameter too, so I've implemented the proposed libusb_get_port_numbers and libusb_get_port_path is now a wrapper around it. As said before the intention is to leave libusb_get_port_path around in the 1.0.x series for both ABI and API backward compat reasons. Regards, Hans ------------------------------------------------------------------------------ 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
