[PATCH 25/25] Fix stdout stream grabbing in format_part_content_text
Carl Worth
cworth at cworth.org
Fri Jun 3 15:38:29 PDT 2011
On Fri, 03 Jun 2011 14:26:48 -0700, Jameson Graef Rollins <jrollins at finestructure.net> wrote:
> Well actually it's only meant to sound like the committer doesn't
> understand the problem!
Heh, OK.
> > I'd like to investigate this more. Perhaps with a test case?
>
> The current tests are how I found the problem! Without this patch at
> least the multipart tests will fail. I don't see how another test will
> add anything.
Ah, in my review I'd managed to get this commit detached from the
original previous commit that introduced the test failures. It's funny
that while you were replying I was reviewing *that* commit and thinking,
"why are all these tests failing now just because they changed to
test_expect_equal_file"?
> Carl, if you (or anyone else) understands what the issue is, then please
> go ahead and modify the commit message.
Done.
> I don't understand things
> enough myself to do any better. Clearly there is some strange
> interaction with things that try to use stdout after
> g_mime_stream_file_new() has already grabbed it.
g_mime_stream_file_new is a bad citizen, API-wise. It closes files that
it didn't open, (by default).
> I really wouldn't block on this, though, since the patch does fix an
> actual bug.
Not blocked. All pushed. New commit message below for reference.
-Carl
commit d5b4d950245605b84c56ce991fa3c59a073a70e5
Author: Jameson Graef Rollins <jrollins at finestructure.net>
Date: Sat May 28 14:52:00 2011 -0700
show: Avoid inadvertently closing stdout
GMime has a nasty habit of taking ownership by default of any FILE*
handed to it va g_mime_stream_file_new. Specifically it will close the
FILE* when the stream is destroyed---even though GMime didn't open the
file itself.
To avoid this bad behavior, we have to carefully set_owner(FALSE)
after calling g_mime_stream_file_new. In the format_part_content_text
function, since commit d92146d3a6809f8ad940302af49cd99a0820665e we've
been calling g_mime_stream_file_new unconditionally, but only calling
g_mime_stream_file_set_owner(FALSE) conditionally.
This led to the FILE* being closed early when notmuch show output was
redirected to a file.
Fixing this fixes the test-suite cases that broke with the previous
commit, (which added redirected "notmuch show" calls to the test suite
to expose this bug).
Edited-by: Carl Worth <cworth at cworth.org> with a new commit message to
explain the bug and fix.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20110603/e6e00e66/attachment.pgp>
More information about the notmuch
mailing list