[PATCH v2 00/13] compression code refactoring and error handling fixes

[Jonathan Nieder]

Hi dpkg team,

These patches clean up the libdpkg compression code a bit and make the
errors from dpkg-deb on compression errors a little more sane (no more
mysterious errors ending with ": Success").  Although they are not
strictly necessary for liblzma support, they helped my sanity in
preparing it.

This is a separate series because the liblzma support patch series was
getting a little long.  I imagine these could be reviewed separately.

I hope you like them.

Regards,
Jonathan

Jonathan Nieder (13):
  libdpkg: fix misspelling of __attribute__((const))
  libdpkg: Reduce scope of combuf[] in compress_cat
  libdpkg: factor out common gzip, bzip2 code
  libdpkg: fix redundant compression error messages
  libdpkg: fix compress_cat to allow I/O to files
  libdpkg: report read errors during (de)compression
  libdpkg: compression: do not handle EINTR
  libdpkg: check for write errors during decompression
  libdpkg: factor out function to execute compressors
  libdpkg: reorder internal fd_fd_filter() function arguments
  libdpkg: simplify code to calculate compression options
  libdpkg: compression: move each format's code into its own function
  libdpkg: compression: check for output errors closing files

 lib/dpkg/Makefile.am           |    1 +
 lib/dpkg/compression-backend.c |  248 ++++++++++++++++++++++++++++++++++++++++
 lib/dpkg/compression-backend.h |   35 ++++++
 lib/dpkg/compression.c         |  163 ++------------------------
 lib/dpkg/macros.h              |    2 +-
 5 files changed, 296 insertions(+), 153 deletions(-)
 create mode 100644 lib/dpkg/compression-backend.c
 create mode 100644 lib/dpkg/compression-backend.h


--
To UNSUBSCRIBE, email to debian-dpkg-REQUEST@...
with a subject of "unsubscribe". Trouble? Contact listmaster@...

[Jonathan Nieder]

gcc does not recognize __attribute__((constant)).

Signed-off-by: Jonathan Nieder <jrnieder@...>
---
 lib/dpkg/macros.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/lib/dpkg/macros.h b/lib/dpkg/macros.h
index 6bc600a..231b477 100644
--- a/lib/dpkg/macros.h
+++ b/lib/dpkg/macros.h
@@ -26,7 +26,7 @@
 
 #if HAVE_C_ATTRIBUTE
 #define DPKG_ATTR_UNUSED __attribute__((unused))
-#define DPKG_ATTR_CONST __attribute__((constant))
+#define DPKG_ATTR_CONST __attribute__((const))
 #define DPKG_ATTR_NORET __attribute__((noreturn))
 #define DPKG_ATTR_PRINTF(n) __attribute__((format(printf, n, n + 1)))
 #else
--
1.6.5.rc1.199.g596ec


--
To UNSUBSCRIBE, email to debian-dpkg-REQUEST@...
with a subject of "unsubscribe". Trouble? Contact listmaster@...

[Jonathan Nieder]

Move the definition of combuf[] closer to its actual usage.  This
should make it easier to factor out repeated code and saves some
stack space in cases where there is no command line argument to
compute.

Signed-off-by: Jonathan Nieder <jrnieder@...>
---
 lib/dpkg/compression.c |   30 ++++++++++++++++++++----------
 1 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/lib/dpkg/compression.c b/lib/dpkg/compression.c
index 2fa0476..f7444af 100644
--- a/lib/dpkg/compression.c
+++ b/lib/dpkg/compression.c
@@ -104,7 +104,6 @@ void decompress_cat(enum compress_type type, int fd_in, int fd_out, char *desc,
 void compress_cat(enum compress_type type, int fd_in, int fd_out, const char *compression, char *desc, ...) {
   va_list al;
   struct varbuf v = VARBUF_INIT;
-  char combuf[6];
 
   va_start(al,desc);
   varbufvprintf(&v, desc, al);
@@ -118,6 +117,7 @@ void compress_cat(enum compress_type type, int fd_in, int fd_out, const char *co
     case compress_type_gzip:
 #ifdef WITH_ZLIB
       {
+        char combuf[6];
         int actualread, actualwrite;
         char buffer[4096];
         gzFile gzfile;
@@ -146,13 +146,17 @@ void compress_cat(enum compress_type type, int fd_in, int fd_out, const char *co
         exit(0);
       }
 #else
-      strncpy(combuf, "-9c", sizeof(combuf));
-      combuf[1]= *compression;
-      fd_fd_filter(fd_in, fd_out, GZIP, "gzip", combuf, v.buf);
+      {
+        char combuf[6];
+        strncpy(combuf, "-9c", sizeof(combuf));
+        combuf[1]= *compression;
+        fd_fd_filter(fd_in, fd_out, GZIP, "gzip", combuf, v.buf);
+      }
 #endif
     case compress_type_bzip2:
 #ifdef WITH_BZ2
       {
+        char combuf[6];
         int actualread, actualwrite;
         char buffer[4096];
         BZFILE *bzfile;
@@ -181,14 +185,20 @@ void compress_cat(enum compress_type type, int fd_in, int fd_out, const char *co
         exit(0);
       }
 #else
-      strncpy(combuf, "-9c", sizeof(combuf));
-      combuf[1]= *compression;
-      fd_fd_filter(fd_in, fd_out, BZIP2, "bzip2", combuf, v.buf);
+      {
+        char combuf[6];
+        strncpy(combuf, "-9c", sizeof(combuf));
+        combuf[1]= *compression;
+        fd_fd_filter(fd_in, fd_out, BZIP2, "bzip2", combuf, v.buf);
+      }
 #endif
     case compress_type_lzma:
-      strncpy(combuf, "-9c", sizeof(combuf));
-      combuf[1] = *compression;
-      fd_fd_filter(fd_in, fd_out, LZMA, "lzma", combuf, v.buf);
+      {
+        char combuf[6];
+        strncpy(combuf, "-9c", sizeof(combuf));
+        combuf[1] = *compression;
+        fd_fd_filter(fd_in, fd_out, LZMA, "lzma", combuf, v.buf);
+      }
     case compress_type_cat:
       fd_fd_copy(fd_in, fd_out, -1, _("%s: compression"), v.buf);
       exit(0);
--
1.6.5.rc1.199.g596ec


--
To UNSUBSCRIBE, email to debian-dpkg-REQUEST@...
with a subject of "unsubscribe". Trouble? Contact listmaster@...

[Jonathan Nieder]

If only C had templates!  Without function templates, macros
like these two will have to do.

This patch refactors the callers for zlib and libbz2 library
functions to share some code, in preparation for some small
changes that apply to both.

Signed-off-by: Jonathan Nieder <jrnieder@...>
---
 lib/dpkg/compression.c |  165 +++++++++++++++++++++---------------------------
 1 files changed, 71 insertions(+), 94 deletions(-)

diff --git a/lib/dpkg/compression.c b/lib/dpkg/compression.c
index f7444af..22e1af7 100644
--- a/lib/dpkg/compression.c
+++ b/lib/dpkg/compression.c
@@ -36,6 +36,67 @@ fd_fd_filter(int fd_in, int fd_out,
   ohshite(_("%s: failed to exec '%s %s'"), desc, cmd, args);
 }
 
+#define DECOMPRESS(format, zFile, zdopen, zread, zerror, ERR_ERRNO, \
+                   fd_in, fd_out, desc) do \
+{ \
+  char buffer[4096]; \
+  int actualread; \
+  zFile zfile = zdopen(fd_in, "r"); \
+  \
+  while ((actualread = zread(zfile, buffer, sizeof(buffer))) > 0) { \
+    if (actualread < 0) { \
+      int err = 0; \
+      const char *errmsg = zerror(zfile, &err); \
+      if (err == ERR_ERRNO) { \
+        if (errno == EINTR) \
+          continue; \
+        errmsg = strerror(errno); \
+      } \
+      ohshite(_("%s: internal " format " error: `%s'"), desc, errmsg); \
+    } \
+    write(fd_out, buffer, actualread); \
+  } \
+  exit(0); \
+} while(0)
+
+#define COMPRESS(format, zFile, zdopen, zwrite, zclose, zerror, ERR_ERRNO, \
+                 fd_in, fd_out, compression, desc) do \
+{ \
+  char combuf[6]; \
+  int actualread, actualwrite; \
+  char buffer[4096]; \
+  zFile zfile; \
+  \
+  strncpy(combuf, "w9", sizeof(combuf)); \
+  combuf[1] = compression; \
+  zfile = zdopen(1, combuf); \
+  while ((actualread = read(0, buffer, sizeof(buffer))) > 0) { \
+    if (actualread < 0) { \
+      if (errno == EINTR) \
+        continue; \
+      ohshite(_("%s: internal " format " error: read: `%s'"), \
+              desc, strerror(errno)); \
+    } \
+    actualwrite = zwrite(zfile, buffer, actualread); \
+    if (actualwrite < 0) { \
+      int err = 0; \
+      const char *errmsg = zerror(zfile, &err); \
+      if (err == ERR_ERRNO) { \
+        if (errno == EINTR) \
+          continue; \
+        errmsg = strerror(errno); \
+      } \
+      ohshite(_("%s: internal " format " error: write: `%s'"), \
+              desc, errmsg); \
+    } \
+    if (actualwrite != actualread) \
+      ohshite(_("%s: internal " format " error: read(%i) != write(%i)"), \
+              desc, actualread, actualwrite); \
+  } \
+  zclose(zfile); \
+  exit(0); \
+} while(0)
+
 void decompress_cat(enum compress_type type, int fd_in, int fd_out, char *desc, ...) {
   va_list al;
   struct varbuf v = VARBUF_INIT;
@@ -47,47 +108,16 @@ void decompress_cat(enum compress_type type, int fd_in, int fd_out, char *desc,
   switch(type) {
     case compress_type_gzip:
 #ifdef WITH_ZLIB
-      {
-        char buffer[4096];
-        int actualread;
-        gzFile gzfile = gzdopen(fd_in, "r");
-        while ((actualread= gzread(gzfile,buffer,sizeof(buffer))) > 0) {
-          if (actualread < 0 ) {
-            int err = 0;
-            const char *errmsg = gzerror(gzfile, &err);
-            if (err == Z_ERRNO) {
-              if (errno == EINTR) continue;
-              errmsg= strerror(errno);
-            }
-          ohshite(_("%s: internal gzip error: `%s'"), v.buf, errmsg);
-          }
-          write(fd_out,buffer,actualread);
-        }
-      }
-      exit(0);
+      DECOMPRESS("gzip", gzFile, gzdopen, gzread, gzerror, Z_ERRNO,
+                 fd_in, fd_out, v.buf);
 #else
       fd_fd_filter(fd_in, fd_out, GZIP, "gzip", "-dc", v.buf);
 #endif
     case compress_type_bzip2:
 #ifdef WITH_BZ2
-      {  
-        char buffer[4096];
-        int actualread;
-        BZFILE *bzfile = BZ2_bzdopen(fd_in, "r");
-        while ((actualread= BZ2_bzread(bzfile,buffer,sizeof(buffer))) > 0) {
-          if (actualread < 0 ) {
-            int err = 0;
-            const char *errmsg = BZ2_bzerror(bzfile, &err);
-            if (err == BZ_IO_ERROR) {
-              if (errno == EINTR) continue;
-              errmsg= strerror(errno);
-            }
-          ohshite(_("%s: internal bzip2 error: `%s'"), v.buf, errmsg);
-          }
-          write(fd_out,buffer,actualread);
-        }
-      }
-      exit(0);
+      DECOMPRESS("bzip2", BZFILE *, BZ2_bzdopen, BZ2_bzread,
+                 BZ2_bzerror, BZ_IO_ERROR,
+                 fd_in, fd_out, v.buf);
 #else
       fd_fd_filter(fd_in, fd_out, BZIP2, "bzip2", "-dc", v.buf);
 #endif
@@ -116,35 +146,8 @@ void compress_cat(enum compress_type type, int fd_in, int fd_out, const char *co
   switch(type) {
     case compress_type_gzip:
 #ifdef WITH_ZLIB
-      {
-        char combuf[6];
-        int actualread, actualwrite;
-        char buffer[4096];
-        gzFile gzfile;
-        strncpy(combuf, "w9", sizeof(combuf));
-        combuf[1]= *compression;
-        gzfile = gzdopen(1, combuf);
-        while((actualread = read(0,buffer,sizeof(buffer))) > 0) {
-          if (actualread < 0 ) {
-            if (errno == EINTR) continue;
-            ohshite(_("%s: internal gzip error: read: `%s'"), v.buf, strerror(errno));
-          }
-          actualwrite= gzwrite(gzfile,buffer,actualread);
-          if (actualwrite < 0 ) {
-            int err = 0;
-            const char *errmsg = gzerror(gzfile, &err);
-            if (err == Z_ERRNO) {
-              if (errno == EINTR) continue;
-            errmsg= strerror(errno);
-            }
-            ohshite(_("%s: internal gzip error: write: `%s'"), v.buf, errmsg);
-          }
-          if (actualwrite != actualread)
-            ohshite(_("%s: internal gzip error: read(%i) != write(%i)"), v.buf, actualread, actualwrite);
-        }
-        gzclose(gzfile);
-        exit(0);
-      }
+      COMPRESS("gzip", gzFile, gzdopen, gzwrite, gzclose, gzerror, Z_ERRNO,
+               fd_in, fd_out, *compression, v.buf);
 #else
       {
         char combuf[6];
@@ -155,35 +158,9 @@ void compress_cat(enum compress_type type, int fd_in, int fd_out, const char *co
 #endif
     case compress_type_bzip2:
 #ifdef WITH_BZ2
-      {
-        char combuf[6];
-        int actualread, actualwrite;
-        char buffer[4096];
-        BZFILE *bzfile;
-        strncpy(combuf, "w9", sizeof(combuf));
-        combuf[1]= *compression;
-        bzfile = BZ2_bzdopen(1, combuf);
-        while((actualread = read(0,buffer,sizeof(buffer))) > 0) {
-          if (actualread < 0 ) {
-            if (errno == EINTR) continue;
-            ohshite(_("%s: internal bzip2 error: read: `%s'"), v.buf, strerror(errno));
-          }
-          actualwrite= BZ2_bzwrite(bzfile,buffer,actualread);
-          if (actualwrite < 0 ) {
-            int err = 0;
-            const char *errmsg = BZ2_bzerror(bzfile, &err);
-            if (err == BZ_IO_ERROR) {
-              if (errno == EINTR) continue;
-              errmsg= strerror(errno);
-            }
-            ohshite(_("%s: internal bzip2 error: write: `%s'"), v.buf, errmsg);
-          }
-          if (actualwrite != actualread)
-            ohshite(_("%s: internal bzip2 error: read(%i) != write(%i)"), v.buf, actualread, actualwrite);
-        }
-        BZ2_bzclose(bzfile);
-        exit(0);
-      }
+      COMPRESS("bzip2", BZFILE *, BZ2_bzdopen, BZ2_bzwrite, BZ2_bzclose,
+               BZ2_bzerror, BZ_IO_ERROR,
+               fd_in, fd_out, *compression, v.buf);
 #else
       {
         char combuf[6];
--
1.6.5.rc1.199.g596ec


--
To UNSUBSCRIBE, email to debian-dpkg-REQUEST@...
with a subject of "unsubscribe". Trouble? Contact listmaster@...

[Jonathan Nieder]

Avoid ohshite() except where errno is useful.  This should avoid
confusing error messages like

        data: internal gzip error: read: stream error: Success

While we're at it, drop the number of bytes read and written from
the read() != write() error message and output the error message
from the compression library instead.

Signed-off-by: Jonathan Nieder <jrnieder@...>
---
 lib/dpkg/compression.c |   16 +++++++---------
 1 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/lib/dpkg/compression.c b/lib/dpkg/compression.c
index 22e1af7..c80a9b1 100644
--- a/lib/dpkg/compression.c
+++ b/lib/dpkg/compression.c
@@ -52,7 +52,8 @@ fd_fd_filter(int fd_in, int fd_out,
           continue; \
         errmsg = strerror(errno); \
       } \
-      ohshite(_("%s: internal " format " error: `%s'"), desc, errmsg); \
+      ohshit(_("%s: internal " format " error: %s: %s"), \
+             desc, "read", errmsg); \
     } \
     write(fd_out, buffer, actualread); \
   } \
@@ -74,11 +75,11 @@ fd_fd_filter(int fd_in, int fd_out,
     if (actualread < 0) { \
       if (errno == EINTR) \
         continue; \
-      ohshite(_("%s: internal " format " error: read: `%s'"), \
-              desc, strerror(errno)); \
+      ohshite(_("%s: internal " format " error: %s"), \
+              desc, "read"); \
     } \
     actualwrite = zwrite(zfile, buffer, actualread); \
-    if (actualwrite < 0) { \
+    if (actualwrite != actualread) { \
       int err = 0; \
       const char *errmsg = zerror(zfile, &err); \
       if (err == ERR_ERRNO) { \
@@ -86,12 +87,9 @@ fd_fd_filter(int fd_in, int fd_out,
           continue; \
         errmsg = strerror(errno); \
       } \
-      ohshite(_("%s: internal " format " error: write: `%s'"), \
-              desc, errmsg); \
+      ohshit(_("%s: internal " format " error: %s: %s"), \
+              desc, "write", errmsg); \
     } \
-    if (actualwrite != actualread) \
-      ohshite(_("%s: internal " format " error: read(%i) != write(%i)"), \
-              desc, actualread, actualwrite); \
   } \
   zclose(zfile); \
   exit(0); \
--
1.6.5.rc1.199.g596ec


--
To UNSUBSCRIBE, email to debian-dpkg-REQUEST@...
with a subject of "unsubscribe". Trouble? Contact listmaster@...

[Jonathan Nieder]

The compress_cat() code had a hidden assumption that fd_in is 0
and fd_out is 1.  This is a bug waiting to happen.  But luckily,
all callers do use those values, so it is harmless.

Signed-off-by: Jonathan Nieder <jrnieder@...>
---
 lib/dpkg/compression.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/dpkg/compression.c b/lib/dpkg/compression.c
index c80a9b1..f1b6836 100644
--- a/lib/dpkg/compression.c
+++ b/lib/dpkg/compression.c
@@ -70,8 +70,8 @@ fd_fd_filter(int fd_in, int fd_out,
   \
   strncpy(combuf, "w9", sizeof(combuf)); \
   combuf[1] = compression; \
-  zfile = zdopen(1, combuf); \
-  while ((actualread = read(0, buffer, sizeof(buffer))) > 0) { \
+  zfile = zdopen(fd_out, combuf); \
+  while ((actualread = read(fd_in, buffer, sizeof(buffer))) > 0) { \
     if (actualread < 0) { \
       if (errno == EINTR) \
         continue; \
--
1.6.5.rc1.199.g596ec


--
To UNSUBSCRIBE, email to debian-dpkg-REQUEST@...
with a subject of "unsubscribe". Trouble? Contact listmaster@...

[Jonathan Nieder]

Instead of using the error handling code, failed reads are being
treated as end of file.  This applies only when using zlib and
libbz2.  In practice it probably has not caused problems
because I/O errors are rare, and often the program at the other
end of the pipe can notice the pipe unexpectedly closing.

Signed-off-by: Jonathan Nieder <jrnieder@...>
---
 lib/dpkg/compression.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/dpkg/compression.c b/lib/dpkg/compression.c
index f1b6836..31da38d 100644
--- a/lib/dpkg/compression.c
+++ b/lib/dpkg/compression.c
@@ -43,7 +43,7 @@ fd_fd_filter(int fd_in, int fd_out,
   int actualread; \
   zFile zfile = zdopen(fd_in, "r"); \
   \
-  while ((actualread = zread(zfile, buffer, sizeof(buffer))) > 0) { \
+  while ((actualread = zread(zfile, buffer, sizeof(buffer)))) { \
     if (actualread < 0) { \
       int err = 0; \
       const char *errmsg = zerror(zfile, &err); \
@@ -71,7 +71,7 @@ fd_fd_filter(int fd_in, int fd_out,
   strncpy(combuf, "w9", sizeof(combuf)); \
   combuf[1] = compression; \
   zfile = zdopen(fd_out, combuf); \
-  while ((actualread = read(fd_in, buffer, sizeof(buffer))) > 0) { \
+  while ((actualread = read(fd_in, buffer, sizeof(buffer)))) { \
     if (actualread < 0) { \
       if (errno == EINTR) \
         continue; \
--
1.6.5.rc1.199.g596ec


--
To UNSUBSCRIBE, email to debian-dpkg-REQUEST@...
with a subject of "unsubscribe". Trouble? Contact listmaster@...

[Jonathan Nieder]

The current callers for the compression code do not install signal
handlers, so there is no occasion to test the EINTR handling.
Perhaps for this reason, since commit 7bf6e0 (add support for using
libz, 2000-12-09) when the current compression/decompression code
was introduced, the EINTR handling has been broken in a number of
ways:

 * interrupted reads were treated as end of file until very
   recently
 * interrupted writes during decompression cause portions of the
   output to be discarded
 * interrupted writes during compression are treated as errors,
   unless the interruption happens before any data from the
   output buffer can be consumed

Since zlib at least cannot recover from an interrupted write
anyway, it seems better to always treat EINTR like any other error.
Callers should specify the SA_RESTART flag when installing signal
handlers for correct behavior on System V style operating systems
(such as Solaris).

Signed-off-by: Jonathan Nieder <jrnieder@...>
---
 lib/dpkg/compression.c |   18 ++++++------------
 1 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/lib/dpkg/compression.c b/lib/dpkg/compression.c
index 31da38d..821c0a4 100644
--- a/lib/dpkg/compression.c
+++ b/lib/dpkg/compression.c
@@ -47,11 +47,9 @@ fd_fd_filter(int fd_in, int fd_out,
     if (actualread < 0) { \
       int err = 0; \
       const char *errmsg = zerror(zfile, &err); \
-      if (err == ERR_ERRNO) { \
-        if (errno == EINTR) \
-          continue; \
+      \
+      if (err == ERR_ERRNO) \
         errmsg = strerror(errno); \
-      } \
       ohshit(_("%s: internal " format " error: %s: %s"), \
              desc, "read", errmsg); \
     } \
@@ -72,21 +70,17 @@ fd_fd_filter(int fd_in, int fd_out,
   combuf[1] = compression; \
   zfile = zdopen(fd_out, combuf); \
   while ((actualread = read(fd_in, buffer, sizeof(buffer)))) { \
-    if (actualread < 0) { \
-      if (errno == EINTR) \
-        continue; \
+    if (actualread < 0) \
       ohshite(_("%s: internal " format " error: %s"), \
               desc, "read"); \
-    } \
+    \
     actualwrite = zwrite(zfile, buffer, actualread); \
     if (actualwrite != actualread) { \
       int err = 0; \
       const char *errmsg = zerror(zfile, &err); \
-      if (err == ERR_ERRNO) { \
-        if (errno == EINTR) \
-          continue; \
+      \
+      if (err == ERR_ERRNO) \
         errmsg = strerror(errno); \
-      } \
       ohshit(_("%s: internal " format " error: %s: %s"), \
               desc, "write", errmsg); \
     } \
--
1.6.5.rc1.199.g596ec


--
To UNSUBSCRIBE, email to debian-dpkg-REQUEST@...
with a subject of "unsubscribe". Trouble? Contact listmaster@...

[Jonathan Nieder]

An unnoticed write error is unlikely to cause major problems,
since the process on the other end still has a chance to notice
the mangled stream.  But it is worth fixing, especially because
the writing end can give a better error message.

Signed-off-by: Jonathan Nieder <jrnieder@...>
---
 lib/dpkg/compression.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/lib/dpkg/compression.c b/lib/dpkg/compression.c
index 821c0a4..f901867 100644
--- a/lib/dpkg/compression.c
+++ b/lib/dpkg/compression.c
@@ -44,6 +44,8 @@ fd_fd_filter(int fd_in, int fd_out,
   zFile zfile = zdopen(fd_in, "r"); \
   \
   while ((actualread = zread(zfile, buffer, sizeof(buffer)))) { \
+    int actualwrite; \
+    \
     if (actualread < 0) { \
       int err = 0; \
       const char *errmsg = zerror(zfile, &err); \
@@ -53,7 +55,11 @@ fd_fd_filter(int fd_in, int fd_out,
       ohshit(_("%s: internal " format " error: %s: %s"), \
              desc, "read", errmsg); \
     } \
-    write(fd_out, buffer, actualread); \
+    \
+    actualwrite = write(fd_out, buffer, actualread); \
+    if (actualwrite != actualread) \
+      ohshite(_("%s: internal " format " error: %s"), \
+              desc, "write"); \
   } \
   exit(0); \
 } while(0)
--
1.6.5.rc1.199.g596ec


--
To UNSUBSCRIBE, email to debian-dpkg-REQUEST@...
with a subject of "unsubscribe". Trouble? Contact listmaster@...

[Jonathan Nieder]

gzip, bzip2, and lzma use the same command-line syntax.  Factor
out a compress_cmd() function that can be used to invoke any one
of these commands.

Signed-off-by: Jonathan Nieder <jrnieder@...>
---
 lib/dpkg/compression.c |   32 ++++++++++++++------------------
 1 files changed, 14 insertions(+), 18 deletions(-)

diff --git a/lib/dpkg/compression.c b/lib/dpkg/compression.c
index f901867..914ba1d 100644
--- a/lib/dpkg/compression.c
+++ b/lib/dpkg/compression.c
@@ -95,6 +95,17 @@ fd_fd_filter(int fd_in, int fd_out,
   exit(0); \
 } while(0)
 
+static void
+compress_cmd(int fd_in, int fd_out, const char *path,
+             const char *cmd, char compression,
+             const char *desc)
+{
+  char combuf[6];
+  strncpy(combuf, "-9c", sizeof(combuf));
+  combuf[1] = compression;
+  fd_fd_filter(fd_in, fd_out, path, cmd, combuf, desc);
+}
+
 void decompress_cat(enum compress_type type, int fd_in, int fd_out, char *desc, ...) {
   va_list al;
   struct varbuf v = VARBUF_INIT;
@@ -147,12 +158,7 @@ void compress_cat(enum compress_type type, int fd_in, int fd_out, const char *co
       COMPRESS("gzip", gzFile, gzdopen, gzwrite, gzclose, gzerror, Z_ERRNO,
                fd_in, fd_out, *compression, v.buf);
 #else
-      {
-        char combuf[6];
-        strncpy(combuf, "-9c", sizeof(combuf));
-        combuf[1]= *compression;
-        fd_fd_filter(fd_in, fd_out, GZIP, "gzip", combuf, v.buf);
-      }
+      compress_cmd(fd_in, fd_out, GZIP, "gzip", *compression, v.buf);
 #endif
     case compress_type_bzip2:
 #ifdef WITH_BZ2
@@ -160,20 +166,10 @@ void compress_cat(enum compress_type type, int fd_in, int fd_out, const char *co
                BZ2_bzerror, BZ_IO_ERROR,
                fd_in, fd_out, *compression, v.buf);
 #else
-      {
-        char combuf[6];
-        strncpy(combuf, "-9c", sizeof(combuf));
-        combuf[1]= *compression;
-        fd_fd_filter(fd_in, fd_out, BZIP2, "bzip2", combuf, v.buf);
-      }
+      compress_cmd(fd_in, fd_out, BZIP2, "bzip2", *compression, v.buf);
 #endif
     case compress_type_lzma:
-      {
-        char combuf[6];
-        strncpy(combuf, "-9c", sizeof(combuf));
-        combuf[1] = *compression;
-        fd_fd_filter(fd_in, fd_out, LZMA, "lzma", combuf, v.buf);
-      }
+      compress_cmd(fd_in, fd_out, LZMA, "lzma", *compression, v.buf);
     case compress_type_cat:
       fd_fd_copy(fd_in, fd_out, -1, _("%s: compression"), v.buf);
       exit(0);
--
1.6.5.rc1.199.g596ec


--
To UNSUBSCRIBE, email to debian-dpkg-REQUEST@...
with a subject of "unsubscribe". Trouble? Contact listmaster@...

[Jonathan Nieder]

Put the compression options string last so it can be replaced with
printf-style format string.

Signed-off-by: Jonathan Nieder <jrnieder@...>
---
 lib/dpkg/compression.c |   13 ++++++-------
 1 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/lib/dpkg/compression.c b/lib/dpkg/compression.c
index 914ba1d..a2fbdb3 100644
--- a/lib/dpkg/compression.c
+++ b/lib/dpkg/compression.c
@@ -20,9 +20,8 @@
 #include <dpkg/buffer.h>
 
 static void
-fd_fd_filter(int fd_in, int fd_out,
-     const char *file, const char *cmd, const char *args,
-     const char *desc)
+fd_fd_filter(int fd_in, int fd_out, const char *desc,
+             const char *file, const char *cmd, const char *args)
 {
   if (fd_in != 0) {
     m_dup2(fd_in, 0);
@@ -103,7 +102,7 @@ compress_cmd(int fd_in, int fd_out, const char *path,
   char combuf[6];
   strncpy(combuf, "-9c", sizeof(combuf));
   combuf[1] = compression;
-  fd_fd_filter(fd_in, fd_out, path, cmd, combuf, desc);
+  fd_fd_filter(fd_in, fd_out, desc, path, cmd, combuf);
 }
 
 void decompress_cat(enum compress_type type, int fd_in, int fd_out, char *desc, ...) {
@@ -120,7 +119,7 @@ void decompress_cat(enum compress_type type, int fd_in, int fd_out, char *desc,
       DECOMPRESS("gzip", gzFile, gzdopen, gzread, gzerror, Z_ERRNO,
                  fd_in, fd_out, v.buf);
 #else
-      fd_fd_filter(fd_in, fd_out, GZIP, "gzip", "-dc", v.buf);
+      fd_fd_filter(fd_in, fd_out, v.buf, GZIP, "gzip", "-dc");
 #endif
     case compress_type_bzip2:
 #ifdef WITH_BZ2
@@ -128,10 +127,10 @@ void decompress_cat(enum compress_type type, int fd_in, int fd_out, char *desc,
                  BZ2_bzerror, BZ_IO_ERROR,
                  fd_in, fd_out, v.buf);
 #else
-      fd_fd_filter(fd_in, fd_out, BZIP2, "bzip2", "-dc", v.buf);
+      fd_fd_filter(fd_in, fd_out, v.buf, BZIP2, "bzip2", "-dc");
 #endif
     case compress_type_lzma:
-      fd_fd_filter(fd_in, fd_out, LZMA, "lzma", "-dc", v.buf);
+      fd_fd_filter(fd_in, fd_out, v.buf, LZMA, "lzma", "-dc");
     case compress_type_cat:
       fd_fd_copy(fd_in, fd_out, -1, _("%s: decompression"), v.buf);
       exit(0);
--
1.6.5.rc1.199.g596ec


--
To UNSUBSCRIBE, email to debian-dpkg-REQUEST@...
with a subject of "unsubscribe". Trouble? Contact listmaster@...

[Jonathan Nieder]

Rather than making the reader track down the meaning of indices
into a fixed-size buffer as it is modified, calculate the
compression options all at once.

Signed-off-by: Jonathan Nieder <jrnieder@...>
---
 lib/dpkg/compression.c |   26 ++++++++++++++++----------
 1 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/lib/dpkg/compression.c b/lib/dpkg/compression.c
index a2fbdb3..33b0922 100644
--- a/lib/dpkg/compression.c
+++ b/lib/dpkg/compression.c
@@ -21,8 +21,16 @@
 
 static void
 fd_fd_filter(int fd_in, int fd_out, const char *desc,
-             const char *file, const char *cmd, const char *args)
+             const char *file, const char *cmd,
+             const char *argfmt, ...)
 {
+  struct varbuf argbuf = VARBUF_INIT;
+  va_list ap;
+
+  va_start(ap, argfmt);
+  varbufvprintf(&argbuf, argfmt, ap);
+  va_end(ap);
+
   if (fd_in != 0) {
     m_dup2(fd_in, 0);
     close(fd_in);
@@ -31,8 +39,11 @@ fd_fd_filter(int fd_in, int fd_out, const char *desc,
     m_dup2(fd_out, 1);
     close(fd_out);
   }
-  execlp(file, cmd, args, NULL);
-  ohshite(_("%s: failed to exec '%s %s'"), desc, cmd, args);
+
+  execlp(file, cmd, argbuf.buf, NULL);
+  ohshite(_("%s: failed to exec '%s %s'"), desc, cmd, argbuf.buf);
+
+  varbuffree(&argbuf);
 }
 
 #define DECOMPRESS(format, zFile, zdopen, zread, zerror, ERR_ERRNO, \
@@ -66,13 +77,11 @@ fd_fd_filter(int fd_in, int fd_out, const char *desc,
 #define COMPRESS(format, zFile, zdopen, zwrite, zclose, zerror, ERR_ERRNO, \
                  fd_in, fd_out, compression, desc) do \
 { \
-  char combuf[6]; \
+  char combuf[] = {'w', compression, '\0'}; \
   int actualread, actualwrite; \
   char buffer[4096]; \
   zFile zfile; \
   \
-  strncpy(combuf, "w9", sizeof(combuf)); \
-  combuf[1] = compression; \
   zfile = zdopen(fd_out, combuf); \
   while ((actualread = read(fd_in, buffer, sizeof(buffer)))) { \
     if (actualread < 0) \
@@ -99,10 +108,7 @@ compress_cmd(int fd_in, int fd_out, const char *path,
              const char *cmd, char compression,
              const char *desc)
 {
-  char combuf[6];
-  strncpy(combuf, "-9c", sizeof(combuf));
-  combuf[1] = compression;
-  fd_fd_filter(fd_in, fd_out, desc, path, cmd, combuf);
+  fd_fd_filter(fd_in, fd_out, desc, path, cmd, "-c%c", compression);
 }
 
 void decompress_cat(enum compress_type type, int fd_in, int fd_out, char *desc, ...) {
--
1.6.5.rc1.199.g596ec


--
To UNSUBSCRIBE, email to debian-dpkg-REQUEST@...
with a subject of "unsubscribe". Trouble? Contact listmaster@...

[Jonathan Nieder]

Add compress_gzip, decompress_gzip, etc functions with code from
compress_cat and decompress_cat.  Instead of

        switch (type) {
        case compress_type_gzip:
        #ifdef WITH_ZLIB
                /* decompress using zlib */
        #else
                fd_fd_filter(fd_in, fd_out, GZIP, "gzip", "-dc", v.buf);
        #endif

we write

        switch(type) {
        case compress_type_gzip:
                decompress_gzip(...);

with decompress_gzip defined appropriately depending on the value
of WITH_ZLIB.  This should make each function easier to read on
its own.

Signed-off-by: Jonathan Nieder <jrnieder@...>
---
 lib/dpkg/Makefile.am           |    1 +
 lib/dpkg/compression-backend.c |  207 ++++++++++++++++++++++++++++++++++++++++
 lib/dpkg/compression-backend.h |   35 +++++++
 lib/dpkg/compression.c         |  149 ++--------------------------
 4 files changed, 254 insertions(+), 138 deletions(-)
 create mode 100644 lib/dpkg/compression-backend.c
 create mode 100644 lib/dpkg/compression-backend.h

diff --git a/lib/dpkg/Makefile.am b/lib/dpkg/Makefile.am
index 4b31209..7428f7c 100644
--- a/lib/dpkg/Makefile.am
+++ b/lib/dpkg/Makefile.am
@@ -22,6 +22,7 @@ libdpkg_a_SOURCES = \
  buffer.c buffer.h \
  cleanup.c \
  compression.c \
+ compression-backend.c compression-backend.h \
  database.c \
  dbmodify.c \
  dump.c \
diff --git a/lib/dpkg/compression-backend.c b/lib/dpkg/compression-backend.c
new file mode 100644
index 0000000..9bfbcee
--- /dev/null
+++ b/lib/dpkg/compression-backend.c
@@ -0,0 +1,207 @@
+#include <config.h>
+#include <compat.h>
+
+#include <dpkg/i18n.h>
+
+#include <stdarg.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <errno.h>
+
+#ifdef WITH_ZLIB
+#include <zlib.h>
+#endif
+#ifdef WITH_BZ2
+#include <bzlib.h>
+#endif
+
+#include <dpkg/dpkg.h>
+#include <dpkg/varbuf.h>
+#include <dpkg/buffer.h>
+#include <dpkg/macros.h>
+
+#include "compression-backend.h"
+
+static void fd_fd_filter(int fd_in, int fd_out, const char *desc,
+ const char *file, const char *cmd, const char *argfmt, ...)
+ DPKG_ATTR_NORET DPKG_ATTR_PRINTF(6);
+
+static void compress_cmd(int fd_in, int fd_out, const char *path,
+ const char *cmd, char compression, const char *desc)
+ DPKG_ATTR_NORET;
+
+static void
+fd_fd_filter(int fd_in, int fd_out, const char *desc,
+ const char *file, const char *cmd,
+ const char *argfmt, ...)
+{
+ struct varbuf argbuf = VARBUF_INIT;
+ va_list ap;
+
+ va_start(ap, argfmt);
+ varbufvprintf(&argbuf, argfmt, ap);
+ va_end(ap);
+
+ if (fd_in != 0) {
+ m_dup2(fd_in, 0);
+ close(fd_in);
+ }
+ if (fd_out != 1) {
+ m_dup2(fd_out, 1);
+ close(fd_out);
+ }
+
+ execlp(file, cmd, argbuf.buf, NULL);
+ ohshite(_("%s: failed to exec '%s %s'"), desc, cmd, argbuf.buf);
+
+ varbuffree(&argbuf);
+}
+
+#define DECOMPRESS(format, zFile, zdopen, zread, zerror, ERR_ERRNO, \
+ fd_in, fd_out, desc) do \
+{ \
+ char buffer[4096]; \
+ int actualread; \
+ zFile zfile = zdopen(fd_in, "r"); \
+ \
+ while ((actualread = zread(zfile, buffer, sizeof(buffer)))) { \
+ int actualwrite; \
+ \
+ if (actualread < 0) { \
+ int err = 0; \
+ const char *errmsg = zerror(zfile, &err); \
+ \
+ if (err == ERR_ERRNO) \
+ errmsg = strerror(errno); \
+ ohshit(_("%s: internal " format " error: %s: %s"), \
+       desc, "read", errmsg); \
+ } \
+ \
+ actualwrite = write(fd_out, buffer, actualread); \
+ if (actualwrite != actualread) \
+ ohshite(_("%s: internal " format " error: %s"), \
+ desc, "write"); \
+ } \
+ exit(0); \
+} while(0)
+
+#define COMPRESS(format, zFile, zdopen, zwrite, zclose, zerror, ERR_ERRNO, \
+ fd_in, fd_out, compression, desc) do \
+{ \
+ char combuf[] = {'w', compression, '\0'}; \
+ int actualread, actualwrite; \
+ char buffer[4096]; \
+ zFile zfile; \
+ \
+ zfile = zdopen(fd_out, combuf); \
+ while ((actualread = read(fd_in, buffer, sizeof(buffer)))) { \
+ if (actualread < 0) \
+ ohshite(_("%s: internal " format " error: %s"), \
+        desc, "read"); \
+ \
+ actualwrite = zwrite(zfile, buffer, actualread); \
+ if (actualwrite != actualread) { \
+ int err = 0; \
+ const char *errmsg = zerror(zfile, &err); \
+ \
+ if (err == ERR_ERRNO) \
+ errmsg = strerror(errno); \
+ ohshit(_("%s: internal " format " error: %s: %s"), \
+        desc, "write", errmsg); \
+ } \
+ } \
+ zclose(zfile); \
+ exit(0); \
+} while(0)
+
+static void
+compress_cmd(int fd_in, int fd_out, const char *path,
+ const char *cmd, char compression, const char *desc)
+{
+ fd_fd_filter(fd_in, fd_out, desc, path, cmd, "-c%c", compression);
+}
+
+#ifdef WITH_ZLIB
+void
+decompress_gzip(int fd_in, int fd_out, const char *desc)
+{
+ DECOMPRESS("gzip", gzFile, gzdopen, gzread, gzerror, Z_ERRNO,
+ fd_in, fd_out, desc);
+}
+
+void
+compress_gzip(int fd_in, int fd_out, char compression, const char *desc)
+{
+ COMPRESS("gzip", gzFile, gzdopen, gzwrite, gzclose, gzerror, Z_ERRNO,
+ fd_in, fd_out, compression, desc);
+}
+#else /* !WITH_ZLIB */
+void
+decompress_gzip(int fd_in, int fd_out, const char *desc)
+{
+ fd_fd_filter(fd_in, fd_out, desc, GZIP, "gzip", "-dc");
+}
+
+void
+compress_gzip(int fd_in, int fd_out, char compression, const char *desc)
+{
+ compress_cmd(fd_in, fd_out, GZIP, "gzip", compression, desc);
+}
+#endif
+
+#ifdef WITH_BZ2
+void
+decompress_bzip2(int fd_in, int fd_out, const char *desc)
+{
+ DECOMPRESS("bzip2", BZFILE *, BZ2_bzdopen, BZ2_bzread,
+ BZ2_bzerror, BZ_IO_ERROR,
+ fd_in, fd_out, desc);
+}
+
+void
+compress_bzip2(int fd_in, int fd_out, char compression, const char *desc)
+{
+ COMPRESS("bzip2", BZFILE *, BZ2_bzdopen, BZ2_bzwrite, BZ2_bzclose,
+ BZ2_bzerror, BZ_IO_ERROR,
+ fd_in, fd_out, compression, desc);
+}
+#else /* !WITH_BZ2 */
+void
+decompress_bzip2(int fd_in, int fd_out, const char *desc)
+{
+ fd_fd_filter(fd_in, fd_out, desc, BZIP2, "bzip2", "-dc");
+}
+
+void
+compress_bzip2(int fd_in, int fd_out, char compression, const char *desc)
+{
+ compress_cmd(fd_in, fd_out, BZIP2, "bzip2", compression, desc);
+}
+#endif
+
+void
+decompress_lzma(int fd_in, int fd_out, const char *desc)
+{
+ fd_fd_filter(fd_in, fd_out, desc, LZMA, "lzma", "-dc");
+}
+
+void
+compress_lzma(int fd_in, int fd_out, char compression, const char *desc)
+{
+ compress_cmd(fd_in, fd_out, LZMA, "lzma", compression, desc);
+}
+
+void
+decompress_noop(int fd_in, int fd_out, const char *desc)
+{
+ fd_fd_copy(fd_in, fd_out, -1, _("%s: decompression"), desc);
+ exit(0);
+}
+
+void
+compress_noop(int fd_in, int fd_out, const char *desc)
+{
+ fd_fd_copy(fd_in, fd_out, -1, _("%s: compression"), desc);
+ exit(0);
+}
diff --git a/lib/dpkg/compression-backend.h b/lib/dpkg/compression-backend.h
new file mode 100644
index 0000000..7f3c5d6
--- /dev/null
+++ b/lib/dpkg/compression-backend.h
@@ -0,0 +1,35 @@
+/*
+ * libdpkg - Debian packaging suite library functions
+ * compression_backend.h - internal functions to compress and
+ *                         decompress archives
+ *
+ * See dpkg.h for the public interface (compress_cat/decompress_cat).
+ */
+
+#ifndef COMPRESSION_BACKEND_H
+#define COMPRESSION_BACKEND_H
+
+#include <config.h>
+#include <compat.h>
+
+#include <dpkg/macros.h>
+
+void decompress_gzip(int fd_in, int fd_out, const char *desc)
+ DPKG_ATTR_NORET;
+void decompress_bzip2(int fd_in, int fd_out, const char *desc)
+ DPKG_ATTR_NORET;
+void decompress_lzma(int fd_in, int fd_out, const char *desc)
+ DPKG_ATTR_NORET;
+void decompress_noop(int fd_in, int fd_out, const char *desc)
+ DPKG_ATTR_NORET;
+
+void compress_gzip(int fd_in, int fd_out,
+ char compression, const char *desc) DPKG_ATTR_NORET;
+void compress_bzip2(int fd_in, int fd_out,
+ char compression, const char *desc) DPKG_ATTR_NORET;
+void compress_lzma(int fd_in, int fd_out,
+ char compression, const char *desc) DPKG_ATTR_NORET;
+void compress_noop(int fd_in, int fd_out,
+ const char *desc) DPKG_ATTR_NORET;
+
+#endif /* COMPRESSION_BACKEND_H */
diff --git a/lib/dpkg/compression.c b/lib/dpkg/compression.c
index 33b0922..813526f 100644
--- a/lib/dpkg/compression.c
+++ b/lib/dpkg/compression.c
@@ -1,115 +1,12 @@
 #include <config.h>
 #include <compat.h>
 
-#include <dpkg/i18n.h>
-
 #include <stdlib.h>
-#include <unistd.h>
-#include <string.h>
-#include <errno.h>
-
-#ifdef WITH_ZLIB
-#include <zlib.h>
-#endif
-#ifdef WITH_BZ2
-#include <bzlib.h>
-#endif
+#include <stdarg.h>
 
 #include <dpkg/dpkg.h>
-#include <dpkg/dpkg-db.h>
-#include <dpkg/buffer.h>
-
-static void
-fd_fd_filter(int fd_in, int fd_out, const char *desc,
-             const char *file, const char *cmd,
-             const char *argfmt, ...)
-{
-  struct varbuf argbuf = VARBUF_INIT;
-  va_list ap;
-
-  va_start(ap, argfmt);
-  varbufvprintf(&argbuf, argfmt, ap);
-  va_end(ap);
-
-  if (fd_in != 0) {
-    m_dup2(fd_in, 0);
-    close(fd_in);
-  }
-  if (fd_out != 1) {
-    m_dup2(fd_out, 1);
-    close(fd_out);
-  }
-
-  execlp(file, cmd, argbuf.buf, NULL);
-  ohshite(_("%s: failed to exec '%s %s'"), desc, cmd, argbuf.buf);
-
-  varbuffree(&argbuf);
-}
-
-#define DECOMPRESS(format, zFile, zdopen, zread, zerror, ERR_ERRNO, \
-                   fd_in, fd_out, desc) do \
-{ \
-  char buffer[4096]; \
-  int actualread; \
-  zFile zfile = zdopen(fd_in, "r"); \
-  \
-  while ((actualread = zread(zfile, buffer, sizeof(buffer)))) { \
-    int actualwrite; \
-    \
-    if (actualread < 0) { \
-      int err = 0; \
-      const char *errmsg = zerror(zfile, &err); \
-      \
-      if (err == ERR_ERRNO) \
-        errmsg = strerror(errno); \
-      ohshit(_("%s: internal " format " error: %s: %s"), \
-             desc, "read", errmsg); \
-    } \
-    \
-    actualwrite = write(fd_out, buffer, actualread); \
-    if (actualwrite != actualread) \
-      ohshite(_("%s: internal " format " error: %s"), \
-              desc, "write"); \
-  } \
-  exit(0); \
-} while(0)
-
-#define COMPRESS(format, zFile, zdopen, zwrite, zclose, zerror, ERR_ERRNO, \
-                 fd_in, fd_out, compression, desc) do \
-{ \
-  char combuf[] = {'w', compression, '\0'}; \
-  int actualread, actualwrite; \
-  char buffer[4096]; \
-  zFile zfile; \
-  \
-  zfile = zdopen(fd_out, combuf); \
-  while ((actualread = read(fd_in, buffer, sizeof(buffer)))) { \
-    if (actualread < 0) \
-      ohshite(_("%s: internal " format " error: %s"), \
-              desc, "read"); \
-    \
-    actualwrite = zwrite(zfile, buffer, actualread); \
-    if (actualwrite != actualread) { \
-      int err = 0; \
-      const char *errmsg = zerror(zfile, &err); \
-      \
-      if (err == ERR_ERRNO) \
-        errmsg = strerror(errno); \
-      ohshit(_("%s: internal " format " error: %s: %s"), \
-              desc, "write", errmsg); \
-    } \
-  } \
-  zclose(zfile); \
-  exit(0); \
-} while(0)
-
-static void
-compress_cmd(int fd_in, int fd_out, const char *path,
-             const char *cmd, char compression,
-             const char *desc)
-{
-  fd_fd_filter(fd_in, fd_out, desc, path, cmd, "-c%c", compression);
-}
+#include <dpkg/varbuf.h>
+#include <dpkg/compression-backend.h>
 
 void decompress_cat(enum compress_type type, int fd_in, int fd_out, char *desc, ...) {
   va_list al;
@@ -121,25 +18,13 @@ void decompress_cat(enum compress_type type, int fd_in, int fd_out, char *desc,
 
   switch(type) {
     case compress_type_gzip:
-#ifdef WITH_ZLIB
-      DECOMPRESS("gzip", gzFile, gzdopen, gzread, gzerror, Z_ERRNO,
-                 fd_in, fd_out, v.buf);
-#else
-      fd_fd_filter(fd_in, fd_out, v.buf, GZIP, "gzip", "-dc");
-#endif
+      decompress_gzip(fd_in, fd_out, v.buf);
     case compress_type_bzip2:
-#ifdef WITH_BZ2
-      DECOMPRESS("bzip2", BZFILE *, BZ2_bzdopen, BZ2_bzread,
-                 BZ2_bzerror, BZ_IO_ERROR,
-                 fd_in, fd_out, v.buf);
-#else
-      fd_fd_filter(fd_in, fd_out, v.buf, BZIP2, "bzip2", "-dc");
-#endif
+      decompress_bzip2(fd_in, fd_out, v.buf);
     case compress_type_lzma:
-      fd_fd_filter(fd_in, fd_out, v.buf, LZMA, "lzma", "-dc");
+      decompress_lzma(fd_in, fd_out, v.buf);
     case compress_type_cat:
-      fd_fd_copy(fd_in, fd_out, -1, _("%s: decompression"), v.buf);
-      exit(0);
+      decompress_noop(fd_in, fd_out, v.buf);
     default:
       exit(1);
   }
@@ -159,25 +44,13 @@ void compress_cat(enum compress_type type, int fd_in, int fd_out, const char *co
 
   switch(type) {
     case compress_type_gzip:
-#ifdef WITH_ZLIB
-      COMPRESS("gzip", gzFile, gzdopen, gzwrite, gzclose, gzerror, Z_ERRNO,
-               fd_in, fd_out, *compression, v.buf);
-#else
-      compress_cmd(fd_in, fd_out, GZIP, "gzip", *compression, v.buf);
-#endif
+      compress_gzip(fd_in, fd_out, *compression, v.buf);
     case compress_type_bzip2:
-#ifdef WITH_BZ2
-      COMPRESS("bzip2", BZFILE *, BZ2_bzdopen, BZ2_bzwrite, BZ2_bzclose,
-               BZ2_bzerror, BZ_IO_ERROR,
-               fd_in, fd_out, *compression, v.buf);
-#else
-      compress_cmd(fd_in, fd_out, BZIP2, "bzip2", *compression, v.buf);
-#endif
+      compress_bzip2(fd_in, fd_out, *compression, v.buf);
     case compress_type_lzma:
-      compress_cmd(fd_in, fd_out, LZMA, "lzma", *compression, v.buf);
+      compress_lzma(fd_in, fd_out, *compression, v.buf);
     case compress_type_cat:
-      fd_fd_copy(fd_in, fd_out, -1, _("%s: compression"), v.buf);
-      exit(0);
+      compress_noop(fd_in, fd_out, v.buf);
     default:
       exit(1);
   }
--
1.6.5.rc1.199.g596ec


--
To UNSUBSCRIBE, email to debian-dpkg-REQUEST@...
with a subject of "unsubscribe". Trouble? Contact listmaster@...

[Jonathan Nieder]

A gzclose call has the same potential for errors as a write,
since the compressor needs to flush its buffers before closing
its output file.  The same applies to BZ2_bzclose, but
unfortunately libbz2's gzio-style API does not expose the error
code.  Luckily, the only possible errors are I/O errors, which
can be detected through errno.

Similarly, explicitly closing a file descriptor can reveal errors
writing out buffered data.

Closing input handles, on the other hand, would be a waste of
time: all it would accomplish is to free some resources held by a
process that is about to exit anyway.

Signed-off-by: Jonathan Nieder <jrnieder@...>
---
 lib/dpkg/compression-backend.c |   55 ++++++++++++++++++++++++++++++++++-----
 1 files changed, 48 insertions(+), 7 deletions(-)

diff --git a/lib/dpkg/compression-backend.c b/lib/dpkg/compression-backend.c
index 9bfbcee..101018a 100644
--- a/lib/dpkg/compression-backend.c
+++ b/lib/dpkg/compression-backend.c
@@ -83,16 +83,21 @@ fd_fd_filter(int fd_in, int fd_out, const char *desc,
  ohshite(_("%s: internal " format " error: %s"), \
  desc, "write"); \
  } \
+ \
+ if (close(fd_out)) \
+ ohshite(_("%s: internal " format " error: %s"), \
+ desc, "close"); \
  exit(0); \
 } while(0)
 
-#define COMPRESS(format, zFile, zdopen, zwrite, zclose, zerror, ERR_ERRNO, \
+#define COMPRESS(format, zFile, zdopen, zwrite, zclose, \
+ zcloseerror, zerror, ERR_ERRNO, \
  fd_in, fd_out, compression, desc) do \
 { \
  char combuf[] = {'w', compression, '\0'}; \
- int actualread, actualwrite; \
  char buffer[4096]; \
  zFile zfile; \
+ int actualread, actualwrite, err = 0; \
  \
  zfile = zdopen(fd_out, combuf); \
  while ((actualread = read(fd_in, buffer, sizeof(buffer)))) { \
@@ -102,7 +107,6 @@ fd_fd_filter(int fd_in, int fd_out, const char *desc,
  \
  actualwrite = zwrite(zfile, buffer, actualread); \
  if (actualwrite != actualread) { \
- int err = 0; \
  const char *errmsg = zerror(zfile, &err); \
  \
  if (err == ERR_ERRNO) \
@@ -111,7 +115,15 @@ fd_fd_filter(int fd_in, int fd_out, const char *desc,
         desc, "write", errmsg); \
  } \
  } \
- zclose(zfile); \
+ \
+ err = zclose(zfile); \
+ if (err) { \
+ const char *errmsg = err == ERR_ERRNO ? \
+ strerror(errno) : zcloseerror(err); \
+ \
+ ohshit(_("%s: internal " format " error: %s: %s"), \
+ desc, "close", errmsg); \
+ } \
  exit(0); \
 } while(0)
 
@@ -133,7 +145,8 @@ decompress_gzip(int fd_in, int fd_out, const char *desc)
 void
 compress_gzip(int fd_in, int fd_out, char compression, const char *desc)
 {
- COMPRESS("gzip", gzFile, gzdopen, gzwrite, gzclose, gzerror, Z_ERRNO,
+ COMPRESS("gzip", gzFile, gzdopen, gzwrite, gzclose,
+ zError, gzerror, Z_ERRNO,
  fd_in, fd_out, compression, desc);
 }
 #else /* !WITH_ZLIB */
@@ -151,6 +164,34 @@ compress_gzip(int fd_in, int fd_out, char compression, const char *desc)
 #endif
 
 #ifdef WITH_BZ2
+/*
+ * BZ2_bzclose does not pass on the error code to the caller.
+ * To work around this, we could
+ *  - set errno to 0 and compare the value afterwards, hoping
+ *    that libbz2 did not recover from any errors itself
+ *  or
+ *  - modify a copy of BZ2_bzclose to expose the error
+ * This function follows the former course, so it can continue
+ * to work even if the layout of BZFILE objects should change.
+ */
+static int
+bzclose(BZFILE *b)
+{
+ errno = 0;
+ BZ2_bzclose(b);
+ if (errno)
+ return BZ_IO_ERROR;
+
+ return 0;
+}
+
+static const char * DPKG_ATTR_CONST
+bzcloseerror(int err)
+{
+ /* bzclose only returns BZ_OK and BZ_IO_ERROR. */
+ return _("Unexpected error (bug)");
+}
+
 void
 decompress_bzip2(int fd_in, int fd_out, const char *desc)
 {
@@ -162,8 +203,8 @@ decompress_bzip2(int fd_in, int fd_out, const char *desc)
 void
 compress_bzip2(int fd_in, int fd_out, char compression, const char *desc)
 {
- COMPRESS("bzip2", BZFILE *, BZ2_bzdopen, BZ2_bzwrite, BZ2_bzclose,
- BZ2_bzerror, BZ_IO_ERROR,
+ COMPRESS("bzip2", BZFILE *, BZ2_bzdopen, BZ2_bzwrite, bzclose,
+ bzcloseerror, BZ2_bzerror, BZ_IO_ERROR,
  fd_in, fd_out, compression, desc);
 }
 #else /* !WITH_BZ2 */
--
1.6.5.rc1.199.g596ec


--
To UNSUBSCRIBE, email to debian-dpkg-REQUEST@...
with a subject of "unsubscribe". Trouble? Contact listmaster@...

[Guillem Jover]

Hi!

On Sat, 2009-10-24 at 17:32:56 -0500, Jonathan Nieder wrote:
> gcc does not recognize __attribute__((constant)).
>
> Signed-off-by: Jonathan Nieder <jrnieder@...>

Nice catch. Pushed, thanks!

regards,
guillem


--
To UNSUBSCRIBE, email to debian-dpkg-REQUEST@...
with a subject of "unsubscribe". Trouble? Contact listmaster@...