On 2013-02-20 10:49, Stefan Farfeleder <stef...@freebsd.org> wrote: >On Wed, Feb 20, 2013 at 09:32:43AM +0000, David Chisnall wrote: >>On 20 Feb 2013, at 08:25, m...@freebsd.org wrote: >>> These should be declared const int *. And the cast shouldn't be >>> needed in C, since void * can be assigned to any other pointer type. >> >> In fact, the entire function body can be replaced with: >> >> return (*(int*)p1 - *(int*)p2); >> >> qsort doesn't require that you return -1, 0, or 1, it requires you return >> <0, 0, or >0. > > The subtraction might overflow and give wrong results. It won't for > these specific elements, but it would be a bad example, IMHO.
That's a good point. The Linux version of the manpage uses a string comparison function as an example, *and* a subtraction, which then requires a lengthy comment to explain what's happening and why all the casts: static int cmpstringp(const void *p1, const void *p2) { /* The actual arguments to this function are "pointers to pointers to char", but strcmp(3) arguments are "pointers to char", hence the following cast plus dereference */ return strcmp(* (char * const *) p1, * (char * const *) p2); } Now I prefer sticking with the rather explicit and rather simple to understand version: /* * Custom comparison function that can compare 'int' values through pointers * passed by qsort(3). */ static int int_compare(const void *p1, const void *p2) { const int *left = p1; const int *right = p2; if (*left < *right) return (-1); else if (*left > *right) return (1); else return (0); } Even the comment is not stricly needed. The code is simpler than the version with the casts, especially if the casts have to be repeated to avoid subtraction induced underflows. _______________________________________________ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"