Am 14.05.19 um 14:56 schrieb Duy Nguyen:
> On Tue, May 14, 2019 at 7:24 AM brian m. carlson
> <sandals@xxxxxxxxxxxxxxxxxxxx> wrote:
>> diff --git a/builtin/am.c b/builtin/am.c
>> index 912d9821b1..340eacbd44 100644
>> --- a/builtin/am.c
>> +++ b/builtin/am.c
>> @@ -441,24 +441,8 @@ static int run_applypatch_msg_hook(struct am_state *state)
>> */
>> static int run_post_rewrite_hook(const struct am_state *state)
>> {
>> - struct child_process cp = CHILD_PROCESS_INIT;
>> - const char *hook = find_hook("post-rewrite");
>> - int ret;
>> -
>> - if (!hook)
>> - return 0;
>> -
>> - argv_array_push(&cp.args, hook);
>> - argv_array_push(&cp.args, "rebase");
>> -
>> - cp.in = xopen(am_path(state, "rewritten"), O_RDONLY);
>> - cp.stdout_to_stderr = 1;
>> - cp.trace2_hook_name = "post-rewrite";
>> -
>> - ret = run_command(&cp);
>> -
>> - close(cp.in);
>
> In the old code, we close cp.in...
>
>> +int post_rewrite_rebase_hook(const char *name, const char *path, void *input)
>> +{
>> + struct child_process child = CHILD_PROCESS_INIT;
>> +
>> + child.in = open(input, O_RDONLY);
>> + child.stdout_to_stderr = 1;
>> + child.trace2_hook_name = "post-rewrite";
>
>> + argv_array_push(&child.args, path);
>> + argv_array_push(&child.args, "rebase");
>> + return run_command(&child);
>
> ... but in the new one we don't. Smells fd leaking to me.
IIRC, run_command always closes the fds that it receives (even on error
paths). Therefore, the old code is incorrect and should not call
close(cp.in).
-- Hannes
