Re: [PATCH v7 16/31] merge-recursive: split out code for determining diff_filepairs
- From
Stefan Beller <sbeller@google.com>
- Date
- Feb 3, 2018, 00:06 UTC
- Message-ID
- <CAGZ79kaB-RAP8f4Ea8bDkdVtkmfLSQ4=aA5bYMV_gXOk3qk-zQ@mail.gmail.com>
- In-Reply-To
- <20180130232533.25846-17-newren@gmail.com>
On Tue, Jan 30, 2018 at 3:25 PM, Elijah Newren <newren@gmail.com> wrote:
Show 7 quoted lines
> @@ -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.
Show 8 quoted lines
> + 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?