Re: [PATCH] merge-recursive.c: use QSORT macro
On Wed, Nov 23, 2016 at 12:49 AM, Jeff King <peff@peff.net> wrote:
Show 31 quoted lines
> 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 !
Show 11 quoted lines
> 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