Re: [PATCH 1/3] add QSORT

> René Scharfe <l.s.r@web.de> writes:
> 
>> Add the macro QSORT, a convenient wrapper for qsort(3) that infers the
>> size of the array elements and supports the convention of initializing
>> empty arrays with a NULL pointer, which we use in some places.
>>
>> Calling qsort(3) directly with a NULL pointer is undefined -- even with
>> an element count of zero -- and allows the compiler to optimize away any
>> following NULL checks.  Using the macro avoids such surprises.
>>
>> Add a semantic patch as well to demonstrate the macro's usage and to
>> automate the transformation of trivial cases.
>>
>> Signed-off-by: Rene Scharfe <l.s.r@web.de>
>> ---
>>  contrib/coccinelle/qsort.cocci | 19 +++++++++++++++++++
>>  git-compat-util.h              |  8 ++++++++
>>  2 files changed, 27 insertions(+)
>>  create mode 100644 contrib/coccinelle/qsort.cocci
> 
> The direct calls to qsort(3) that this series leaves behind are
> interesting.
> 
> 1. builtin/index-pack.c has this:
> 
> 	if (1 < opts->anomaly_nr)
> 		qsort(opts->anomaly, opts->anomaly_nr, sizeof(uint32_t), cmp_uint32);
> 
> where opts->anomaly is coming from pack.h:
> 
>     struct pack_idx_option {
>             unsigned flags;
>             ...
>             int anomaly_alloc, anomaly_nr;
>             uint32_t *anomaly;
>     };
> 
> I cannot quite see how the automated conversion misses it?  It's not
> like base and nmemb are type-restricted in the rule (they are both
> just "expression"s).
> 
> 2. builtin/shortlog.c has this:
> 
> 	qsort(log->list.items, log->list.nr, sizeof(struct string_list_item),
> 	      log->summary ? compare_by_counter : compare_by_list);
> 
> where log->list is coming from shortlog.h:
> 
>     struct shortlog {
>             struct string_list list;
>     };
> 
> and string-list.h says:
> 
>     struct string_list {
>             struct string_list_item *items;
>             unsigned int nr, alloc;
>             ...
>     };
> 
> which seems to be a good candidate for this rule:
> 
>     type T;
>     T *base;
>     expression nmemb, compar;
>     @@
>     - qsort(base, nmemb, sizeof(T), compar);
>     + QSORT(base, nmemb, compar);
> 
> if we take "T == struct string_list_item".