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.
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... 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.
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.
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.
As to the other aspect of [1], while I'm still trying to remember what
was the platform that warranted having a list, my recollection right now
is that it wasn't Windows. As a matter of fact, my testing on Windows
show no adverse behaviour to that removal, so I think this patch should
indeed be OK, once we fix the ctx issue above.
Apart from that, while I'm still planning to do more Windows testing,
things look in good shape. I ran the MSVC and Clang analyzers on the new
code, and didn't pick anything else.
Regards,
/Pete
[1]
https://github.com/jwrdegoede/libusbx/commit/6391d86e818bf4ed63842cad278c2fb2572dc5d8
------------------------------------------------------------------------------
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
[email protected]
https://lists.sourceforge.net/lists/listinfo/libusbx-devel
