From owner-svn-src-head@FreeBSD.ORG Wed Feb 20 09:58:42 2013 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id C4A5F1B0; Wed, 20 Feb 2013 09:58:42 +0000 (UTC) (envelope-from gkeramidas@gmail.com) Received: from mail-wi0-f180.google.com (mail-wi0-f180.google.com [209.85.212.180]) by mx1.freebsd.org (Postfix) with ESMTP id 9C09F11C; Wed, 20 Feb 2013 09:58:41 +0000 (UTC) Received: by mail-wi0-f180.google.com with SMTP id hi8so5953528wib.7 for ; Wed, 20 Feb 2013 01:58:40 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=x-received:sender:date:from:to:cc:subject:message-id:references :mime-version:content-type:content-disposition:in-reply-to; bh=e+VGxdvFOJ2tcs3QZ7Sg4JTbZULhwcPYpAU8gtogf4w=; b=j+o/65hLkAQh8WxYt3pOnNR3K3hDx9t/aIPEG/bgWQ307swl6CwAfrsvhvk2PkoyQM 2s9B4UPPyOh+6xsZzgEGebeeRNtlOvgW9CFat25dn0Z+JoErkNWQ4WlR6Vp9nrHwZY46 N0IB65/BMLIAVTTOXssuGfHc7kaSjCn+cSmJrn5ZN6nATD+Q0uLjU8dixAPa8tdCltoi jVa9fIWAs9f/o4HkTOdelaf2L27zMOoplAJjBD0+6Uzlxobu3tzvRWSpm6u5A5aupvML HJOVcdMCfoktDg6UmSTZoOBLml5Egkn2opefCNxS/93139l/D37yxBwON5c7korNoFhP syxg== X-Received: by 10.180.87.170 with SMTP id az10mr31561074wib.3.1361354320606; Wed, 20 Feb 2013 01:58:40 -0800 (PST) Received: from saturn (217-162-217-29.dynamic.hispeed.ch. [217.162.217.29]) by mx.google.com with ESMTPS id m6sm32800876wic.2.2013.02.20.01.58.38 (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Wed, 20 Feb 2013 01:58:39 -0800 (PST) Sender: Giorgos Keramidas Date: Wed, 20 Feb 2013 10:58:35 +0100 From: Giorgos Keramidas To: Stefan Farfeleder Subject: Re: svn commit: r247014 - head/lib/libc/stdlib Message-ID: <20130220095834.GD26651@saturn> References: <201302192357.r1JNveLq039940@svn.freebsd.org> <79B2F826-6CB5-4CD5-8C7D-8220833403EF@FreeBSD.org> <20130220094941.GA1438@mole.fafoe.narf.at> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20130220094941.GA1438@mole.fafoe.narf.at> Cc: svn-src-head@FreeBSD.org, mdf@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org, David Chisnall X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 20 Feb 2013 09:58:42 -0000 On 2013-02-20 10:49, Stefan Farfeleder 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.