Am 30.09.2016 um 01:21 schrieb René Scharfe:
> Am 30.09.2016 um 00:36 schrieb Junio C Hamano:
>> René Scharfe <l.s.r@xxxxxx> 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@xxxxxx>
>>> ---
>>> 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".
>
> Transformations for these two are generated if we pass --all-includes
> to spatch. So let's do that.
And here's the result:
-- >8 --
Subject: [PATCH] use QSORT, part 2
Convert two more qsort(3) calls to QSORT to reduce code size and for
better safety and consistency.
Signed-off-by: Rene Scharfe <l.s.r@xxxxxx>
---
Squashable.
builtin/index-pack.c | 3 +--
builtin/shortlog.c | 2 +-
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 7657d0a..0a27bab 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -1531,8 +1531,7 @@ static void read_v2_anomalous_offsets(struct packed_git *p,
opts->anomaly[opts->anomaly_nr++] = ntohl(idx2[off * 2 + 1]);
}
- if (1 < opts->anomaly_nr)
- qsort(opts->anomaly, opts->anomaly_nr, sizeof(uint32_t), cmp_uint32);
+ QSORT(opts->anomaly, opts->anomaly_nr, cmp_uint32);
}
static void read_idx_option(struct pack_idx_option *opts, const char *pack_name)
diff --git a/builtin/shortlog.c b/builtin/shortlog.c
index 25fa8a6..ba0e115 100644
--- a/builtin/shortlog.c
+++ b/builtin/shortlog.c
@@ -308,7 +308,7 @@ void shortlog_output(struct shortlog *log)
struct strbuf sb = STRBUF_INIT;
if (log->sort_by_number)
- qsort(log->list.items, log->list.nr, sizeof(struct string_list_item),
+ QSORT(log->list.items, log->list.nr,
log->summary ? compare_by_counter : compare_by_list);
for (i = 0; i < log->list.nr; i++) {
const struct string_list_item *item = &log->list.items[i];
--
2.10.0