Fix signed vs unsigned value comparisons

Those were making some code to never be executed. This fixes most of the
bugs introduced with commit ea93ed48c17445d01b67f3fa8a20a5a644a89e5b.

diff --git a/ChangeLog b/ChangeLog
index eeb174bd2da9a492c752dc4c9dc34e659fced322..984ef053f080f0b4d552d567a54c0962cc49141b 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,25 @@
+2008-01-07  Guillem Jover  <guillem@debian.org>
+
+       * dpkg-deb/extract.c (extracthalf): Move negative member length check
+       to ...
+       (parseheaderlength): ... here. Change return type to size_t. Change
+       variable r to ssize_t. Use strtol instead of strtoul to easily catch
+       negative values.
+       * dpkg-deb/info.c (info_spew): Use %zu in a format string instead of
+       %lu to print pathlen, a size_t variable.
+       * dpkg-split/queue.c (do_auto): Change j from unsinged int to int.
+       Cast j to unsigned int when comparing it to i.
+       * lib/varbuf.c (varbufprintf): Change ou from unsigned int to size_t.
+       Change r from unsigned int to int. Cast value compared to r to int.
+       (varbufprintf): Likewise.
+       * src/archives.c (tarobject): Change r from size_t to ssize_t.
+       Cast r to size_t when comparing it to symlinkfn.size.
+       * src/main.c (setforce): Cast string difference to size_t instead of
+       int.
+       * utils/start-stop-daemon.c (parse_schedule): Change str_len from
+       ptrdiff_t to size_t. Cast string difference to size_t instead of
+       ptrdiff_t. Remove ptrdiff_t cast to sizeof.
+
 2008-01-07  Guillem Jover  <guillem@debian.org>
 
        * scripts/Dpkg/BuildOptions.pm (set): Parse all options separated

diff --git a/debian/changelog b/debian/changelog
index 0753382108fcbd0753a43dd79ad78526636ab0f5..ea465f7d1f9eef342597ab1e15fb1b5af7d45528 100644 (file)
--- a/debian/changelog
+++ b/debian/changelog
@@ -47,6 +47,8 @@ dpkg (1.14.15) UNRELEASED; urgency=low
     and allow overriding its value from the environment. Closes: #458589
   * Fix Dpkg::BuildOptions to parse all options in DEB_BUILD_OPTIONS, so
     that dpkg-buildpackage called with -j preserves unrecognized options.
+  * Fix several signed vs unsigned value comparisons that were making some
+    code to never be executed.
 
   [ Updated dpkg translations ]
   * Norwegian Bokmål (Hans Fredrik Nordhaug). Closes: #457918, #458732

diff --git a/dpkg-deb/extract.c b/dpkg-deb/extract.c
index 24cc1c13cdd7f291df7da24dd64517f55f251e99..edd6ab960f51cd8da638a626a2395b3a9ae692e4 100644 (file)
--- a/dpkg-deb/extract.c
+++ b/dpkg-deb/extract.c
@@ -63,10 +63,12 @@ static void readfail(FILE *a, const char *filename, const char *what) {
   }
 }
 
-static unsigned long parseheaderlength(const char *inh, size_t len,
-                                       const char *fn, const char *what) {
+static size_t
+parseheaderlength(const char *inh, size_t len,
+                  const char *fn, const char *what)
+{
   char lintbuf[15];
-  unsigned long r;
+  ssize_t r;
   char *endp;
 
   if (memchr(inh,0,len))
@@ -75,10 +77,12 @@ static unsigned long parseheaderlength(const char *inh, size_t len,
   memcpy(lintbuf,inh,len);
   lintbuf[len]= ' ';
   *strchr(lintbuf,' ')= 0;
-  r= strtoul(lintbuf,&endp,10);
+  r = strtol(lintbuf, &endp, 10);
+  if (r < 0)
+    ohshit(_("file `%.250s' is corrupt - negative member length %zi"), fn, r);
   if (*endp)
     ohshit(_("file `%.250s' is corrupt - bad digit (code %d) in %s"),fn,*endp,what);
-  return r;
+  return (size_t)r;
 }
 
 void extracthalf(const char *debar, const char *directory,
@@ -118,8 +122,6 @@ void extracthalf(const char *debar, const char *directory,
         ohshit(_("file `%.250s' is corrupt - bad magic at end of first header"),debar);
       memberlen= parseheaderlength(arh.ar_size,sizeof(arh.ar_size),
                                    debar,"member length");
-      if (memberlen<0)
-        ohshit(_("file `%.250s' is corrupt - negative member length %zi"),debar,memberlen);
       if (!header_done) {
         if (memcmp(arh.ar_name,"debian-binary   ",sizeof(arh.ar_name)) &&
            memcmp(arh.ar_name,"debian-binary/   ",sizeof(arh.ar_name)))

diff --git a/dpkg-deb/info.c b/dpkg-deb/info.c
index a76594818edaa8e4261a0d26c838e5b87f02cdb3..16fee858a838438cae38bae6f8b9403a0e9b52b6 100644 (file)
--- a/dpkg-deb/info.c
+++ b/dpkg-deb/info.c
@@ -96,7 +96,7 @@ static void info_spew(const char *debar, const char *directory,
     pathlen = strlen(directory) + strlen(component) + 2;
     controlfile = (void *) realloc((void *) controlfile, pathlen);
     if (!controlfile)
-      ohshite(_("realloc failed (%lu bytes)"), pathlen);
+      ohshite(_("realloc failed (%zu bytes)"), pathlen);
     memset(controlfile, 0, sizeof(controlfile));
 
     strcat(controlfile, directory);

diff --git a/dpkg-split/queue.c b/dpkg-split/queue.c
index 6a1312fa10625e2f23b77d7a29d313bba4f7ce98..342d19fdd970e119766fe463fc8180d6c0ee6452 100644 (file)
--- a/dpkg-split/queue.c
+++ b/dpkg-split/queue.c
@@ -97,8 +97,8 @@ void do_auto(const char *const *argv) {
   const char *partfile;
   struct partinfo *pi, *refi, *npi, **partlist, *otherthispart;
   struct partqueue *pq;
-  unsigned int i, j;
-  int ap;
+  unsigned int i;
+  int j, ap;
   long nr;
   FILE *part;
   void *buffer;
@@ -163,7 +163,7 @@ void do_auto(const char *const *argv) {
     /* There are still some parts missing. */
     for (i=0, ap=0; i<refi->maxpartn; i++)
       if (!partlist[i])
-        printf("%s%d", !ap++ ? "" : i==j ? _(" and ") : ", ", i+1);
+        printf("%s%d", !ap++ ? "" : i == (unsigned int)j ? _(" and ") : ", ", i + 1);
     printf(").\n");
 
   } else {

diff --git a/lib/varbuf.c b/lib/varbuf.c
index 3ef97f5442e8173ee13cee97031d17a16a2af088..9f8a6665904e13b6cf3b94ff92829aca347e3466 100644 (file)
--- a/lib/varbuf.c
+++ b/lib/varbuf.c
@@ -44,7 +44,8 @@ void varbufdupc(struct varbuf *v, int c, ssize_t n) {
 }
 
 int varbufprintf(struct varbuf *v, const char *fmt, ...) {
-  unsigned int ou, r;
+  size_t ou;
+  int r;
   va_list al;
 
   ou= v->used;
@@ -57,12 +58,13 @@ int varbufprintf(struct varbuf *v, const char *fmt, ...) {
     va_end(al);
     if (r < 0) r= (v->size-ou+1) * 2;
     v->used= ou+r;
-  } while (r >= v->size-ou-1);
+  } while (r >= (int)(v->size - ou - 1));
   return r;
 }
 
 int varbufvprintf(struct varbuf *v, const char *fmt, va_list va) {
-  unsigned int ou, r;
+  size_t ou;
+  int r;
   va_list al;
 
   ou= v->used;
@@ -74,7 +76,7 @@ int varbufvprintf(struct varbuf *v, const char *fmt, va_list va) {
     r= vsnprintf(v->buf+ou,v->size-ou,fmt,al);
     if (r < 0) r= (v->size-ou+1) * 2;
     v->used= ou+r;
-  } while (r >= v->size-ou-1);
+  } while (r >= (int)(v->size - ou - 1));
   return r;
 }
 

diff --git a/src/archives.c b/src/archives.c
index 64a104ef97191a92502857b9ffe9aa5032b471f6..71a1254672313c8e42101aa8cbed92b00137c976 100644 (file)
--- a/src/archives.c
+++ b/src/archives.c
@@ -374,7 +374,7 @@ int tarobject(struct TarInfo *ti) {
   struct conffile *conff;
   struct tarcontext *tc= (struct tarcontext*)ti->UserData;
   int statr, fd, i, existingdirectory, keepexisting;
-  size_t r;
+  ssize_t r;
   struct stat stab, stabtmp;
   char databuf[TARBLKSZ];
   struct fileinlist *nifd, **oldnifd;
@@ -765,7 +765,7 @@ int tarobject(struct TarInfo *ti) {
         varbufextend(&symlinkfn);
         r= readlink(fnamevb.buf,symlinkfn.buf,symlinkfn.size);
         if (r<0) ohshite(_("unable to read link `%.255s'"),ti->Name);
-      } while (r == symlinkfn.size);
+      } while ((size_t)r == symlinkfn.size);
       symlinkfn.used= r; varbufaddc(&symlinkfn,0);
       if (symlink(symlinkfn.buf,fnametmpvb.buf))
         ohshite(_("unable to make backup symlink for `%.255s'"),ti->Name);

diff --git a/src/main.c b/src/main.c
index 7fadab9fc90e8ac92358e50e055ba20b487937c4..866147888d5b788ed991537d77aeff96ec960b4d 100644 (file)
--- a/src/main.c
+++ b/src/main.c
@@ -352,7 +352,7 @@ static void setforce(const struct cmdinfo *cip, const char *value) {
 
   for (;;) {
     comma= strchr(value,',');
-    l= comma ? (int)(comma-value) : strlen(value);
+    l = comma ? (size_t)(comma - value) : strlen(value);
     for (fip=forceinfos; fip->name; fip++)
       if (!strncmp(fip->name,value,l) && strlen(fip->name)==l) break;
     if (!fip->name) {

diff --git a/utils/start-stop-daemon.c b/utils/start-stop-daemon.c
index b5cf162febe121af6d905f69e59ecf79335391bb..3140ceef66caa6260a5d9273d749259d56a7c636 100644 (file)
--- a/utils/start-stop-daemon.c
+++ b/utils/start-stop-daemon.c
@@ -455,7 +455,7 @@ parse_schedule(const char *schedule_str) {
        char item_buf[20];
        const char *slash;
        int count, repeatat;
-       ptrdiff_t str_len;
+       size_t str_len;
 
        count = 0;
        for (slash = schedule_str; *slash; slash++)
@@ -481,8 +481,8 @@ parse_schedule(const char *schedule_str) {
                repeatat = -1;
                while (schedule_str != NULL) {
                        slash = strchr(schedule_str,'/');
-                       str_len = slash ? slash - schedule_str : strlen(schedule_str);
-                       if (str_len >= (ptrdiff_t)sizeof(item_buf))
+                       str_len = slash ? (size_t)(slash - schedule_str) : strlen(schedule_str);
+                       if (str_len >= sizeof(item_buf))
                                badusage("invalid schedule item: far too long"
                                         " (you must delimit items with slashes)");
                        memcpy(item_buf, schedule_str, str_len);