Date: Wed, 20 Feb 2013 10:58:35 +0100 From: Giorgos Keramidas <keramida@FreeBSD.org> To: Stefan Farfeleder <stefanf@FreeBSD.org> Cc: svn-src-head@FreeBSD.org, mdf@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org, David Chisnall <theraven@FreeBSD.org> Subject: Re: svn commit: r247014 - head/lib/libc/stdlib Message-ID: <20130220095834.GD26651@saturn> In-Reply-To: <20130220094941.GA1438@mole.fafoe.narf.at> References: <201302192357.r1JNveLq039940@svn.freebsd.org> <CAMBSHm-ZPNAYj1tqs%2B8paYJgn69=zPe5O-tsn1dvn4t2156muA@mail.gmail.com> <79B2F826-6CB5-4CD5-8C7D-8220833403EF@FreeBSD.org> <20130220094941.GA1438@mole.fafoe.narf.at>
next in thread | previous in thread | raw e-mail | index | archive | help
On 2013-02-20 10:49, Stefan Farfeleder <stefanf@FreeBSD.org> wrote: >On Wed, Feb 20, 2013 at 09:32:43AM +0000, David Chisnall wrote: >>On 20 Feb 2013, at 08:25, mdf@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.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20130220095834.GD26651>