Hi Siarhei!
Am 16.03.2016 um 17:58 schrieb Siarhei Siamashka:
Hello Bernhard,
Thanks, this is clearly a useful feature.
[...]
Should we also give more explicit credit in the commit message
to NextThingCo people for their work on the initial implementation
of this patch? Yes, I know that the github tracker link has all
the necessary details, but still we could be a bit nicer.
I have added Wynter Woods to CC.
Agreed. While the functionality has been refactored extensively, the
patch was originally inspired by their work. But we also should
encourage people to contribute additions like this back to the
sunxi-tools repo or the mailing list...
Are we sure that the *handle variable is set to NULL if libusb_open()
call fails? The documentation at
http://libusb.sourceforge.net/api-1.0/group__dev.html#ga8163100afdf933fabed0db7fa81c89d1
says
"handle - output location for the returned device handle
pointer. Only populated when the return code is 0"
Are we sure that the 'errno' has not been overwritten by something
else by now? The http://man7.org/linux/man-pages/man3/errno.3.html
says:
A common mistake is to do
if (somecall() == -1) {
printf("somecall() failed\n");
if (errno == ...) { ... }
}
where errno no longer needs to have the value it had upon return from
somecall() (i.e., it may have been changed by the printf(3)). If the
value of errno should be preserved across a library call, it must be
saved:
if (somecall() == -1) {
int errsv = errno;
printf("somecall() failed\n");
if (errsv == ...) { ... }
}
You're right. I have relocated the "result == NULL" case back right
after the libusb_open_device_with_vid_pid() call where it originated
from, and now handle libusb_get_device_list() and libusb_open() failures
separately.
What if the 'dev_arg' string is malformed and contains only a single
decimal number without the ':' separator? It is probably best to
rewrite this as
if (sscanf(dev_arg, "%u:%u", &busnum, &devnum) != 2) {
fprintf(stderr, "ERROR: Expected 'bus:devnum', got '%s'.\n", dev_arg);
exit(1);
}
With the stroul() I'd expect one or both variables to end up as 0, which
would never match a valid USB bus:devnum combination (so we'd bail out
with a "ERROR: Bus %03d Device %03d not found in libusb device list"
message).
However, your solution is probably cleaner. Would it be safe to use "%u"
on the signed variables? For now I have preferred "%d" instead...
I'll be submitting a v4 shortly, which also addresses various style
issues you pointed out.
Regards, B. Nortmann
--
You received this message because you are subscribed to the Google Groups
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
