Re: [PATCH 15/16] commit-reach: make can_all_from_reach... linear

[Jeff King <peff@peff.net>]

On Fri, Oct 05, 2018 at 08:48:27PM +0200, René Scharfe wrote:

> If the comparison function has proper types then we need to declare a
> version with void pointer parameters as well to give to qsort(3).  I
> think using cast function pointers is undefined.  Perhaps like this?

I think it's undefined, too, though we have many instances already.

> +#define DEFINE_SORT(name, type, compare)				\
> +static int compare##_void(const void *one, const void *two)		\
> +{									\
> +	return compare(one, two);					\
> +}									\
> +static void name(type base, size_t nmemb)				\
> +{									\
> +	const type dummy = NULL;					\
> +	if (nmemb > 1)							\
> +		qsort(base, nmemb, sizeof(base[0]), compare##_void);	\
> +	else if (0)							\
> +		compare(dummy, dummy);					\
> +}

I do like that this removes the need to have the code block aspart of
the macro.

Did you measure to see if there is any runtime impact?

As an aside, we may need to take a "scope" argument in case somebody
wants to do this in a non-static way. It would be nice if we could make
this "static inline", but I don't think even a clever compiler would be
able to omit the wrapper call.

-Peff