Comment 3 for bug 1300876

Revision history for this message
Richard Brittain (richard-brittain) wrote : Re: [Bug 1300876] Re: close failures to stdout not detected

On Thu, 10 Apr 2014, Yavor Nikolov wrote:

> @richard-brittain, thanks for reporting this!

I love this program. It has made a huge difference to some of our work -
we generally use it for compressing multi-GB VM disk images.
I'm a bit rusty on coding - it has been a while since I looked at anything
as complex as this.

> I pushed a fix for that here: https://code.launchpad.net/~yavor-nikolov/pbzip2/bug-1300876-explicit-close-of-stdout
> Would you be able to test it?
> If all is working fine I think we'll be able to release version with that fix soon.

Thanks. I'll test this, but skimming your diffs, I think there may be too
many places where you removed the check for stdout vs named file.

Anywhere that we detect an error and are going to abort anyway, it would
be useful to close stout, but the exit fail status is probably already
going to get set. However, when looping over multiple input files which
complete without error, we don't want to close stdout inside the loop - in
this case it effectively concatenates the input files to a single
compressed output file (e.g., the code near line 2078 should retain the
'if (OutputStdOut == 0)' block.)

That's why I suggested an explicit close only for stdout, at the end of
the big 'for' loop, although I suspected there may be other places that
need it too.

We also need to still skip the writeFileMetaData() call on stdout, since
it isn't a real file.

Another very minor error - I think the error message for a close failure
is missing the word 'not'!

"pbzip2: *ERROR: Could close output file! Aborting..."

Richard

--
Richard Brittain, Research Computing Group,
                    IT Services, 37 Dewey Field Road, HB6219
                    Dartmouth College, Hanover NH 03755
<email address hidden> 6-2085