Hi. I comment beween quotes. :)
On 17/04/16 09:26, Christian PERRIER wrote:
@@ -97,22 +96,28 @@ garmin_open ( garmin_unit * garmin )
if ( err ) {
printf("libusb_open failed: %s\n",libusb_error_name(err));
garmin->usb.handle = NULL;
- } else if ( garmin->verbose != 0 ) {
- printf("[garmin] libusb_open = %p\n",garmin->usb.handle);
+ } else {
+ if ( garmin->verbose != 0 ) {
+ printf("[garmin] libusb_open = %p\n",garmin->usb.handle);
+ }
I'm not really skilled in C, but isn't that just cosmetic?
No, it isn't just cosmetic. The problem is that structure is a bit
ugly (I agree). Original makes:
if (error) {
[...]
} else if (only_execute_when_user_wants_verbosity) {
print message
err = execute_core_function_libusb
if (err) {
[...]
} else if (only_execute_when_user_wants_verbosity) {
[...]
}
}
In that situation, the function (execute_core_function_libusb) of
libusb library only are executed when the user runs the program with -v
main parameter (wants verbosity).
You could safe the {} of the if (verbosity] condition as it is just
contain one sentence. Make something like:
if (error) {
} else {
if (verbosity) print message
err = execute_core_function_libusb
if (err) {
[...]
} else {
if (verbosity) print message
err = execute_core_function_libusb
}
}
But I prefer mark the block of code with {} for readbility,
specially in that situation.
I'm not skilled C programmer, too (I used loooong time ago :)), but
I probably didn't make that structure. I'd just make:
If (error) {
[...]
} else {
if (verbosity) print message
err = execute_core_function_1_libusb
}
If (not error) {
if (verbosity) print message
err = execute_core_function__n_libusb
}
Probably is better for readbility, but it is less efficient. So, I
preferred to maintain the original structure and make a "quick fix".
- err = libusb_set_configuration(garmin->usb.handle,1);
+ err = libusb_set_configuration(garmin->usb.handle,1);
Ditto
if ( err ) {
- printf("libusb_set_configuration failed: %s\n",
+ printf("libusb_set_configuration failed: %s\n",
Ditto
And so on....
I'd be happy to apply the patch, of course, but do you think that it
can be "cleaned"?
I can try to do it myself but......I'm a bit afraid to break things
doing so.
I agree. Of course can be cleaned. :)
I can clean the non-functional cosmetic (unintentional from me)
changes of the patch: lines, blanks...
Maybe you want to add to this conversation the person who makes the
libusb1.0 port as I'm not skilled with libusb (only learned the needed
to make this fix). As I said, the fix works for me and the tests I made
goes fine, but maybe there is something I did not consider.
I'll try to make the re-patch along today.
Thanks. :)
