[poppler] Stream reset and close in ImageStream

[Carlos Garcia Campos]

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
   elkalmail at yahoo.es
   carlosgc at gnome.org
   http://carlosgc.linups.org
PGP key: http://pgp.mit.edu:11371/pks/lookup?op=get&search=0x523E6462
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Reset-the-stream-in-constructor-and-close-it-in-dest.patch
Type: text/x-patch
Size: 13135 bytes
Desc: not available
Url : http://lists.freedesktop.org/archives/poppler/attachments/20090505/bb267c6e/attachment.bin 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Esta parte del mensaje =?ISO-8859-1?Q?est=E1?= firmada
 digitalmente
Url : http://lists.freedesktop.org/archives/poppler/attachments/20090505/bb267c6e/attachment.pgp