Hi all, we have received a bug report in the evince list today about this file:
http://www.thomsongrassvalley.com/docs/DataSheets/switchers/kayenne/SWT-3025D.pdf at a first glance it seemed like a corrupted PDF file, since there were a lot of parser and lexer errors on stderr: Error (19684): Illegal character <4b> in hex string Error (19684): Illegal character <29> in hex string Error (19684): Illegal character <28> in hex string Error (19684): Illegal character <79> in hex string Error (19684): Illegal character <29> in hex string Error (19686): Illegal character <70> in hex string ....... However, the problem is not reproducible with the splash backend .... quite strange because gfx, parser, lexer and so on is shared code that has nothing to do with cairo or splash. After a lot of debugging, I've finally figured out what the problem is, There was a missing str->close() in CairoOutputDev::drawSoftMaskedImage(). The FileStream API is very confusing, reset() saves the current file position and seeks to the begining, and close() doesn't close the file, but restores the position to the previously saved position when reset was called. I think it makes more sense to call them reset() - restore() or save() - reset() - restore(). ImageStream has a reset() method wich is actually a wrapper to reset its stream, but it doesn't provide a close() method. Every time we use ImageStream we repeat this pattern: imgStr = new ImageStream(str, ...); imgStr->reset(); ....... str->close(); delete imgStr; With this API is quite easy to forget the str->close(), and I've noticed it's missing in other places too, indeed. We need to know that ImageStream::reset() calls str->reset() in order to know that we have to call str->close() to restore the file position. Confusing. So, I propose to remove the reset() method from the ImageStream class and call str->reset() in the constructor and str->close() in the destructor. See the attached patch. What do you think? -- Carlos Garcia Campos [email protected] [email protected] http://carlosgc.linups.org PGP key: http://pgp.mit.edu:11371/pks/lookup?op=get&search=0x523E6462
From 399f6467d9c4bbe920787698b5f22a1b862a9cae Mon Sep 17 00:00:00 2001 From: Carlos Garcia Campos <[email protected]> Date: Tue, 5 May 2009 18:45:48 +0200 Subject: [PATCH] Reset the stream in constructor and close it in destructor of ImageStream --- poppler/ArthurOutputDev.cc | 2 -- poppler/CairoOutputDev.cc | 9 --------- poppler/PSOutputDev.cc | 9 --------- poppler/Page.cc | 1 - poppler/SplashOutputDev.cc | 12 ------------ poppler/Stream.cc | 7 +++---- poppler/Stream.h | 3 --- utils/HtmlOutputDev.cc | 1 - utils/ImageOutputDev.cc | 1 - 9 files changed, 3 insertions(+), 42 deletions(-) diff --git a/poppler/ArthurOutputDev.cc b/poppler/ArthurOutputDev.cc index fe562fe..cc816c6 100644 --- a/poppler/ArthurOutputDev.cc +++ b/poppler/ArthurOutputDev.cc @@ -687,7 +687,6 @@ void ArthurOutputDev::drawImageMask(GfxState *state, Object *ref, Stream *str, /* TODO: Do we want to cache these? */ imgStr = new ImageStream(str, width, 1, 1); - imgStr->reset(); invert_bit = invert ? 1 : 0; @@ -757,7 +756,6 @@ void ArthurOutputDev::drawImage(GfxState *state, Object *ref, Stream *str, imgStr = new ImageStream(str, width, colorMap->getNumPixelComps(), colorMap->getBits()); - imgStr->reset(); /* ICCBased color space doesn't do any color correction * so check its underlying color space as well */ diff --git a/poppler/CairoOutputDev.cc b/poppler/CairoOutputDev.cc index 98adb7c..0b617de 100644 --- a/poppler/CairoOutputDev.cc +++ b/poppler/CairoOutputDev.cc @@ -1115,7 +1115,6 @@ void CairoOutputDev::drawImageMaskRegular(GfxState *state, Object *ref, Stream * /* TODO: Do we want to cache these? */ imgStr = new ImageStream(str, width, 1, 1); - imgStr->reset(); invert_bit = invert ? 1 : 0; @@ -1266,7 +1265,6 @@ void CairoOutputDev::drawImageMaskPrescaled(GfxState *state, Object *ref, Stream /* TODO: Do we want to cache these? */ imgStr = new ImageStream(str, width, 1, 1); - imgStr->reset(); invert_bit = invert ? 1 : 0; @@ -1457,7 +1455,6 @@ void CairoOutputDev::drawMaskedImage(GfxState *state, Object *ref, { ImageStream *maskImgStr; maskImgStr = new ImageStream(maskStr, maskWidth, 1, 1); - maskImgStr->reset(); int row_stride = (maskWidth + 3) & ~3; unsigned char *maskBuffer; @@ -1487,7 +1484,6 @@ void CairoOutputDev::drawMaskedImage(GfxState *state, Object *ref, maskWidth, maskHeight, row_stride); delete maskImgStr; - maskStr->close(); unsigned char *buffer; unsigned int *dest; @@ -1503,7 +1499,6 @@ void CairoOutputDev::drawMaskedImage(GfxState *state, Object *ref, imgStr = new ImageStream(str, width, colorMap->getNumPixelComps(), colorMap->getBits()); - imgStr->reset(); /* ICCBased color space doesn't do any color correction * so check its underlying color space as well */ @@ -1582,7 +1577,6 @@ void CairoOutputDev::drawSoftMaskedImage(GfxState *state, Object *ref, Stream *s maskImgStr = new ImageStream(maskStr, maskWidth, maskColorMap->getNumPixelComps(), maskColorMap->getBits()); - maskImgStr->reset(); int row_stride = (maskWidth + 3) & ~3; unsigned char *maskBuffer; @@ -1602,7 +1596,6 @@ void CairoOutputDev::drawSoftMaskedImage(GfxState *state, Object *ref, Stream *s maskWidth, maskHeight, row_stride); delete maskImgStr; - maskStr->close(); unsigned char *buffer; unsigned int *dest; @@ -1619,7 +1612,6 @@ void CairoOutputDev::drawSoftMaskedImage(GfxState *state, Object *ref, Stream *s imgStr = new ImageStream(str, width, colorMap->getNumPixelComps(), colorMap->getBits()); - imgStr->reset(); /* ICCBased color space doesn't do any color correction * so check its underlying color space as well */ @@ -1711,7 +1703,6 @@ void CairoOutputDev::drawImage(GfxState *state, Object *ref, Stream *str, imgStr = new ImageStream(str, width, colorMap->getNumPixelComps(), colorMap->getBits()); - imgStr->reset(); /* ICCBased color space doesn't do any color correction * so check its underlying color space as well */ diff --git a/poppler/PSOutputDev.cc b/poppler/PSOutputDev.cc index f02bd98..e09dc93 100644 --- a/poppler/PSOutputDev.cc +++ b/poppler/PSOutputDev.cc @@ -924,7 +924,6 @@ DeviceNRecoder::~DeviceNRecoder() { void DeviceNRecoder::reset() { imgStr = new ImageStream(str, width, colorMap->getNumPixelComps(), colorMap->getBits()); - imgStr->reset(); } GBool DeviceNRecoder::fillBuf() { @@ -4444,7 +4443,6 @@ void PSOutputDev::doImageL1(Object *ref, GfxImageColorMap *colorMap, // set up to process the data stream imgStr = new ImageStream(str, width, colorMap->getNumPixelComps(), colorMap->getBits()); - imgStr->reset(); // process the data stream i = 0; @@ -4464,7 +4462,6 @@ void PSOutputDev::doImageL1(Object *ref, GfxImageColorMap *colorMap, if (i != 0) { writePSChar('\n'); } - str->close(); delete imgStr; // imagemask @@ -4508,7 +4505,6 @@ void PSOutputDev::doImageL1Sep(GfxImageColorMap *colorMap, // set up to process the data stream imgStr = new ImageStream(str, width, colorMap->getNumPixelComps(), colorMap->getBits()); - imgStr->reset(); // process the data stream i = 0; @@ -4542,7 +4538,6 @@ void PSOutputDev::doImageL1Sep(GfxImageColorMap *colorMap, writePSChar('\n'); } - str->close(); delete imgStr; gfree(lineBuf); } @@ -4573,7 +4568,6 @@ void PSOutputDev::doImageL2(Object *ref, GfxImageColorMap *colorMap, // isn't allowed with inline images anyway numComps = colorMap->getNumPixelComps(); imgStr = new ImageStream(str, width, numComps, colorMap->getBits()); - imgStr->reset(); rects0Len = rects1Len = rectsOutLen = 0; rectsSize = rectsOutSize = 64; rects0 = (PSOutImgClipRect *)gmallocn(rectsSize, sizeof(PSOutImgClipRect)); @@ -4709,12 +4703,10 @@ void PSOutputDev::doImageL2(Object *ref, GfxImageColorMap *colorMap, gfree(rects0); gfree(rects1); delete imgStr; - str->close(); // explicit masking } else if (maskStr) { imgStr = new ImageStream(maskStr, maskWidth, 1, 1); - imgStr->reset(); rects0Len = rects1Len = rectsOutLen = 0; rectsSize = rectsOutSize = 64; rects0 = (PSOutImgClipRect *)gmallocn(rectsSize, sizeof(PSOutImgClipRect)); @@ -4811,7 +4803,6 @@ void PSOutputDev::doImageL2(Object *ref, GfxImageColorMap *colorMap, gfree(rects0); gfree(rects1); delete imgStr; - maskStr->close(); } // color space diff --git a/poppler/Page.cc b/poppler/Page.cc index 1cc2cfb..010adf3 100644 --- a/poppler/Page.cc +++ b/poppler/Page.cc @@ -548,7 +548,6 @@ GBool Page::loadThumb(unsigned char **data_out, ImageStream *imgstr = new ImageStream(str, width, colorMap->getNumPixelComps(), colorMap->getBits()); - imgstr->reset(); for (int row = 0; row < height; ++row) { for (int col = 0; col < width; ++col) { Guchar pix[gfxColorMaxComps]; diff --git a/poppler/SplashOutputDev.cc b/poppler/SplashOutputDev.cc index dc4661a..4cf8ed1 100644 --- a/poppler/SplashOutputDev.cc +++ b/poppler/SplashOutputDev.cc @@ -1724,7 +1724,6 @@ void SplashOutputDev::drawImageMask(GfxState *state, Object *ref, Stream *str, mat[5] = ctm[3] + ctm[5]; imgMaskData.imgStr = new ImageStream(str, width, 1, 1); - imgMaskData.imgStr->reset(); imgMaskData.invert = invert ? 0 : 1; imgMaskData.width = width; imgMaskData.height = height; @@ -1740,7 +1739,6 @@ void SplashOutputDev::drawImageMask(GfxState *state, Object *ref, Stream *str, } delete imgMaskData.imgStr; - str->close(); } struct SplashOutImageData { @@ -1988,7 +1986,6 @@ void SplashOutputDev::drawImage(GfxState *state, Object *ref, Stream *str, imgData.imgStr = new ImageStream(str, width, colorMap->getNumPixelComps(), colorMap->getBits()); - imgData.imgStr->reset(); imgData.colorMap = colorMap; imgData.maskColors = maskColors; imgData.colorMode = colorMode; @@ -2067,7 +2064,6 @@ void SplashOutputDev::drawImage(GfxState *state, Object *ref, Stream *str, gfree(imgData.lookup); delete imgData.imgStr; - str->close(); } struct SplashOutMaskedImageData { @@ -2223,7 +2219,6 @@ void SplashOutputDev::drawMaskedImage(GfxState *state, Object *ref, mat[4] = 0; mat[5] = 0; imgMaskData.imgStr = new ImageStream(maskStr, maskWidth, 1, 1); - imgMaskData.imgStr->reset(); imgMaskData.invert = maskInvert ? 0 : 1; imgMaskData.width = maskWidth; imgMaskData.height = maskHeight; @@ -2237,7 +2232,6 @@ void SplashOutputDev::drawMaskedImage(GfxState *state, Object *ref, maskSplash->fillImageMask(&imageMaskSrc, &imgMaskData, maskWidth, maskHeight, mat, gFalse); delete imgMaskData.imgStr; - maskStr->close(); delete maskSplash; //----- draw the source image @@ -2253,7 +2247,6 @@ void SplashOutputDev::drawMaskedImage(GfxState *state, Object *ref, imgData.imgStr = new ImageStream(str, width, colorMap->getNumPixelComps(), colorMap->getBits()); - imgData.imgStr->reset(); imgData.colorMap = colorMap; imgData.mask = maskBitmap; imgData.colorMode = colorMode; @@ -2325,7 +2318,6 @@ void SplashOutputDev::drawMaskedImage(GfxState *state, Object *ref, delete maskBitmap; gfree(imgData.lookup); delete imgData.imgStr; - str->close(); } } @@ -2364,7 +2356,6 @@ void SplashOutputDev::drawSoftMaskedImage(GfxState *state, Object *ref, imgMaskData.imgStr = new ImageStream(maskStr, maskWidth, maskColorMap->getNumPixelComps(), maskColorMap->getBits()); - imgMaskData.imgStr->reset(); imgMaskData.colorMap = maskColorMap; imgMaskData.maskColors = NULL; imgMaskData.colorMode = splashModeMono8; @@ -2386,7 +2377,6 @@ void SplashOutputDev::drawSoftMaskedImage(GfxState *state, Object *ref, maskSplash->drawImage(&imageSrc, &imgMaskData, splashModeMono8, gFalse, maskWidth, maskHeight, mat); delete imgMaskData.imgStr; - maskStr->close(); gfree(imgMaskData.lookup); delete maskSplash; splash->setSoftMask(maskBitmap); @@ -2396,7 +2386,6 @@ void SplashOutputDev::drawSoftMaskedImage(GfxState *state, Object *ref, imgData.imgStr = new ImageStream(str, width, colorMap->getNumPixelComps(), colorMap->getBits()); - imgData.imgStr->reset(); imgData.colorMap = colorMap; imgData.maskColors = NULL; imgData.colorMode = colorMode; @@ -2467,7 +2456,6 @@ void SplashOutputDev::drawSoftMaskedImage(GfxState *state, Object *ref, splash->setSoftMask(NULL); gfree(imgData.lookup); delete imgData.imgStr; - str->close(); } void SplashOutputDev::beginTransparencyGroup(GfxState *state, double *bbox, diff --git a/poppler/Stream.cc b/poppler/Stream.cc index 4200101..e3650cf 100644 --- a/poppler/Stream.cc +++ b/poppler/Stream.cc @@ -405,16 +405,15 @@ ImageStream::ImageStream(Stream *strA, int widthA, int nCompsA, int nBitsA) { } imgLine = (Guchar *)gmallocn(imgLineSize, sizeof(Guchar)); imgIdx = nVals; + + str->reset(); } ImageStream::~ImageStream() { + str->close(); gfree(imgLine); } -void ImageStream::reset() { - str->reset(); -} - GBool ImageStream::getPixel(Guchar *pix) { int i; diff --git a/poppler/Stream.h b/poppler/Stream.h index 2d1598f..ffc4f16 100644 --- a/poppler/Stream.h +++ b/poppler/Stream.h @@ -288,9 +288,6 @@ public: ~ImageStream(); - // Reset the stream. - void reset(); - // Gets the next pixel from the stream. <pix> should be able to hold // at least nComps elements. Returns false at end of file. GBool getPixel(Guchar *pix); diff --git a/utils/HtmlOutputDev.cc b/utils/HtmlOutputDev.cc index bda3846..28b2177 100644 --- a/utils/HtmlOutputDev.cc +++ b/utils/HtmlOutputDev.cc @@ -1379,7 +1379,6 @@ void HtmlOutputDev::drawImage(GfxState *state, Object *ref, Stream *str, // Initialize the image stream ImageStream *imgStr = new ImageStream(str, width, colorMap->getNumPixelComps(), colorMap->getBits()); - imgStr->reset(); // For each line... for (int y = 0; y < height; y++) { diff --git a/utils/ImageOutputDev.cc b/utils/ImageOutputDev.cc index be3807f..4461698 100644 --- a/utils/ImageOutputDev.cc +++ b/utils/ImageOutputDev.cc @@ -202,7 +202,6 @@ void ImageOutputDev::drawImage(GfxState *state, Object *ref, Stream *str, // initialize stream imgStr = new ImageStream(str, width, colorMap->getNumPixelComps(), colorMap->getBits()); - imgStr->reset(); // for each line... for (y = 0; y < height; ++y) { -- 1.6.0.4
signature.asc
Description: Esta parte del mensaje está firmada digitalmente
_______________________________________________ poppler mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/poppler
