{"thread":{"id":"10920","subject":"[PATCH] Fix start_command closing cmd->out/in regardless of cmd->close_out/in","startedAt":"2007-11-18T17:36:06Z","lastAt":"2007-11-19T08:46:17Z","messageCount":4,"participants":["Ping Yin","Junio C Hamano","Johannes Sixt"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"60239","messageId":"1195407366-1610-1-git-send-email-pkufranky@gmail.com","threadId":"10920","inReplyTo":null,"subject":"[PATCH] Fix start_command closing cmd->out/in regardless of cmd->close_out/in","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2007-11-18T17:36:06Z","receivedAt":"2007-11-18T17:36:06Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"When 'FILE *fp' is assigned to child_process.out and then start_command or\nrun_command is run, the standard output of the child process is expected to\nbe outputed to fp. However, sometimes fp is not expected to be closed since\nfurther IO may be still performmed on fp.\n---\n run-command.c |    4 ----\n 1 files changed, 0 insertions(+), 4 deletions(-)\n\ndiff --git a/run-command.c b/run-command.c\nindex 476d00c..4e5f58d 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -115,13 +115,9 @@ int start_command(struct child_process *cmd)\n \n \tif (need_in)\n \t\tclose(fdin[0]);\n-\telse if (cmd->in)\n-\t\tclose(cmd->in);\n \n \tif (need_out)\n \t\tclose(fdout[1]);\n-\telse if (cmd->out > 1)\n-\t\tclose(cmd->out);\n \n \tif (need_err)\n \t\tclose(fderr[1]);\n-- \n1.5.3.5.1876.g7ba19-dirty\n"},{"id":"60248","messageId":"7vmytbv11m.fsf@gitster.siamese.dyndns.org","threadId":"10920","inReplyTo":"1195407366-1610-1-git-send-email-pkufranky@gmail.com","subject":"Re: [PATCH] Fix start_command closing cmd->out/in regardless of cmd->close_out/in","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-18T18:52:37Z","receivedAt":"2007-11-18T18:52:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ping Yin <pkufranky@gmail.com> writes:\n\n> When 'FILE *fp' is assigned to child_process.out and then start_command or\n> run_command is run, the standard output of the child process is expected to\n> be outputed to fp. However, sometimes fp is not expected to be closed since\n> further IO may be still performmed on fp.\n\nCould you clarify \"However, sometimes\" with a concrete codepath\nthat has this problem in the proposed commit log message, and\nSign-off your patch please?\n"},{"id":"60271","messageId":"47413DB9.9030306@viscovery.net","threadId":"10920","inReplyTo":"1195407366-1610-1-git-send-email-pkufranky@gmail.com","subject":"Re: [PATCH] Fix start_command closing cmd->out/in regardless of cmd->close_out/in","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2007-11-19T07:39:37Z","receivedAt":"2007-11-19T07:39:37Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Ping Yin schrieb:\n> When 'FILE *fp' is assigned to child_process.out and then start_command or\n> run_command is run, the standard output of the child process is expected to\n> be outputed to fp. However, sometimes fp is not expected to be closed since\n> further IO may be still performmed on fp.\n> ---\n>  run-command.c |    4 ----\n>  1 files changed, 0 insertions(+), 4 deletions(-)\n> \n> diff --git a/run-command.c b/run-command.c\n> index 476d00c..4e5f58d 100644\n> --- a/run-command.c\n> +++ b/run-command.c\n> @@ -115,13 +115,9 @@ int start_command(struct child_process *cmd)\n>  \n>  \tif (need_in)\n>  \t\tclose(fdin[0]);\n> -\telse if (cmd->in)\n> -\t\tclose(cmd->in);\n>  \n>  \tif (need_out)\n>  \t\tclose(fdout[1]);\n> -\telse if (cmd->out > 1)\n> -\t\tclose(cmd->out);\n>  \n>  \tif (need_err)\n>  \t\tclose(fderr[1]);\n\nThis is dangerous! You have to audit all current callers whether they close \ncmd->in or cmd->out (if they don't need the fd anymore). Otherwise you risk \nto keep a writable pipe end open and then the reader hangs, waiting for \ninput that will never arrive.\n\n-- Hannes\n"},{"id":"60279","messageId":"7vfxz2oc6e.fsf@gitster.siamese.dyndns.org","threadId":"10920","inReplyTo":"47413DB9.9030306@viscovery.net","subject":"Re: [PATCH] Fix start_command closing cmd->out/in regardless of cmd->close_out/in","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-19T08:46:17Z","receivedAt":"2007-11-19T08:46:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt <j.sixt@viscovery.net> writes:\n\n> Ping Yin schrieb:\n>> When 'FILE *fp' is assigned to child_process.out and then start_command or\n>> run_command is run, the standard output of the child process is expected to\n>> be outputed to fp. However, sometimes fp is not expected to be closed since\n>> further IO may be still performmed on fp.\n>> ---\n>>  run-command.c |    4 ----\n>>  1 files changed, 0 insertions(+), 4 deletions(-)\n>>\n>> diff --git a/run-command.c b/run-command.c\n>> index 476d00c..4e5f58d 100644\n>> --- a/run-command.c\n>> +++ b/run-command.c\n>> @@ -115,13 +115,9 @@ int start_command(struct child_process *cmd)\n>>   \tif (need_in)\n>>  \t\tclose(fdin[0]);\n>> -\telse if (cmd->in)\n>> -\t\tclose(cmd->in);\n>>   \tif (need_out)\n>>  \t\tclose(fdout[1]);\n>> -\telse if (cmd->out > 1)\n>> -\t\tclose(cmd->out);\n>>   \tif (need_err)\n>>  \t\tclose(fderr[1]);\n>\n> This is dangerous! You have to audit all current callers whether they\n> close cmd->in or cmd->out (if they don't need the fd\n> anymore).\n\nI am reasonably sure that they are already relying on these auto\nclosing of the file descriptors.\n\nFunny that somebody falls into the trap the day after we\ndiscussed it on another thread.\n"}]}