On Wed, Nov 23, 2016 at 12:49 AM, Jeff King <peff@xxxxxxxx> 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@xxxxxxxxx> >> --- >> 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