svn commit: r247014 - head/lib/libc/stdlib

[Giorgos Keramidas]

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.