Re: [PATCH v7 16/31] merge-recursive: split out code for determining diff_filepairs

[Stefan Beller <sbeller@google.com>]

On Tue, Jan 30, 2018 at 3:25 PM, Elijah Newren <newren@gmail.com> wrote:

> @@ -1354,10 +1345,43 @@ static struct string_list *get_renames(struct merge_options *o,
>         diffcore_std(&opts);
>         if (opts.needed_rename_limit > o->needed_rename_limit)
>                 o->needed_rename_limit = opts.needed_rename_limit;
> -       for (i = 0; i < diff_queued_diff.nr; ++i) {
> +
> +       ret = malloc(sizeof(struct diff_queue_struct));

Please use xmalloc() and while at it, please use "*ret" as the argument
to sizeof. The reason is slightly better maintainability, as then the type
of ret can be changed at the declaration and the sizeof computation is still
correct.

> +       ret->queue = diff_queued_diff.queue;
> +       ret->nr = diff_queued_diff.nr;
> +       /* Ignore diff_queued_diff.alloc; we won't be changing size at all */
> +
> +       opts.output_format = DIFF_FORMAT_NO_OUTPUT;
> +       diff_queued_diff.nr = 0;
> +       diff_queued_diff.queue = NULL;
> +       diff_flush(&opts);

The comment is rather meant for the later lines or the former lines
(where ret is assigned), the empty line seems like it could go before
the comment?