dpkg version comparison bug

Eugene T. Bordenkircher eugebo at gmail.com
Wed Nov 19 21:54:25 UTC 2008


Hello all,

Below, you'll find a patch against the current SVN head to fix a couple of bugs 
in the version comparison code for dpkg. As a brief explanation: (1) the old 
code did not handle epochs that were greater than a single digit, and (2) it 
did not compare alpha() portions of the version string correctly, most notably 
portions that included ~'s and other punctuation.

You'll notice that the solution is almost a mirror image of the official code 
in dpkg.  I looked at it and couldn't think of any better way than that to do 
the comparison.  I added the copyright information at the top to make it official.

So it correctly compares versions now and probably more importantly to some:

    text	   data	    bss	    dec	    hex	filename
  688420	   2168	   9388	 699976	  aae48	busybox.orig/busybox
  688273	   2168	   9388	 699829	  aadb5	busybox.new/busybox

Enjoy, and let me know if there is anything I missed or need to do a bit 
differently.

-- 
Eugene T. Bordenkircher
eugebo at gmail.com--- busybox.orig/archival/dpkg.c	2008-11-19 12:48:29.000000000 
-0800
+++ busybox.new/archival/dpkg.c	2008-11-19 12:56:43.000000000 -0800
@@ -6,6 +6,10 @@
   *  written by glenn mcgrath with the help of others
   *  copyright (c) 2001 by glenn mcgrath
   *
+ *  parts of the version comparison code is plucked from the real dpkg
+ *  application which is licensed GPLv2 and
+ *  copyright (c) 1995 Ian Jackson <ian at chiark.greenend.org.uk>
+ *
   *  started life as a busybox implementation of udpkg
   *
   * licensed under gplv2 or later, see file license in this tarball for details.
@@ -28,6 +32,8 @@
  #include <fnmatch.h>
  #include "unarchive.h"

+#include <ctype.h>
+
  /* note: if you vary hash_prime sizes be aware,
   * 1) tweaking these will have a big effect on how much memory this program uses.
   * 2) for computational efficiency these hash tables should be at least 20%
@@ -183,60 +189,41 @@
  	return probe_address;
  }

-/* Need to rethink version comparison, maybe the official dpkg has something i 
can use ? */
-static int version_compare_part(const char *version1, const char *version2)
+/* This code is taken from dpkg and was converted to a function to save
+ * a few bytes in the text section of busybox */
+static int order( char x )
  {
-	int upstream_len1 = 0;
-	int upstream_len2 = 0;
-	char *name1_char;
-	char *name2_char;
-	int len1 = 0;
-	int len2 = 0;
-	int tmp_int;
-	int ver_num1;
-	int ver_num2;
-
-	if (version1 == NULL) {
-		version1 = xstrdup("");
-	}
-	if (version2 == NULL) {
-		version2 = xstrdup("");
-	}
-	upstream_len1 = strlen(version1);
-	upstream_len2 = strlen(version2);
-
-	while ((len1 < upstream_len1) || (len2 < upstream_len2)) {
-		/* Compare non-digit section */
-		tmp_int = strcspn(&version1[len1], "0123456789");
-		name1_char = xstrndup(&version1[len1], tmp_int);
-		len1 += tmp_int;
-		tmp_int = strcspn(&version2[len2], "0123456789");
-		name2_char = xstrndup(&version2[len2], tmp_int);
-		len2 += tmp_int;
-		tmp_int = strcmp(name1_char, name2_char);
-		free(name1_char);
-		free(name2_char);
-		if (tmp_int != 0) {
-			return tmp_int;
-		}
-
-		/* Compare digits */
-		tmp_int = strspn(&version1[len1], "0123456789");
-		name1_char = xstrndup(&version1[len1], tmp_int);
-		len1 += tmp_int;
-		tmp_int = strspn(&version2[len2], "0123456789");
-		name2_char = xstrndup(&version2[len2], tmp_int);
-		len2 += tmp_int;
-		ver_num1 = atoi(name1_char);
-		ver_num2 = atoi(name2_char);
-		free(name1_char);
-		free(name2_char);
-		if (ver_num1 < ver_num2) {
-			return -1;
+	return ( x == '~' ? -1
+			 : isdigit(x) ? 0
+			 : !(x) ? 0
+			 : isalpha(x) ? x
+			 : x + 256 );
  		}
-		if (ver_num1 > ver_num2) {
-			return 1;
+
+/* This code is taken from dpkg and modified slightly to work with busybox */
+static int version_compare_part(const char *val, const char *ref)
+{
+	if (!val) val = "";
+	if (!ref) ref = "";
+
+	while (*val || *ref) {
+		int first_diff = 0;
+
+		while ( (*val && !isdigit(*val)) || (*ref && !isdigit(*ref)) ) {
+			int vc = order(*val), rc = order(*ref);
+			if (vc != rc) return vc - rc;
+			val++; ref++;
+		}
+
+		while ( *val == '0' ) val++;
+		while ( *ref == '0' ) ref++;
+		while (isdigit(*val) && isdigit(*ref)) {
+			if (!first_diff) first_diff = *val - *ref;
+			val++; ref++;
  		}
+		if (isdigit(*val)) return 1;
+		if (isdigit(*ref)) return -1;
+		if (first_diff) return first_diff;
  	}
  	return 0;
  }
@@ -250,27 +237,23 @@
  	char *ch_ver1 = name_hashtable[ver1];
  	char *ch_ver2 = name_hashtable[ver2];

-	char epoch1, epoch2;
+	unsigned long epoch1 = 0, epoch2 = 0;
+	char *colon;
  	char *deb_ver1, *deb_ver2;
-	char *ver1_ptr, *ver2_ptr;
  	char *upstream_ver1;
  	char *upstream_ver2;
  	int result;

  	/* Compare epoch */
-	if (ch_ver1[1] == ':') {
-		epoch1 = ch_ver1[0];
-		ver1_ptr = strchr(ch_ver1, ':') + 1;
-	} else {
-		epoch1 = '0';
-		ver1_ptr = ch_ver1;
-	}
-	if (ch_ver2[1] == ':') {
-		epoch2 = ch_ver2[0];
-		ver2_ptr = strchr(ch_ver2, ':') + 1;
-	} else {
-		epoch2 = '0';
-		ver2_ptr = ch_ver2;
+	colon = strchr(ch_ver1,':');
+	if (colon) {
+		epoch1 = atoi(ch_ver1);
+		ch_ver1 = colon+1;
+	}
+	colon = strchr(ch_ver2,':');
+	if (colon) {
+		epoch2 = atoi(ch_ver2);
+		ch_ver2 = colon+1;
  	}
  	if (epoch1 < epoch2) {
  		return -1;
@@ -280,8 +263,8 @@
  	}

  	/* Compare upstream version */
-	upstream_ver1 = xstrdup(ver1_ptr);
-	upstream_ver2 = xstrdup(ver2_ptr);
+	upstream_ver1 = xstrdup(ch_ver1);
+	upstream_ver2 = xstrdup(ch_ver2);

  	/* Chop off debian version, and store for later use */
  	deb_ver1 = strrchr(upstream_ver1, '-');

----------------------------



More information about the busybox mailing list