[netpbm/f19] Fixed: #1029512 pnmtops hangs during conversion

Petr Hracek phracek at fedoraproject.org
Thu Dec 12 12:49:11 UTC 2013


commit f2f60ac9504149747914bf1f43c6e615f6496c4e
Author: Petr Hracek <phracek at redhat.com>
Date:   Thu Dec 12 13:46:51 2013 +0100

    Fixed: #1029512 pnmtops hangs during conversion

 netpbm-pnmtops-hangs.patch |  327 ++++++++++++++++++++++++++++++++++++++++++++
 netpbm.spec                |    5 +-
 2 files changed, 331 insertions(+), 1 deletions(-)
---
diff --git a/netpbm-pnmtops-hangs.patch b/netpbm-pnmtops-hangs.patch
new file mode 100644
index 0000000..4efe6f6
--- /dev/null
+++ b/netpbm-pnmtops-hangs.patch
@@ -0,0 +1,327 @@
+diff --git a/converter/other/pnmtops.c b/converter/other/pnmtops.c
+index e393931..e1f2d18 100644
+--- a/converter/other/pnmtops.c
++++ b/converter/other/pnmtops.c
+@@ -42,6 +42,8 @@
+ #include <unistd.h>
+ #include <assert.h>
+ #include <string.h>
++#include <errno.h>
++#include <signal.h>
+ #ifndef NOFLATE
+ #include <zlib.h>
+ #endif
+@@ -52,6 +54,37 @@
+ #include "shhopt.h"
+ #include "nstring.h"
+ 
++
++
++static void
++setSignals() {
++/*----------------------------------------------------------------------------
++   Set up the process-global signal-related state.
++
++   Note that we can't rely on defaults, because much of this is inherited
++   from the process that forked and exec'ed this program.
++-----------------------------------------------------------------------------*/
++    /* See waitForChildren() for why we do this to SIGCHLD */
++
++    struct sigaction sigchldAction;
++    int rc;
++    sigset_t emptySet;
++
++    sigemptyset(&emptySet);
++
++    sigchldAction.sa_handler = SIG_DFL;
++    sigchldAction.sa_mask = emptySet;
++    sigchldAction.sa_flags = SA_NOCLDSTOP;
++
++    rc = sigaction(SIGCHLD, &sigchldAction, NULL);
++
++    if (rc != 0)
++        pm_error("sigaction() to set up signal environment failed, "
++                 "errno = %d (%s)", errno, strerror(errno));
++}
++
++
++
+ struct cmdlineInfo {
+     /* All the information the user supplied in the command line,
+        in a form easy for the program to use.
+@@ -256,21 +289,17 @@ parseCommandLine(int argc, const char ** argv,
+     validateCompDimension(width, 72, "-width value");
+     validateCompDimension(height, 72, "-height value");
+     
+-    overflow2(width, 72);
+     cmdlineP->width  = width * 72;
+-    overflow2(height, 72);
+     cmdlineP->height = height * 72;
+ 
+     if (imagewidthSpec) {
+         validateCompDimension(imagewidth, 72, "-imagewidth value");
+-        overflow2(imagewidth, 72);
+         cmdlineP->imagewidth = imagewidth * 72;
+     }
+     else
+         cmdlineP->imagewidth = 0;
+     if (imageheightSpec) {
+-        validateCompDimension(imageheight, 72, "-imageheight value");
+-        overflow2(imageheight, 72);
++        validateCompDimension(imagewidth, 72, "-imageheight value");
+         cmdlineP->imageheight = imageheight * 72;
+     }
+     else
+@@ -339,6 +368,33 @@ basebasename(const char * const filespec) {
+ 
+ 
+ 
++static void
++writeFile(const unsigned char * const buffer,
++          size_t                const writeCt,
++          const char *          const name,
++          FILE *                const ofP) {
++
++    size_t writtenCt;
++
++    writtenCt = fwrite(buffer, 1, writeCt, ofP);
++
++    if (writtenCt != writeCt)
++        pm_error("Error writing to %s output file", name);
++}
++
++
++
++static void
++writeFileChar(const char * const buffer,
++              size_t       const writeCt,
++              const char * const name,
++              FILE *       const ofP) {
++
++    writeFile((const unsigned char *)buffer, writeCt, name, ofP);
++}
++
++
++
+ #define MAX_FILTER_CT 10
+     /* The maximum number of filters this code is capable of applying */
+ 
+@@ -498,18 +554,12 @@ flateFilter(FILE *          const ifP,
+         */
+         do {
+             unsigned int have;
+-            size_t bytesWritten;
+ 
+             strm.avail_out = chunkSz;
+             strm.next_out = out;
+             deflate(&strm, flush);
+             have = chunkSz - strm.avail_out;
+-            bytesWritten = fwrite(out, 1, have, ofP);
+-            if (ferror(ofP) || bytesWritten != have) {
+-                deflateEnd(&strm);
+-                pm_error("Error writing to internal pipe during "
+-                         "flate compression.");
+-            }
++            writeFile(out, have, "flate filter", ofP);
+         } while (strm.avail_out == 0);
+         assert(strm.avail_in == 0);     /* all input is used */
+ 
+@@ -554,7 +604,7 @@ rlePutBuffer (unsigned int    const repeat,
+         fputc(repeatitem, fP);
+     } else {
+         fputc(count - 1, fP);
+-        fwrite(itembuf, 1, count, fP);
++        writeFile(itembuf, count, "rlePutBuffer", fP);
+     }
+ }
+ 
+@@ -677,23 +727,24 @@ asciiHexFilter(FILE *          const ifP,
+     unsigned char inbuff[40], outbuff[81];
+ 
+     for (eof = false; !eof; ) {
+-        size_t bytesRead;
++        size_t readCt;
+ 
+-        bytesRead = fread(inbuff, 1, 40, ifP);
++        readCt = fread(inbuff, 1, 40, ifP);
+ 
+-        if (bytesRead == 0)
++        if (readCt == 0)
+             eof = true;
+         else {
+             unsigned int i;
+ 
+-            for (i = 0; i < bytesRead; ++i) {
++            for (i = 0; i < readCt; ++i) {
+                 int const item = inbuff[i]; 
+                 outbuff[i*2]   = hexits[item >> 4];
+                 outbuff[i*2+1] = hexits[item & 15];
+             }
+         }
+-        outbuff[bytesRead * 2] = '\n';
+-        fwrite(outbuff, 1, bytesRead*2 + 1, ofP);
++        outbuff[readCt * 2] = '\n';
++
++        writeFile(outbuff, readCt * 2 + 1, "asciiHex filter", ofP);
+     }
+ 
+     fclose(ifP);
+@@ -731,7 +782,8 @@ ascii85Filter(FILE *          const ifP,
+             ++count;
+ 
+             if (value == 0 && count == 4) {
+-                putchar('z');  /* Ascii85 encoding z exception */
++                writeFileChar("z", 1, "ASCII 85 filter", ofP);
++                    /* Ascii85 encoding z exception */
+                 ++outcount;
+                 count = 0;
+             } else if (count == 4) {
+@@ -741,13 +793,14 @@ ascii85Filter(FILE *          const ifP,
+                 outbuff[1] = value % 85 + 33;
+                 outbuff[0] = value / 85 + 33;
+ 
+-                fwrite(outbuff, 1, count + 1, ofP);
++                writeFileChar(outbuff, count + 1, "ASCII 85 filter", ofP);
++
+                 count = value = 0;
+                 outcount += 5; 
+             }
+ 
+             if (outcount > 75) {
+-                putchar('\n');
++                writeFileChar("\n", 1, "ASCII 85 filter", ofP);
+                 outcount = 0;
+             }
+         }
+@@ -763,7 +816,7 @@ ascii85Filter(FILE *          const ifP,
+         outbuff[0] = value / 85 + 33;
+         outbuff[count + 1] = '\n';
+ 
+-        fwrite(outbuff, 1, count + 2, ofP);
++        writeFileChar(outbuff, count + 2, "ASCII 85 filter", ofP);
+     }
+ 
+     fclose(ifP);
+@@ -873,13 +926,22 @@ addFilter(const char *    const description,
+           OutputEncoder * const oeP,
+           FILE **         const feedFilePP,
+           pid_t *         const pidList) {
++/*----------------------------------------------------------------------------
++   Add a filter to the front of the chain.
+ 
+-    pid_t pid;
++   Spawn a process to do the filtering, by running function 'filter'.
++
++   *feedFilePP is the present head of the chain.  We make the new filter
++   process write its output to that and get its input from a new pipe.
++   We update *feedFilePP to the sending end of the new pipe.
+ 
++   Add to the list pidList[] the PID of the process we spawn.
++-----------------------------------------------------------------------------*/
+     FILE * const oldFeedFileP = *feedFilePP;
+ 
+     FILE * newFeedFileP;
+-        
++    pid_t pid;
++
+     spawnFilter(oldFeedFileP, filter, oeP, &newFeedFileP, &pid);
+             
+     if (verbose)
+@@ -887,7 +949,7 @@ addFilter(const char *    const description,
+                    description, (unsigned)pid);
+     
+     fclose(oldFeedFileP);  /* Child keeps this open now */
+-    
++
+     addToPidList(pidList, pid);
+ 
+     *feedFilePP = newFeedFileP;
+@@ -949,6 +1011,15 @@ waitForChildren(const pid_t * const pidList) {
+    Wait for all child processes with PIDs in pidList[] to exit.
+    In pidList[], end-of-list is marked with a special zero value.
+ -----------------------------------------------------------------------------*/
++    /* There's an odd behavior in Unix such that if you have set the
++       action for SIGCHLD to ignore the signal (even though ignoring the
++       signal is the default), the process' children do not become
++       zombies.  Consequently, waitpid() always fails with ECHILD - but
++       nonetheless waits for the child to exit.
++    
++       We expect the process not to have the action for SIGCHLD set that
++       way.
++    */
+     unsigned int i;
+ 
+     for (i = 0; pidList[i]; ++i) {
+@@ -960,7 +1031,8 @@ waitForChildren(const pid_t * const pidList) {
+ 
+         rc = waitpid(pidList[i], &status, 0);
+         if (rc == -1)
+-            pm_error ("waitpid() for child %u failed", i);
++            pm_error ("waitpid() for child %u failed, errno=%d (%s)",
++                      i, errno, strerror(errno));
+         else if (status != EXIT_SUCCESS)
+             pm_error ("Child process %u terminated abnormally", i);
+     }
+@@ -1724,7 +1796,7 @@ convertRowPbm(struct pam *     const pamP,
+         bitrow[colChars-1] <<= padRight;  /* right edge */
+     }
+ 
+-    fwrite(bitrow, 1, colChars, fP); 
++    writeFile(bitrow, colChars, "PBM reader", fP);
+ }
+ 
+ 
+@@ -1867,6 +1939,21 @@ convertRaster(struct pam * const inpamP,
+ 
+ 
+ 
++/* FILE MANAGEMENT: File management is pretty hairy here.  A filter, which
++   runs in its own process, needs to be able to cause its output file to
++   close because it might be an internal pipe and the next stage needs to
++   know output is done.  So the forking process must close its copy of the
++   file descriptor.  BUT: if the output of the filter is not an internal
++   pipe but this program's output, then we don't want it closed when the
++   filter terminates because we'll need it to be open for the next image
++   the program converts (with a whole new chain of filters).
++   
++   To prevent the progam output file from getting closed, we pass a
++   duplicate of it to spawnFilters() and keep the original open.
++*/
++
++
++
+ static void
+ convertPage(FILE *       const ifP, 
+             int          const turnflag, 
+@@ -1954,7 +2041,7 @@ convertPage(FILE *       const ifP,
+ 
+     fflush(stdout);
+     filterChainOfP = fdopen(dup(fileno(stdout)), "w");
+-        /* spawnFilters() closes this. See FILE MANAGEMENT above */
++        /* spawnFilters() closes this.  See FILE MANAGEMENT above */
+ 
+     spawnFilters(filterChainOfP, &oe, &feedFileP, filterPidList);
+  
+@@ -1979,6 +2066,8 @@ main(int argc, const char * argv[]) {
+ 
+     pm_proginit(&argc, argv);
+ 
++    setSignals();
++
+     parseCommandLine(argc, argv, &cmdline);
+ 
+     verbose = cmdline.verbose;
+@@ -1994,6 +2083,13 @@ main(int argc, const char * argv[]) {
+         name = strdup("noname");
+     else
+         name = basebasename(cmdline.inputFileName);
++
++    /* This program manages file descriptors in a way that assumes
++       that new files will get file descriptor numbers less than 10,
++       so we close superfluous files now to make sure that's true.
++    */
++    closeAllBut(fileno(ifP), fileno(stdout), fileno(stderr));
++
+     {
+         int eof;  /* There are no more images in the input file */
+         unsigned int imageSeq;
diff --git a/netpbm.spec b/netpbm.spec
index 9f4e1c9..19d1b38 100644
--- a/netpbm.spec
+++ b/netpbm.spec
@@ -1,7 +1,7 @@
 Summary: A library for handling different graphics file formats
 Name: netpbm
 Version: 10.61.02
-Release: 5%{?dist}
+Release: 6%{?dist}
 # See copyright_summary for details
 License: BSD and GPLv2 and IJG and MIT and Public Domain
 Group: System Environment/Libraries
@@ -271,6 +271,9 @@ rm -rf $RPM_BUILD_ROOT
 %doc userguide/*
 
 %changelog
+* Thu Dec 12 2013 Petr Hracek <phracek at redhat.com> - 10.61.02-6
+- Fixed: #1029512 pnmtops hangs during conversion
+
 * Mon Jun 17 2013 Petr Hracek <phracek at redhat.com> - 10.61.02-5
 - Manual page corrections (#948531)
 


More information about the scm-commits mailing list