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

On Wed, Nov 23, 2016 at 12:49 AM, Jeff King [off-list ref] wrote:

On Tue, Nov 22, 2016 at 07:30:19PM +0700, Nguyễn Thái Ngọc Duy wrote:

quoted

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 <redacted>
---
  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...

quoted

-     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