Re: [PATCH 1/4] stash: convert apply to builtin

[Christian Couder <christian.couder@xxxxxxxxx>]

On Sun, Mar 25, 2018 at 8:40 AM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote:
> On Sat, Mar 24, 2018 at 1:37 PM, Joel Teichroeb <joel@xxxxxxxxxxxxx> wrote:

>> +static int do_apply_stash(const char *prefix, struct stash_info *info, int index)
>> +{
>> +       if (index) {
>> +               if (!oidcmp(&info->b_tree, &info->i_tree) || !oidcmp(&c_tree, &info->i_tree)) {
>> +                       has_index = 0;
>> +               } else {
>> +                       struct child_process cp = CHILD_PROCESS_INIT;
>> +                       struct strbuf out = STRBUF_INIT;
>> +                       struct argv_array args = ARGV_ARRAY_INIT;
>> +                       cp.git_cmd = 1;
>> +                       argv_array_pushl(&cp.args, "diff-tree", "--binary", NULL);
>> +                       argv_array_pushf(&cp.args, "%s^2^..%s^2", sha1_to_hex(info->w_commit.hash), sha1_to_hex(info->w_commit.hash));
>> +                       if (pipe_command(&cp, NULL, 0, &out, 0, NULL, 0))
>> +                               return -1;
>
> Leaking 'out'?
>
>> +
>> +                       child_process_init(&cp);
>> +                       cp.git_cmd = 1;
>> +                       argv_array_pushl(&cp.args, "apply", "--cached", NULL);
>> +                       if (pipe_command(&cp, out.buf, out.len, NULL, 0, NULL, 0))
>> +                               return -1;
>
> Leaking 'out'.

It might be a good idea to have small functions encapsulating the
forks of git commands. For example:

static int diff_tree_binary(struct strbuf *out, struct object_id *w_commit)
{
    struct child_process cp = CHILD_PROCESS_INIT;
    const char *w_commit_hex = oid_to_hex(w_commit);

    cp.git_cmd = 1;
    argv_array_pushl(&cp.args, "diff-tree", "--binary", NULL);
    argv_array_pushf(&cp.args, "%s^2^..%s^2", w_commit_hex, w_commit_hex);

    return pipe_command(&cp, NULL, 0, out, 0, NULL, 0);
}

static int apply_cached(struct strbuf *out)
{
    struct child_process cp = CHILD_PROCESS_INIT;

    cp.git_cmd = 1;
    argv_array_pushl(&cp.args, "apply", "--cached", NULL);
    return pipe_command(&cp, out->buf, out->len, NULL, 0, NULL, 0);
}

This could help find leaks and maybe later make it easier to call
libified code instead of forking a git command.