On Thu, Mar 19, 2009 at 04:02:19AM -0700, Junio C Hamano wrote:
> > Do we really want an API for that? Calling qsort() directly should be
> > obvious enough, no?
>
> I think so. If it were done like this (notice the lack of double
> indirection in the cmp_fn signature):
>
> typedef int string_list_item_cmp_fn(const struct string_list_item *, const struct string_list_item *);
>
> void sort_string_list_with_fn(struct string_list *list, string_list_item_cmp_fn *);
>
> it would have made more sense, though.
IIRC, that is actually not valid C according to the standard (that is,
even though a void* can be implicitly assigned to any other pointer, a
function taking a void* and a function taking another pointer do not
necessarily have the same function signature or calling conventions).
Which is why cmp_items in string-list.c already does the indirection.
That being said, I think this is probably one of those cases where it
works fine on any sane platform that we care about and we can choose to
ignore the standard.
Actually, I think an even nicer API would be to give a function that
_just_ compares the util fields; string_list_sort() would sort on the
"string" fields, and only call the util cmp to sort entries with equal
strings. Sadly, because qsort lacks a slot for extra data to pass to the
callback, implementing this ends up with a global variable:
static int (*cmp_util)(const void *, const void *);
int cmp_items(const void *a, const void *b)
{
const struct string_list_item *one = a;
const struct string_list_item *two = b;
int cmp = strcmp(one->string, two->string);
return cmp ? cmp : cmp_util(one->util, b->util);
}
int sort_string_list_with_util(struct string_list *list,
int (*fn)(const void *, const void *))
{
cmp_util = fn;
qsort(list->items, list->nr, sizeof(*list->items), cmp_items);
}
If only C had closures. ;)
But this is really getting beyond the issue at hand. I can submit a
finalized patch; just let me know how you prefer it.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
