[netpbm/f20] Fixed: #1029512 pnmtops hangs during conversion
Petr Hracek
phracek at fedoraproject.org
Thu Dec 12 12:03:33 UTC 2013
commit 934cb0dd331de8eab3b48db86352bf5c28980a83
Author: Petr Hracek <phracek at redhat.com>
Date: Thu Dec 12 12:54:23 2013 +0100
Fixed: #1029512 pnmtops hangs during conversion
netpbm-pnmtops-hangs.patch | 327 ++++++++++++++++++++++++++++++++++++++++++++
netpbm.spec | 8 +-
2 files changed, 333 insertions(+), 2 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 96d1e25..424ce56 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: 7%{?dist}
+Release: 8%{?dist}
# See copyright_summary for details
License: BSD and GPLv2 and IJG and MIT and Public Domain
Group: System Environment/Libraries
@@ -38,6 +38,7 @@ Patch27: netpbm-multipage-pam.patch
Patch28: netpbm-compare-same-images.patch
#Patch29: netpbm-man-corrections.patch
Patch29: netpbm-manual-pages.patch
+Patch30: netpbm-pnmtops-hangs.patch
BuildRequires: libjpeg-devel, libpng-devel, libtiff-devel, flex
BuildRequires: libX11-devel, python, jasper-devel, libxml2-devel
@@ -116,7 +117,7 @@ netpbm-doc. You'll also need to install the netpbm-progs package.
%patch28 -p1 -b .compare-same-images
#%patch29 -p1 -b .man-corrections
%patch29 -p1 -b .manual-pages
-exit 0
+%patch30 -p1 -b .pnmtops-hangs
sed -i 's/STRIPFLAG = -s/STRIPFLAG =/g' config.mk.in
rm -rf converter/other/jpeg2000/libjasper/
@@ -271,6 +272,9 @@ rm -rf $RPM_BUILD_ROOT
%doc userguide/*
%changelog
+* Thu Dec 12 2013 Petr Hracek <phracek at redhat.com> - 10.61.02-8
+- Fixed: #1029512 pnmtops hangs during conversion
+
* Sat Aug 03 2013 Fedora Release Engineering <rel-eng at lists.fedoraproject.org> - 10.61.02-7
- Rebuilt for https://fedoraproject.org/wiki/Fedora_20_Mass_Rebuild
More information about the scm-commits
mailing list