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

From: Jeff King <peff@peff.net>
Date: Nov 22, 2016, 17:49 UTC
Message-ID: <20161122174946.jy5at4g7rifu3und@sigill.intra.peff.net>
In-Reply-To: <20161122123019.7169-1-pclouds@gmail.com>

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

Show 16 quoted lines

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

-Peff