From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by olra.theworths.org (Postfix) with ESMTP id 973E5431FD0 for ; Fri, 3 Jun 2011 15:38:42 -0700 (PDT) X-Virus-Scanned: Debian amavisd-new at olra.theworths.org X-Spam-Flag: NO X-Spam-Score: 0.01 X-Spam-Level: X-Spam-Status: No, score=0.01 tagged_above=-999 required=5 tests=[T_MIME_NO_TEXT=0.01] autolearn=disabled Received: from olra.theworths.org ([127.0.0.1]) by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id TZkBs+xkvFAH for ; Fri, 3 Jun 2011 15:38:41 -0700 (PDT) Received: from arlo.cworth.org (arlo.cworth.org [50.43.72.2]) by olra.theworths.org (Postfix) with ESMTP id 9F8A2431FB6 for ; Fri, 3 Jun 2011 15:38:41 -0700 (PDT) Received: from yoom.home.cworth.org (localhost [127.0.0.1]) by arlo.cworth.org (Postfix) with ESMTP id 9C17229A5CC; Fri, 3 Jun 2011 15:38:40 -0700 (PDT) Received: by yoom.home.cworth.org (Postfix, from userid 1000) id 8EA0C54C02C; Fri, 3 Jun 2011 15:38:40 -0700 (PDT) From: Carl Worth To: Jameson Graef Rollins , Notmuch Mail Subject: Re: [PATCH 25/25] Fix stdout stream grabbing in format_part_content_text In-Reply-To: <87lixihahz.fsf@servo.factory.finestructure.net> References: <1306619520-25730-1-git-send-email-jrollins@finestructure.net> <1306619520-25730-2-git-send-email-jrollins@finestructure.net> <1306619520-25730-3-git-send-email-jrollins@finestructure.net> <1306619520-25730-4-git-send-email-jrollins@finestructure.net> <1306619520-25730-5-git-send-email-jrollins@finestructure.net> <1306619520-25730-6-git-send-email-jrollins@finestructure.net> <1306619520-25730-7-git-send-email-jrollins@finestructure.net> <1306619520-25730-8-git-send-email-jrollins@finestructure.net> <1306619520-25730-9-git-send-email-jrollins@finestructure.net> <1306619520-25730-10-git-send-email-jrollins@finestructure.net> <1306619520-25730-11-git-send-email-jrollins@finestructure.net> <1306619520-25730-12-git-send-email-jrollins@finestructure.net> <1306619520-25730-13-git-send-email-jrollins@finestructure.net> <1306619520-25730-14-git-send-email-jrollins@finestructure.net> <1306619520-25730-15-git-send-email-jrollins@finestructure.net> <1306619520-25730-16-git-se nd-email-jrollins@finestructure.net> <1306619520-25730-17-git-send-email-jrollins@finestructure.net> <1306619520-25730-18-git-send-email-jrollins@finestructure.net> <1306619520-25730-19-git-send-email-jrollins@finestructure.net> <1306619520-25730-20-git-send-email-jrollins@finestructure.net> <1306619520-25730-21-git-send-email-jrollins@finestructure.net> <1306619520-25730-22-git-send-email-jrollins@finestructure.net> <1306619520-25730-23-git-send-email-jrollins@finestructure.net> <1306619520-25730-24-git-send-email-jrollins@finestructure.net> <1306619520-25730-25-git-send-email-jrollins@finestructure.net> <1306619520-25730-26-git-send-email-jrollins@finestructure.net> <87r57almd9.fsf@yoom.home.cworth.org> <87lixihahz.fsf@servo.factory.finestructure.net> User-Agent: Notmuch/0.5 (http://notmuchmail.org) Emacs/23.3.1 (i486-pc-linux-gnu) Date: Fri, 03 Jun 2011 15:38:29 -0700 Message-ID: <87d3iulevu.fsf@yoom.home.cworth.org> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha1; protocol="application/pgp-signature" X-BeenThere: notmuch@notmuchmail.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: "Use and development of the notmuch mail system." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 03 Jun 2011 22:38:42 -0000 --=-=-= Content-Transfer-Encoding: quoted-printable On Fri, 03 Jun 2011 14:26:48 -0700, Jameson Graef Rollins 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? >=20 > 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. =2DCarl commit d5b4d950245605b84c56ce991fa3c59a073a70e5 Author: Jameson Graef Rollins Date: Sat May 28 14:52:00 2011 -0700 show: Avoid inadvertently closing stdout =20=20=20=20 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. =20=20=20=20 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. =20=20=20=20 This led to the FILE* being closed early when notmuch show output was redirected to a file. =20=20=20=20 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). =20=20=20=20 Edited-by: Carl Worth with a new commit message to explain the bug and fix. --=-=-= Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) iEYEARECAAYFAk3pYmUACgkQ6JDdNq8qSWjz3QCfZ3bmMGBdHscBBSOKiUSRaxHo 7hUAnRFkfAXcy9bK6c/+KTcsj+SOsL8o =r/3M -----END PGP SIGNATURE----- --=-=-=--