Package: garmin-forerunner-tools
Version: 0.10repacked-7
Severity: normal
Tags: patch
User: ubuntu-de...@lists.ubuntu.com
Usertags: origin-ubuntu xenial ubuntu-patch
Hi folks,
The Ubuntu autobuilders have detected a problem with your package on 64-bit
architectures. The code is using various standard C functions without
including the headers necessary to declare them. Implicit declarations are
treated as returning an int, which means for any of these functions that
return a pointer the return value will be truncated on 64-bit architectures,
typically resulting in a segfault.
The attached patch fixes the various missing function declarations, which
should eliminate various bugs - including segfaults on 64-bit architectures
- and allow the package to build in Ubuntu, where this is treated as a build
failure.
In the process, I've also identified some issues in debian/rules that
prevent the package from cleanly building in place more than once. Please
find the patch for all of these issues attached. It has been uploaded to
Ubuntu with the following changelog:
* debian/patches/missing-prototypes: include missing headers to ensure
proper declarations.
* fix debian/rules dependencies to not make config.status depend on
'configure' target, a file that will be removed in debian/rules clean.
* fix clean target to not fail.
As an aside, I had a brief look at bug #816314 to see if it was related. It
wasn't; the crashing function was unaffected by this bug, and the crash was
reported on i386, a 32-bit architecture. But what I saw of the code in the
process leaves me concerned about the overall code quality in this package.
In particular, this construction in garmin_open():
if ( garmin->usb.handle == NULL ) {
if ( ctx == NULL ) {
[...]
if ( err ) {
[...]
return ( garmin->usb.handle != NULL );
}
}
[...]
}
So obviously, garmin->usb.handle is NULL at this point...?
The build log also reports that libgarmintools is not linked against the
libusb library that it depends on, which could cause bugs later if libusb
ever started using symbol versioning.
Hopefully this patch will help with this package's utility, but it looks to
me like some deeper maintenance might be in order.
Thanks for considering,
--
Steve Langasek Give me a lever long enough and a Free OS
Debian Developer to set it on, and I can move the world.
Ubuntu Developer http://www.debian.org/
slanga...@ubuntu.com vor...@debian.org
diff -Nru garmin-forerunner-tools-0.10repacked/debian/patches/missing-prototypes garmin-forerunner-tools-0.10repacked/debian/patches/missing-prototypes
--- garmin-forerunner-tools-0.10repacked/debian/patches/missing-prototypes 1969-12-31 16:00:00.000000000 -0800
+++ garmin-forerunner-tools-0.10repacked/debian/patches/missing-prototypes 2016-03-27 22:55:23.000000000 -0700
@@ -0,0 +1,106 @@
+Description: include missing headers to ensure proper declarations
+ garmin-forerunner-tools uses several standard C functions without including
+ the proper headers needed to pick up their declarations. Some of these
+ functions return pointers. Failure to include the headers means the
+ return type is assumed to be an int, which means the pointer is implicitly
+ cast to an int, losing data on 64-bit architectures.
+Author: Steve Langasek <steve.langa...@ubuntu.com>
+
+Index: garmin-forerunner-tools-0.10repacked/src/usb_comm.c
+===================================================================
+--- garmin-forerunner-tools-0.10repacked.orig/src/usb_comm.c
++++ garmin-forerunner-tools-0.10repacked/src/usb_comm.c
+@@ -21,6 +21,7 @@
+ #include <stdio.h>
+ #include <string.h>
+ #include <ctype.h>
++#include <stdlib.h>
+ #include <libusb.h>
+ #include "garmin.h"
+
+Index: garmin-forerunner-tools-0.10repacked/src/unpack.c
+===================================================================
+--- garmin-forerunner-tools-0.10repacked.orig/src/unpack.c
++++ garmin-forerunner-tools-0.10repacked/src/unpack.c
+@@ -22,6 +22,7 @@
+ #include <sys/stat.h>
+ #include <fcntl.h>
+ #include <unistd.h>
++#include <stdlib.h>
+ #include <string.h>
+ #include <errno.h>
+ #include "garmin.h"
+Index: garmin-forerunner-tools-0.10repacked/src/pack.c
+===================================================================
+--- garmin-forerunner-tools-0.10repacked.orig/src/pack.c
++++ garmin-forerunner-tools-0.10repacked/src/pack.c
+@@ -23,6 +23,8 @@
+ #include <fcntl.h>
+ #include <errno.h>
+ #include <string.h>
++#include <stdlib.h>
++#include <unistd.h>
+ #include "garmin.h"
+
+
+Index: garmin-forerunner-tools-0.10repacked/src/run.c
+===================================================================
+--- garmin-forerunner-tools-0.10repacked.orig/src/run.c
++++ garmin-forerunner-tools-0.10repacked/src/run.c
+@@ -18,6 +18,8 @@
+ */
+
+ #include "config.h"
++#include <stdlib.h>
++#include <unistd.h>
+ #include <time.h>
+ #include <string.h>
+ #include <errno.h>
+Index: garmin-forerunner-tools-0.10repacked/src/garmin_get_info.c
+===================================================================
+--- garmin-forerunner-tools-0.10repacked.orig/src/garmin_get_info.c
++++ garmin-forerunner-tools-0.10repacked/src/garmin_get_info.c
+@@ -19,6 +19,7 @@
+
+ #include "config.h"
+ #include <stdio.h>
++#include <unistd.h>
+ #include "garmin.h"
+
+
+Index: garmin-forerunner-tools-0.10repacked/src/garmin_gmap.c
+===================================================================
+--- garmin-forerunner-tools-0.10repacked.orig/src/garmin_gmap.c
++++ garmin-forerunner-tools-0.10repacked/src/garmin_gmap.c
+@@ -20,6 +20,7 @@
+ #include "config.h"
+ #include <math.h>
+ #include <stdio.h>
++#include <stdlib.h>
+ #include "garmin.h"
+
+
+Index: garmin-forerunner-tools-0.10repacked/src/garmin_gchart.c
+===================================================================
+--- garmin-forerunner-tools-0.10repacked.orig/src/garmin_gchart.c
++++ garmin-forerunner-tools-0.10repacked/src/garmin_gchart.c
+@@ -2,6 +2,7 @@
+ #include <math.h>
+ #include <float.h>
+ #include <stdio.h>
++#include <stdlib.h>
+ #include <string.h>
+ #include "garmin.h"
+
+Index: garmin-forerunner-tools-0.10repacked/src/garmin_gpx.c
+===================================================================
+--- garmin-forerunner-tools-0.10repacked.orig/src/garmin_gpx.c
++++ garmin-forerunner-tools-0.10repacked/src/garmin_gpx.c
+@@ -20,6 +20,7 @@
+ #include "config.h"
+ #include <math.h>
+ #include <stdio.h>
++#include <stdlib.h>
+ #include <string.h>
+ #include <time.h>
+ #include "garmin.h"
diff -Nru garmin-forerunner-tools-0.10repacked/debian/patches/series garmin-forerunner-tools-0.10repacked/debian/patches/series
--- garmin-forerunner-tools-0.10repacked/debian/patches/series 2016-02-13 09:51:26.000000000 -0800
+++ garmin-forerunner-tools-0.10repacked/debian/patches/series 2016-03-27 22:39:34.000000000 -0700
@@ -4,3 +4,4 @@
gcc4.8
python_dir_from_upstream.patch
libusb-1.0.patch
+missing-prototypes
diff -Nru garmin-forerunner-tools-0.10repacked/debian/rules garmin-forerunner-tools-0.10repacked/debian/rules
--- garmin-forerunner-tools-0.10repacked/debian/rules 2014-09-20 00:27:33.000000000 -0700
+++ garmin-forerunner-tools-0.10repacked/debian/rules 2016-03-27 23:00:37.000000000 -0700
@@ -15,7 +15,7 @@
INSTALLDIR = $(CURDIR)/debian/garmin-forerunner-tools
-config.status: configure
+config.status:
dh_testdir
dh_autoreconf
./configure --host=$(DEB_HOST_GNU_TYPE) \
@@ -39,7 +39,7 @@
rm -f build-stamp
[ ! -e Makefile ] || $(MAKE) distclean
dh_autoreconf_clean
- -rm config.status config.log Makefile
+ rm -f config.status config.log Makefile
dh_clean
install: build
