Am 21.12.2016 um 17:12 schrieb Jeff King:
On Wed, Dec 21, 2016 at 10:36:41AM +0100, René Scharfe wrote:
One shortcoming is that the comparison function is restricted to working
with the string members of items; util is inaccessible to it. Another
one is that the value of cmp is passed in a global variable to
cmp_items(), making string_list_sort() non-reentrant.
I think this approach is OK for string_list, but it doesn't help the
general case that wants qsort_s() to actually access global data. I
don't know how common that is in our codebase, though.
So I'm fine with it, but I think we might eventually need to revisit the
qsort_s() thing anyway.
I have to admit I didn't even consider the possibility that the pattern
of accessing global variables in qsort(1) compare functions could have
spread.
And indeed, at least ref-filter.c::compare_refs() and
worktree.c::compare_worktree() so that as well. The latter uses
fspathcmp(), which is OK as ignore_case is only set once when reading
the config, but the first one looks, well, interesting. Perhaps a
single ref filter per program is enough?
Anyway, that's a good enough argument for me for adding that newfangled
C11 function after all..
Btw.: Found with
git grep 'QSORT.*;$' |
sed 's/.* /int /; s/);//' |
sort -u |
git grep -Ww -f-
and
git grep -A1 'QSORT.*,$'
Remove the intermediate layer, i.e. cmp_items(), make the comparison
functions compatible with qsort(3) and pass them pointers to full items.
This allows comparisons to also take the util member into account, and
avoids the need to pass the real comparison function to an intermediate
function, removing the need for a global function.
I'm not sure if access to the util field is really of any value, after
looking at it in:
http://public-inbox.org/git/20161125171546.fa3zpapbjngjcl26@xxxxxxxxxxxxxxxxxxxxx/
Though note that if we do take this patch, there are probably one or two
spots that could switch from QSORT() to string_list_sort().
Yes, but as you noted in that thread there is not much point in doing
that; the only net win is that we can pass a list as a single pointer
instead of as base pointer and element count -- the special compare
function needs to be specified anyway (once), somehow.
René