svn commit: r247014 - head/lib/libc/stdlib
Giorgos Keramidas
keramida at FreeBSD.org
Wed Feb 20 09:58:42 UTC 2013
On 2013-02-20 10:49, Stefan Farfeleder <stefanf at FreeBSD.org> wrote:
>On Wed, Feb 20, 2013 at 09:32:43AM +0000, David Chisnall wrote:
>>On 20 Feb 2013, at 08:25, mdf at 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.
More information about the svn-src-head
mailing list