Debian Bug report logs - #816314
[garmin-forerunner-tools] Segmentation fault

Toggle useless messages

Message #5 received at submit@bugs.debian.org (full text, mbox, reply):

From: Fenix <fenixian@gmail.com>

To: submit@bugs.debian.org

Subject: [garmin-forerunner-tools] Segmentation fault

Date: Mon, 29 Feb 2016 20:32:47 +0100

Package: garmin-forerunner-tools
Version: 0.10repacked-7
Severity: important

--- Please enter the report below this line. ---


    Dear maintainer:

    All programs (garmin-get-info, garmin-save-runs) of 
garmin-forerunner-tools packages throws me a Segmentation Fault.

    With old versions it worked fine.


$ garmin-get-info

Violación de segmento

$ garmin-get-info -v

[garmin] libusb_init succeeded
[garmin] found VID 091e, PID 0003[garmin] libusb_open = 0x8079430
[garmin] libusb_set_configuration[1] succeeded
[garmin] libusb_claim_interface[0] succeeded
[garmin] libusb_get_config_descriptor_by_value succeeded
[garmin] intr IN  = 1
[garmin] bulk OUT = 2
[garmin] bulk IN  = 3
<write type="0x00" id="0x0005" size="0"/>
<write type="0x00" id="0x0005" size="0"/>
<write type="0x00" id="0x0005" size="0"/>
<write type="0x14" id="0x00fe" size="0"/>
<garmin_unit id="0">
 <garmin_product id="0" software_version="0.00">
  <product_description>(null)</product_description>
 </garmin_product>
 <garmin_protocols>
  <garmin_physical protocol="P000"/>
  <garmin_link protocol="L000"/>
  <garmin_command protocol="A000"/>
 </garmin_protocols>
</garmin_unit>



$ valgrind garmin-get-info

==2586== Use of uninitialised value of size 4
==2586==    at 0x406B600: garmin_open (in /usr/lib/libgarmintools.so.4.2.0)
==2586==    by 0x40747C1: garmin_init (in /usr/lib/libgarmintools.so.4.2.0)
==2586==    by 0x80485B9: ??? (in /usr/bin/garmin_get_info)
==2586==    by 0x40C870D: (below main) (in 
/lib/i386-linux-gnu/i686/cmov/libc-2.21.so)
==2586==
==2586== Invalid read of size 4
==2586==    at 0x406B603: garmin_open (in /usr/lib/libgarmintools.so.4.2.0)
==2586==    by 0x40747C1: garmin_init (in /usr/lib/libgarmintools.so.4.2.0)
==2586==    by 0x80485B9: ??? (in /usr/bin/garmin_get_info)
==2586==    by 0x40C870D: (below main) (in 
/lib/i386-linux-gnu/i686/cmov/libc-2.21.so)
==2586==  Address 0xc0012 is not stack'd, malloc'd or (recently) free'd
==2586==
==2586==
==2586== Process terminating with default action of signal 11 (SIGSEGV)
==2586==  Access not within mapped region at address 0xC0012
==2586==    at 0x406B603: garmin_open (in /usr/lib/libgarmintools.so.4.2.0)
==2586==    by 0x40747C1: garmin_init (in /usr/lib/libgarmintools.so.4.2.0)
==2586==    by 0x80485B9: ??? (in /usr/bin/garmin_get_info)
==2586==    by 0x40C870D: (below main) (in 
/lib/i386-linux-gnu/i686/cmov/libc-2.21.so)
==2586==  If you believe this happened as a result of a stack
==2586==  overflow in your program's main thread (unlikely but
==2586==  possible), you can try to increase the size of the
==2586==  main thread stack using the --main-stacksize= flag.
==2586==  The main thread stack size used in this run was 8388608.
==2586==
==2586== HEAP SUMMARY:
==2586==     in use at exit: 16,069 bytes in 31 blocks
==2586==   total heap usage: 1,277 allocs, 1,246 frees, 622,195 bytes 
allocated
==2586==
==2586== LEAK SUMMARY:
==2586==    definitely lost: 0 bytes in 0 blocks
==2586==    indirectly lost: 0 bytes in 0 blocks
==2586==      possibly lost: 192 bytes in 4 blocks
==2586==    still reachable: 15,877 bytes in 27 blocks
==2586==         suppressed: 0 bytes in 0 blocks
==2586== Rerun with --leak-check=full to see details of leaked memory
==2586==
==2586== For counts of detected and suppressed errors, rerun with: -v
==2586== Use --track-origins=yes to see where uninitialised values come from
==2586== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 from 0)



Thanks.




--- System information. ---
Architecture: i386
Kernel:       Linux 4.3.0-1-686-pae

Debian Release: stretch/sid
  500 testing         www.deb-multimedia.org
  500 testing         security.debian.org
  500 testing         ftp.de.debian.org
  500 stable          dl.google.com

--- Package information. ---
Depends              (Version) | Installed
==============================-+-==============
libc6                 (>= 2.4) | 2.21-9
libusb-0.1-4     (>= 2:0.1.12) | 2:0.1.12-28


Package's Recommends field is empty.

Package's Suggests field is empty.

Message #10 received at 816314@bugs.debian.org (full text, mbox, reply):

From: Fenix <fenixian@gmail.com>

To: 816314@bugs.debian.org

Subject: Fix patch for 816314.

Date: Wed, 6 Apr 2016 00:47:09 +0200

[Message part 1 (text/plain, inline)]

Dear maintainer:

As the new version didn't fix this bug, I looked to the code and I find 
the problem (at least for me, but I really don't know how this error has 
been hidden just now. Maybe the old libusb masked the error in the code?).

The problem is in protocol.c

In the code:

--
case Tag_Appl_Prot_Id:
memset(datatypes,0,size * sizeof(uint16));
for ( j = i+1; p.packet.data[3*j] == Tag_Data_Type_Id; j++ ) {
datatypes[j-i-1] = get_uint16(p.packet.data + 3*j + 1);
}
--


The outing condition for the FOR loop throws the segmentation because 
didn't check the limit of j.


I fixed it checking first the counter 'j' and adjust it to the limit of 
the data.


--
case Tag_Appl_Prot_Id:
memset(datatypes,0,size * sizeof(uint16));
for ( j = i+1; (j<=size) && (p.packet.data[3*j] == Tag_Data_Type_Id); 
j++ ) {
datatypes[j-i-1] = get_uint16(p.packet.data + 3*j + 1);
}
--


I attach the patch file that fix this bug.

This is my first time I send a patch, so maybe it doesn't correct. If 
you need more information or anything else feel free to ask.


Thanks.

[816314_fix.patch (text/x-patch, attachment)]

Message #15 received at 816314@bugs.debian.org (full text, mbox, reply):

From: Christian PERRIER <bubulle@debian.org>

To: Fenix <fenixian@gmail.com>, 816314@bugs.debian.org

Subject: Re: [Pkg-running-devel] Bug#816314: Fix patch for 816314.

Date: Wed, 6 Apr 2016 08:32:42 +0200

[Message part 1 (text/plain, inline)]

Quoting Fenix (fenixian@gmail.com):
> 
> Dear maintainer:
> 
> As the new version didn't fix this bug, I looked to the code and I find the
> problem (at least for me, but I really don't know how this error has been
> hidden just now. Maybe the old libusb masked the error in the code?).
> 
> The problem is in protocol.c

.../...

Thanks a lot. Not owning a Forerunner anymore, I'm indeed in the blind
to test the package, so I couldn't do much for this bug.

So, your help is highly appreciated. I'm in the process of
test-building the package and will upload it ASAP (from what I
understand, it won't be worse than the current version of the package
anyway.

BTW, the bug was indeed release critical, I should have raised its
severity to serious

[signature.asc (application/pgp-signature, inline)]

Message #20 received at 816314-close@bugs.debian.org (full text, mbox, reply):

From: Christian Perrier <bubulle@debian.org>

To: 816314-close@bugs.debian.org

Subject: Bug#816314: fixed in garmin-forerunner-tools 0.10repacked-9

Date: Wed, 06 Apr 2016 09:50:50 +0000

Source: garmin-forerunner-tools
Source-Version: 0.10repacked-9

We believe that the bug you reported is fixed in the latest version of
garmin-forerunner-tools, which is due to be installed in the Debian FTP archive.

A summary of the changes between this version and the previous one is
attached.

Thank you for reporting the bug, which will now be closed.  If you
have further comments please address them to 816314@bugs.debian.org,
and the maintainer will reopen the bug report if appropriate.

Debian distribution maintenance software
pp.
Christian Perrier <bubulle@debian.org> (supplier of updated garmin-forerunner-tools package)

(This message was generated automatically at their request; if you
believe that there is a problem with it please contact the archive
administrators by mailing ftpmaster@ftp-master.debian.org)


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Format: 1.8
Date: Wed, 06 Apr 2016 08:57:27 +0200
Source: garmin-forerunner-tools
Binary: garmin-forerunner-tools
Architecture: source i386
Version: 0.10repacked-9
Distribution: unstable
Urgency: medium
Maintainer: Debian running develpment group <pkg-running-devel@lists.alioth.debian.org>
Changed-By: Christian Perrier <bubulle@debian.org>
Description:
 garmin-forerunner-tools - retrieve data from Garmin Forerunner/Edge GPS devices
Closes: 816314
Changes:
 garmin-forerunner-tools (0.10repacked-9) unstable; urgency=medium
 .
   * Fix segmentation fault by properly checking the size of a counter
     in protocol.c.
     Great thanks to Fenix for the patch
     Closes: #816314
Checksums-Sha1:
 7a81d77e28a20f48d490dd753f7cd6bc48d69bbe 2254 garmin-forerunner-tools_0.10repacked-9.dsc
 35c2d5fef922f7dfce8e0ec18b6c155c94239a84 16348 garmin-forerunner-tools_0.10repacked-9.debian.tar.xz
 ac567721fb37a59843c4517c41114e1ebd7ee074 193636 garmin-forerunner-tools-dbgsym_0.10repacked-9_i386.deb
 7ebee932df1f437c89eedff2848d5f8d933194ee 119000 garmin-forerunner-tools_0.10repacked-9_i386.deb
Checksums-Sha256:
 7ca55e98cad94ee69b484508b7f64dc5247e0c0c10579a40d3347ad4e595c547 2254 garmin-forerunner-tools_0.10repacked-9.dsc
 fed08f6663197563802e95ff861f719aac9b99d791aad38090afdeb4d79f3640 16348 garmin-forerunner-tools_0.10repacked-9.debian.tar.xz
 6e90173fed70c15a0627aa8f5c540c33162848308562cbcc2ea93443a1c7ba3a 193636 garmin-forerunner-tools-dbgsym_0.10repacked-9_i386.deb
 107b29a2f0f2644d80e82910f7042133d17745acd878a8db07e6e57c39a6dcab 119000 garmin-forerunner-tools_0.10repacked-9_i386.deb
Files:
 50005d8ea009f8372b908b7a40697a0d 2254 utils extra garmin-forerunner-tools_0.10repacked-9.dsc
 8f186360eb567efeafca8a981defb517 16348 utils extra garmin-forerunner-tools_0.10repacked-9.debian.tar.xz
 866e63a023f04f9f943933705ee68f23 193636 debug extra garmin-forerunner-tools-dbgsym_0.10repacked-9_i386.deb
 e0588897815a1c2790b5a5f7273f6142 119000 utils extra garmin-forerunner-tools_0.10repacked-9_i386.deb

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQIcBAEBCAAGBQJXBMjEAAoJEIcvcCxNbiWo2bIP+wZ1A9HKIn1q3F1YTiGDjdvw
OkbeA0uxfKEdi1s4SC63Y3ap9lQuHHAxvPhdsL1a/pN1JnfnM8Dfr/Uif1h90Dxe
uy/y3xMBzRwoRULQ9HB8tXc+giUKuZHfsietVpg/rr+k/3+KO4+0qkXtWTd3eq3m
LG3qbewgkl5Y3AKjCjMUR5Lv6N5TTcB6YkuGgUHlyrVa8/eul4GmCmILpIEFn/xy
1E3sh5AjUGuujTo6laGWGXmTmnCyUxJcyd3hV0WqVwFGFBlz+egz7Iq/g4i8Wbpy
aDI2oGKndmGppi0cxXJAwvRy/ACcjsLr30DIc0XJRkNLFB/N0w/2tZc5YF7zq3aR
uvfcgeSATKrJmSriPye/WTWbxbPGOEqGCMAYD/buIYq5I+I0LwzrPOwv6Y3ElBY0
EXtl7T1nCaZn6FH16+UpWEQ8FEhT7v7Yu1a0TeAHle8NigYQ7OQvj9praHt+LwxR
Ki5eTUYLIEGz3238CY8y0RVBG/0cROaNpd++Q8MpdLQWoxb9JZBy4LgME5o0bNUI
tKdZuKYsgRwmeLvc5z61dfqha1AgLD42F9oVGThumvyVeuXdExux+8qXhzhLdmAL
Mu74Lg9tiacz9WcXzwqiOpzhePMNo9EYvBDEDztNlkpiHPyiVOdKCPvNS0pjF5Nv
acu4pk42eGC1FAMLPBEK
=Ge12
-----END PGP SIGNATURE-----

Message #25 received at 816314@bugs.debian.org (full text, mbox, reply):

From: Chris AtLee <chris@atlee.ca>

To: 816314@bugs.debian.org

Subject: Re: Bug#816314: fixed in garmin-forerunner-tools 0.10repacked-9

Date: Thu, 14 Apr 2016 13:21:39 -0400

[Message part 1 (text/plain, inline)]

I'm still getting a segfault in the latest version. Here's the traceback:

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff7bba378 in garmin_open (garmin=garmin@entry=0x7fffffffe350) at
usb_comm.c:137
137     usb_comm.c: No such file or directory.
(gdb) where
#0  0x00007ffff7bba378 in garmin_open (garmin=garmin@entry=0x7fffffffe350)
at usb_comm.c:137
#1  0x00007ffff7bc3180 in garmin_init (garmin=garmin@entry=0x7fffffffe350,
    verbose=<optimized out>) at protocol.c:1186
#2  0x0000000000400725 in main (argc=<optimized out>, argv=<optimized out>)
    at garmin_save_runs.c:36

On Wed, 06 Apr 2016 09:50:50 +0000 Christian Perrier <bubulle@debian.org>
wrote:
> Source: garmin-forerunner-tools
> Source-Version: 0.10repacked-9
>
> We believe that the bug you reported is fixed in the latest version of
> garmin-forerunner-tools, which is due to be installed in the Debian FTP
archive.
>
> A summary of the changes between this version and the previous one is
> attached.
>
> Thank you for reporting the bug, which will now be closed.  If you
> have further comments please address them to 816314@bugs.debian.org,
> and the maintainer will reopen the bug report if appropriate.
>
> Debian distribution maintenance software
> pp.
> Christian Perrier <bubulle@debian.org> (supplier of updated
garmin-forerunner-tools package)
>
> (This message was generated automatically at their request; if you
> believe that there is a problem with it please contact the archive
> administrators by mailing ftpmaster@ftp-master.debian.org)
>
>
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA256
>
> Format: 1.8
> Date: Wed, 06 Apr 2016 08:57:27 +0200
> Source: garmin-forerunner-tools
> Binary: garmin-forerunner-tools
> Architecture: source i386
> Version: 0.10repacked-9
> Distribution: unstable
> Urgency: medium
> Maintainer: Debian running develpment group <
pkg-running-devel@lists.alioth.debian.org>
> Changed-By: Christian Perrier <bubulle@debian.org>
> Description:
>  garmin-forerunner-tools - retrieve data from Garmin Forerunner/Edge GPS
devices
> Closes: 816314
> Changes:
>  garmin-forerunner-tools (0.10repacked-9) unstable; urgency=medium
>  .
>    * Fix segmentation fault by properly checking the size of a counter
>      in protocol.c.
>      Great thanks to Fenix for the patch
>      Closes: #816314
> Checksums-Sha1:
>  7a81d77e28a20f48d490dd753f7cd6bc48d69bbe 2254
garmin-forerunner-tools_0.10repacked-9.dsc
>  35c2d5fef922f7dfce8e0ec18b6c155c94239a84 16348
garmin-forerunner-tools_0.10repacked-9.debian.tar.xz
>  ac567721fb37a59843c4517c41114e1ebd7ee074 193636
garmin-forerunner-tools-dbgsym_0.10repacked-9_i386.deb
>  7ebee932df1f437c89eedff2848d5f8d933194ee 119000
garmin-forerunner-tools_0.10repacked-9_i386.deb
> Checksums-Sha256:
>  7ca55e98cad94ee69b484508b7f64dc5247e0c0c10579a40d3347ad4e595c547 2254
garmin-forerunner-tools_0.10repacked-9.dsc
>  fed08f6663197563802e95ff861f719aac9b99d791aad38090afdeb4d79f3640 16348
garmin-forerunner-tools_0.10repacked-9.debian.tar.xz
>  6e90173fed70c15a0627aa8f5c540c33162848308562cbcc2ea93443a1c7ba3a 193636
garmin-forerunner-tools-dbgsym_0.10repacked-9_i386.deb
>  107b29a2f0f2644d80e82910f7042133d17745acd878a8db07e6e57c39a6dcab 119000
garmin-forerunner-tools_0.10repacked-9_i386.deb
> Files:
>  50005d8ea009f8372b908b7a40697a0d 2254 utils extra
garmin-forerunner-tools_0.10repacked-9.dsc
>  8f186360eb567efeafca8a981defb517 16348 utils extra
garmin-forerunner-tools_0.10repacked-9.debian.tar.xz
>  866e63a023f04f9f943933705ee68f23 193636 debug extra
garmin-forerunner-tools-dbgsym_0.10repacked-9_i386.deb
>  e0588897815a1c2790b5a5f7273f6142 119000 utils extra
garmin-forerunner-tools_0.10repacked-9_i386.deb

[Message part 2 (text/html, inline)]

Message #30 received at 816314@bugs.debian.org (full text, mbox, reply):

From: Christian PERRIER <bubulle@debian.org>

To: Chris AtLee <chris@atlee.ca>, 816314@bugs.debian.org, Fenix <fenixian@gmail.com>

Subject: Re: [Pkg-running-devel] Bug#816314: fixed in garmin-forerunner-tools 0.10repacked-9

Date: Fri, 15 Apr 2016 06:53:00 +0200

[Message part 1 (text/plain, inline)]

Quoting Chris AtLee (chris@atlee.ca):
> I'm still getting a segfault in the latest version. Here's the traceback:
> 
> Program received signal SIGSEGV, Segmentation fault.
> 0x00007ffff7bba378 in garmin_open (garmin=garmin@entry=0x7fffffffe350) at
> usb_comm.c:137
> 137     usb_comm.c: No such file or directory.
> (gdb) where
> #0  0x00007ffff7bba378 in garmin_open (garmin=garmin@entry=0x7fffffffe350)
> at usb_comm.c:137
> #1  0x00007ffff7bc3180 in garmin_init (garmin=garmin@entry=0x7fffffffe350,
>     verbose=<optimized out>) at protocol.c:1186
> #2  0x0000000000400725 in main (argc=<optimized out>, argv=<optimized out>)
>     at garmin_save_runs.c:36


We may need to check this with Fenix, who provided the patch I
used. Sadly, I'm not in position to test myself....

[signature.asc (application/pgp-signature, inline)]

Message #35 received at 816314@bugs.debian.org (full text, mbox, reply):

From: Fenix <fenixian@gmail.com>

To: 816314@bugs.debian.org

Subject: Re: [Pkg-running-devel] Bug#816314: fixed in garmin-forerunner-tools 0.10repacked-9

Date: Fri, 15 Apr 2016 11:03:20 +0200

On 15/04/16 06:53, Christian PERRIER wrote:
> Quoting Chris AtLee (chris@atlee.ca):
>> I'm still getting a segfault in the latest version. Here's the traceback:
>>
>> Program received signal SIGSEGV, Segmentation fault.
>> 0x00007ffff7bba378 in garmin_open (garmin=garmin@entry=0x7fffffffe350) at
>> usb_comm.c:137
>> 137     usb_comm.c: No such file or directory.
>> (gdb) where
>> #0  0x00007ffff7bba378 in garmin_open (garmin=garmin@entry=0x7fffffffe350)
>> at usb_comm.c:137
>> #1  0x00007ffff7bc3180 in garmin_init (garmin=garmin@entry=0x7fffffffe350,
>>      verbose=<optimized out>) at protocol.c:1186
>> #2  0x0000000000400725 in main (argc=<optimized out>, argv=<optimized out>)
>>      at garmin_save_runs.c:36
>
>
> We may need to check this with Fenix, who provided the patch I
> used. Sadly, I'm not in position to test myself....
>


  Hi.

  I just went to re-open this. :)


  I don't know if that is important, but I see that in the master is 
not my change. Is it possible that the repacked9 is in real repacked8 again?

Message #40 received at 816314@bugs.debian.org (full text, mbox, reply):

From: Fenix <fenixian@gmail.com>

To: 816314@bugs.debian.org

Subject: Re: [Pkg-running-devel] Bug#816314: fixed in garmin-forerunner-tools 0.10repacked-9

Date: Fri, 15 Apr 2016 20:52:01 +0200

Oooops.

I think that I know what happens. I just did clone [ 
git://git.debian.org/git/pkg-running/garmin-forerunner-tools.git ] 
thinking that the master branch was updated until the last repackage.

I just see that the patches is inside de Debian/patches and not applied 
to the source code.


So, was me who was seeing a non updated code. I'm sorry. I didn't know 
the Debian workflow.


I'm going to update the code with the patches, and try to say something.

Message #45 received at 816314@bugs.debian.org (full text, mbox, reply):

From: Christian PERRIER <bubulle@debian.org>

To: Fenix <fenixian@gmail.com>, 816314@bugs.debian.org

Subject: Re: [Pkg-running-devel] Bug#816314: Bug#816314: fixed in garmin-forerunner-tools 0.10repacked-9

Date: Sat, 16 Apr 2016 09:25:57 +0200

[Message part 1 (text/plain, inline)]

Quoting Fenix (fenixian@gmail.com):
> Oooops.
> 
> I think that I know what happens. I just did clone [
> git://git.debian.org/git/pkg-running/garmin-forerunner-tools.git ] thinking
> that the master branch was updated until the last repackage.
> 
> I just see that the patches is inside de Debian/patches and not applied to
> the source code.
> 
> 
> So, was me who was seeing a non updated code. I'm sorry. I didn't know the
> Debian workflow.
> 
> 
> I'm going to update the code with the patches, and try to say something.

Well, the "816314_fix.patch" *is* in master.

The patch is in debian/patches *and* listed in debian/patches/series,
so as far as I can tell, it is applied at build time and thus is
ending in the built binaries.

Still the debian/0.10repacked-9 tag didn't end up on Alioth because of
an omission of mine on my local copy. This is now fixed.

[signature.asc (application/pgp-signature, inline)]

Message #50 received at 816314@bugs.debian.org (full text, mbox, reply):

From: Fenix <fenixian@gmail.com>

To: 816314@bugs.debian.org

Subject: Fix patch for libusb1.0 update.

Date: Sat, 16 Apr 2016 19:27:36 +0200

[Message part 1 (text/plain, inline)]

    Hi.


    I have fixed this. There were two problems in usb_comm.c when the 
update to libusb1.0:


	1) It seems libusb1.0 changes the addres of the endpoint to talk to. 
The code made a bit operation that makes the device unachievable.

	2) There were core code of libusb that only was execute with the -v 
(verbose) option, because an incorrect conditional anidation.


   The patch I attach fix the two problems and works for me (now with 
libusb1.0 :P). I have tested garmin_get_info, garmin_save_runs and 
garmin_gpx. If you need more information about this fix, please, feel 
free to ask.



   Thanks. :)

[libusb1.0_fix.patch (text/x-patch, attachment)]

Message #55 received at 816314@bugs.debian.org (full text, mbox, reply):

From: Christian PERRIER <bubulle@debian.org>

To: Fenix <fenixian@gmail.com>, 816314@bugs.debian.org

Subject: Re: [Pkg-running-devel] Bug#816314: Fix patch for libusb1.0 update.

Date: Sun, 17 Apr 2016 09:26:38 +0200

[Message part 1 (text/plain, inline)]

Quoting Fenix (fenixian@gmail.com):
> 
>     Hi.
> 
> 
>     I have fixed this. There were two problems in usb_comm.c when the update
> to libusb1.0:
> 
> 
> 	1) It seems libusb1.0 changes the addres of the endpoint to talk to. The
> code made a bit operation that makes the device unachievable.
> 
> 	2) There were core code of libusb that only was execute with the -v
> (verbose) option, because an incorrect conditional anidation.
> 
> 
>    The patch I attach fix the two problems and works for me (now with
> libusb1.0 :P). I have tested garmin_get_info, garmin_save_runs and
> garmin_gpx. If you need more information about this fix, please, feel free
> to ask.

It builds fine.

Still, there are a few chunks in the patch that look like noise to me.

For instance:

> diff --git a/src/usb_comm.c b/src/usb_comm.c
> index f00f6d9..9c5afa3 100644
> --- a/src/usb_comm.c
> +++ b/src/usb_comm.c
> @@ -72,9 +72,8 @@ garmin_open ( garmin_unit * garmin )
>        }
>      }
>      cnt = libusb_get_device_list(ctx,&dl);
> -    
> +
.../...


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

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

[signature.asc (application/pgp-signature, inline)]

Message #60 received at 816314@bugs.debian.org (full text, mbox, reply):

From: Fenix <fenixian@gmail.com>

To: 816314@bugs.debian.org

Subject: Re: [Pkg-running-devel] Bug#816314: Fix patch for libusb1.0 update.

Date: Sun, 17 Apr 2016 13:14:56 +0200

    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. :)

Message #65 received at 816314@bugs.debian.org (full text, mbox, reply):

From: Fenix <fenixian@gmail.com>

To: 816314@bugs.debian.org

Subject: Fix patch v2 for libusb1.0 update.

Date: Sun, 17 Apr 2016 18:58:59 +0200

[Message part 1 (text/plain, inline)]

    Hi.

    I attach the previous patch cleaned of unfunctional cosmetic changes.

    As I said in my previous message, I maintain the complex anidation 
of the original conditional structure. It is ugly, but it works. Anyway, 
if you think it should be refactor, we can make it a bit more readable.

   So, I'm keeping from previuos patch (libusb1.0_fix.patch):

   1) The elimination of a duplicate mark of block {} in the main for 
(this is cosmetic but is really ugly keep it. :))

   2) The changes for the else block in the conditional blocks 
(Functional change).

   3) The changes that pass the new (for libusb1.0) endpoints 
directions of the device (functional change). In this, I change too the 
printed messages for a better understanding for verbose purpose 
(cosmetic but I think it's neccesary for good verbosity).



   If you have any question or suggestion, please, feel free to ask.

   As I said in my last message, could be a good idea that the original 
patcher of the port to libusb1.0 checks this patch. I suppose he is 
experience with libusb, and I could overlook something related with.


   Thanks.  :)

[libusb1.0_fix_v2.patch (text/x-patch, attachment)]

Message #70 received at 816314@bugs.debian.org (full text, mbox, reply):

From: Christian PERRIER <bubulle@debian.org>

To: Fenix <fenixian@gmail.com>, 816314@bugs.debian.org

Subject: Re: [Pkg-running-devel] Bug#816314: Fix patch v2 for libusb1.0 update.

Date: Sun, 24 Apr 2016 08:44:56 +0200

[Message part 1 (text/plain, inline)]

Quoting Fenix (fenixian@gmail.com):
> 
>     Hi.
> 
>     I attach the previous patch cleaned of unfunctional cosmetic changes.
> 
>     As I said in my previous message, I maintain the complex anidation of
> the original conditional structure. It is ugly, but it works. Anyway, if you
> think it should be refactor, we can make it a bit more readable.
> 
>    So, I'm keeping from previuos patch (libusb1.0_fix.patch):
> 
>    1) The elimination of a duplicate mark of block {} in the main for (this
> is cosmetic but is really ugly keep it. :))
> 
>    2) The changes for the else block in the conditional blocks (Functional
> change).
> 
>    3) The changes that pass the new (for libusb1.0) endpoints directions of
> the device (functional change). In this, I change too the printed messages
> for a better understanding for verbose purpose (cosmetic but I think it's
> neccesary for good verbosity).
> 
> 
> 
>    If you have any question or suggestion, please, feel free to ask.
> 
>    As I said in my last message, could be a good idea that the original
> patcher of the port to libusb1.0 checks this patch. I suppose he is
> experience with libusb, and I could overlook something related with.


I just uploaded -10 yesterday.

Please let me know if it works OK now.

[signature.asc (application/pgp-signature, inline)]

Message #75 received at 816314@bugs.debian.org (full text, mbox, reply):

From: Simon Frei <freisim93@gmail.com>

To: Christian PERRIER <bubulle@debian.org>, 816314@bugs.debian.org

Subject: Re: Bug#816314: [Pkg-running-devel] Bug#816314: Fix patch v2 for libusb1.0 update.

Date: Sun, 24 Apr 2016 19:18:11 +0200

Yes, here it works. Thanks for maintaining this package!

On 24/04/16 08:44, Christian PERRIER wrote:
> Quoting Fenix (fenixian@gmail.com):
>>      Hi.
>>
>>      I attach the previous patch cleaned of unfunctional cosmetic changes.
>>
>>      As I said in my previous message, I maintain the complex anidation of
>> the original conditional structure. It is ugly, but it works. Anyway, if you
>> think it should be refactor, we can make it a bit more readable.
>>
>>     So, I'm keeping from previuos patch (libusb1.0_fix.patch):
>>
>>     1) The elimination of a duplicate mark of block {} in the main for (this
>> is cosmetic but is really ugly keep it. :))
>>
>>     2) The changes for the else block in the conditional blocks (Functional
>> change).
>>
>>     3) The changes that pass the new (for libusb1.0) endpoints directions of
>> the device (functional change). In this, I change too the printed messages
>> for a better understanding for verbose purpose (cosmetic but I think it's
>> neccesary for good verbosity).
>>
>>
>>
>>     If you have any question or suggestion, please, feel free to ask.
>>
>>     As I said in my last message, could be a good idea that the original
>> patcher of the port to libusb1.0 checks this patch. I suppose he is
>> experience with libusb, and I could overlook something related with.
>
> I just uploaded -10 yesterday.
>
> Please let me know if it works OK now.
>
>

Send a report that this bug log contains spam.