René Scharfe <[email protected]> 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 <[email protected]>
> ---
> 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".
3. builtin/show-branch.c does this:
qsort(ref_name + bottom, top - bottom, sizeof(ref_name[0]),
compare_ref_name);
where ref_name[] is a file-scope global:
static char *ref_name[MAX_REVS + 1];
and top and bottom are plain integers. The sizeof() does not take
the size of *base, so it is understandable that this does not get
automatically converted.
It seems that some calls to this function _could_ send the same top
and bottom, asking for 0 element array to be sorted, by the way.
Thanks for an amusing read.
