{"thread":{"id":"17451","subject":"[PATCH] Don't try to reclose in command_close_bidi_pipe","startedAt":"2009-01-30T04:59:30Z","lastAt":"2009-01-31T00:03:21Z","messageCount":4,"participants":["Yuval Kogman","Tay Ray Chuan"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"102545","messageId":"1233291570-23295-1-git-send-email-nothingmuch@woobling.org","threadId":"17451","inReplyTo":null,"subject":"[PATCH] Don't try to reclose in command_close_bidi_pipe","fromName":"Yuval Kogman","fromEmail":"nothingmuch@woobling.org","sentAt":"2009-01-30T04:59:30Z","receivedAt":"2009-01-30T04:59:30Z","isPatch":true,"sender":{"key":"nothingmuch@woobling.org","avatar":"https://gravatar.com/avatar/8aeee9ffc2fdaff6410edb92db8c0fe2031d2965faa1f8e48489953c06caefd8?d=mp&s=160"},"body":"Some commands require their standard input to be closed (like\ngit-commit-tree). This patch changes command_close_bidi_pipe so no\nlonger tries to close already closed handles, resulting in an error.\n---\n perl/Git.pm |    1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\ndiff --git a/perl/Git.pm b/perl/Git.pm\nindex 7d7f2b1..283bba8 100644\n--- a/perl/Git.pm\n+++ b/perl/Git.pm\n@@ -422,6 +422,7 @@ sub command_close_bidi_pipe {\n \tlocal $?;\n \tmy ($pid, $in, $out, $ctx) = @_;\n \tforeach my $fh ($in, $out) {\n+\t\tnext unless defined(fileno($fh));\n \t\tunless (close $fh) {\n \t\t\tif ($!) {\n \t\t\t\tcarp \"error closing pipe: $!\";\n-- \n1.6.1\n"},{"id":"102600","messageId":"be6fef0d0901300635v4e6009f1n7da405ebc4c8dd68@mail.gmail.com","threadId":"17451","inReplyTo":"1233291570-23295-1-git-send-email-nothingmuch@woobling.org","subject":"Re: [PATCH] Don't try to reclose in command_close_bidi_pipe","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2009-01-30T14:35:54Z","receivedAt":"2009-01-30T14:35:54Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Hi,\n\nOn Fri, Jan 30, 2009 at 12:59 PM, Yuval Kogman <nothingmuch@woobling.org> wrote:\n> Some commands require their standard input to be closed (like\n> git-commit-tree). This patch changes command_close_bidi_pipe so no\n> longer tries to close already closed handles, resulting in an error.\n\n>From this message, you seem to intend this as a fix. Can you tell us\nhow one might go about to reproduce this issue?\n\n> ---\n>  perl/Git.pm |    1 +\n>  1 files changed, 1 insertions(+), 0 deletions(-)\n>\n> diff --git a/perl/Git.pm b/perl/Git.pm\n> index 7d7f2b1..283bba8 100644\n> --- a/perl/Git.pm\n> +++ b/perl/Git.pm\n> @@ -422,6 +422,7 @@ sub command_close_bidi_pipe {\n>        local $?;\n>        my ($pid, $in, $out, $ctx) = @_;\n>        foreach my $fh ($in, $out) {\n> +               next unless defined(fileno($fh));\n\nQuoting the perldoc for fileno at http://perldoc.perl.org/functions/fileno.html\n\n\"(Filehandles connected to memory objects via new features of open may\nreturn undefined even though they are open.)\"\n\nIs \"unless defined(fileno($fh))\" a reliable way to check if the handle\nis closed?\n\n-- \nCheers,\nRay Chuan\n"},{"id":"102603","messageId":"a891e1bd0901300706h4fdffe24m78c418f8c1ae7cd8@mail.gmail.com","threadId":"17451","inReplyTo":"be6fef0d0901300635v4e6009f1n7da405ebc4c8dd68@mail.gmail.com","subject":"Re: [PATCH] Don't try to reclose in command_close_bidi_pipe","fromName":"Yuval Kogman","fromEmail":"nothingmuch@woobling.org","sentAt":"2009-01-30T15:06:11Z","receivedAt":"2009-01-30T15:06:11Z","isPatch":true,"sender":{"key":"nothingmuch@woobling.org","avatar":"https://gravatar.com/avatar/8aeee9ffc2fdaff6410edb92db8c0fe2031d2965faa1f8e48489953c06caefd8?d=mp&s=160"},"body":"Resending, forgot to CC\n\n2009/1/30 Tay Ray Chuan <rctay89@gmail.com>\n> From this message, you seem to intend this as a fix. Can you tell us\n> how one might go about to reproduce this issue?\n\nHere is an example:\nhttp://github.com/yanick/git-cpan-patch/blob/ed67d3f86f371764935fd0da3e7f08536c95b606/git-cpan-import#L190\n\nSince git-commit-tree requires the message to be written before it can\nwrite the sha1 of the new commit object, the handle has to be closed\nalready, which makes command_close_bidi_pipe die with an error.\nThe workaround in the linked code reopens a fake handle so that\nclose() will not error.\nFor what it's worth, I've been poking at t/t9700/test.pl but since\nit's pretty sparse I figured I should contribute this fix\nindependently.\n\n> Is \"unless defined(fileno($fh))\" a reliable way to check if the handle\n> is closed?\n\nYes, since the handle is expected to be the result of a pipe open. The\ndocumentation refers to this case:\n\n    open my $fh, \"<\", \\$buffer;\n\nin which case the filehandle $fh has no associated file descriptor.\n\nfileno() is not a universal check, but it is applicable in this situation.\n"},{"id":"102662","messageId":"be6fef0d0901301603m452b1579q509bdc997ecad7c4@mail.gmail.com","threadId":"17451","inReplyTo":"a891e1bd0901300706h4fdffe24m78c418f8c1ae7cd8@mail.gmail.com","subject":"Re: [PATCH] Don't try to reclose in command_close_bidi_pipe","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2009-01-31T00:03:21Z","receivedAt":"2009-01-31T00:03:21Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Hi,\n\nOn Fri, Jan 30, 2009 at 11:06 PM, Yuval Kogman <nothingmuch@woobling.org> wrote:\n> Here is an example:\n> http://github.com/yanick/git-cpan-patch/blob/ed67d3f86f371764935fd0da3e7f08536c95b606/git-cpan-import#L190\n>\n> Since git-commit-tree requires the message to be written before it can\n> write the sha1 of the new commit object, the handle has to be closed\n> already, which makes command_close_bidi_pipe die with an error.\n> The workaround in the linked code reopens a fake handle so that\n> close() will not error.\n> For what it's worth, I've been poking at t/t9700/test.pl but since\n> it's pretty sparse I figured I should contribute this fix\n> independently.\n\nYour usage seems to a very special case. You're closing the handle,\nand then invoking the method \"command_close_bidi_pipe\", whose job is\nto close the pipe handles passed to it (among other things). So, it\ndoesn't seem very good to make \"command_close_bidi_pipe\" to take into\naccount that you closed your handle independently.\n\nMaybe you could write a pair of new functions for the commit-tree\ncommand in the Git package, something like the\n\"_open_hash_and_insert_object_if_needed\" and \"hash_and_insert_object\"\npair.\n\n-- \nCheers,\nRay Chuan\n"}]}