Re: [PATCH] merge-recursive.c: use QSORT macro

[Duy Nguyen <pclouds@gmail.com>]

On Wed, Nov 23, 2016 at 12:49 AM, Jeff King <peff@peff.net> wrote:
> On Tue, Nov 22, 2016 at 07:30:19PM +0700, Nguyễn Thái Ngọc Duy wrote:
>
>> This is the follow up of rs/qsort series, merged in b8688ad (Merge
>> branch 'rs/qsort' - 2016-10-10), where coccinelle was used to do
>> automatic transformation.
>>
>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>> ---
>>   coccinelle missed this place, understandably, because it can't know
>>   that
>>
>>       sizeof(*entries->items)
>>
>>   is the same as
>>
>>       sizeof(*df_name_compare.items)
>>
>>   without some semantic analysis.
>
> That made me wonder why "entries" is used at all. Does it point to the
> same struct? But no, df_name_compare is a string list we create with the
> same list of strings.
>
> Which is why...
>
>> -     qsort(df_sorted_entries.items, entries->nr, sizeof(*entries->items),
>> +     QSORT(df_sorted_entries.items, entries->nr,
>>             string_list_df_name_compare);
>
> ...it's OK to use entries->nr here, and not df_sorted_entries.nr. It
> still seems a bit odd, though.

Argh.. I completely overlooked that entries->nr !

> Maybe it's worth making this:
>
>   QSORT(df_sorted_entries.items, df_sorted_entries.nr,
>         string_list_df_name_compare);
>
> while we're at it. Another possibility is:
>
>   df_sorted_entries.cmp = string_list_df_name_compare;
>   string_list_sort(&df_sorted_entries);
>
> It's not any shorter, but maybe it's conceptually simpler.

Agreed. Shall I re-roll with this?
-- 
Duy