Re: [PATCH v2 3/7] rebase: add support for multiple hooks

> On Tue, May 14, 2019 at 7:24 AM brian m. carlson
> <sandals@crustytoothpaste.net> 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.