From owner-cvs-all@FreeBSD.ORG Tue Sep 23 15:37:34 2008 Return-Path: Delivered-To: cvs-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 60BF71065671; Tue, 23 Sep 2008 15:37:34 +0000 (UTC) (envelope-from sam@freebsd.org) Received: from ebb.errno.com (ebb.errno.com [69.12.149.25]) by mx1.freebsd.org (Postfix) with ESMTP id 11B238FC0C; Tue, 23 Sep 2008 15:37:28 +0000 (UTC) (envelope-from sam@freebsd.org) Received: from trouble.errno.com (trouble.errno.com [10.0.0.248]) (authenticated bits=0) by ebb.errno.com (8.13.6/8.12.6) with ESMTP id m8NFbRZt068355 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Tue, 23 Sep 2008 08:37:28 -0700 (PDT) (envelope-from sam@freebsd.org) Message-ID: <48D90D37.4000808@freebsd.org> Date: Tue, 23 Sep 2008 08:37:27 -0700 From: Sam Leffler Organization: FreeBSD Project User-Agent: Thunderbird 2.0.0.9 (X11/20071125) MIME-Version: 1.0 To: obrien@freebsd.org References: <200809231446.m8NEkQev007507@repoman.freebsd.org> <48D9038B.3040000@freebsd.org> <20080923151353.GC50098@dragon.NUXI.org> In-Reply-To: <20080923151353.GC50098@dragon.NUXI.org> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-DCC-Rhyolite-Metrics: ebb.errno.com; whitelist Cc: cvs-src@freebsd.org, src-committers@freebsd.org, cvs-all@freebsd.org Subject: Re: cvs commit: src/sys/conf files src/sys/fs/tmpfs tmpfs.h tmpfs_subr.c tmpfs_vnops.c src/sys/i386/i386 bios.c src/sys/ia64/ia64 efi.c sal.c src/sys/libkern memcmp.c src/sys/mips/mips support.S src/sys/sys libkern.h X-BeenThere: cvs-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: CVS commit messages for the entire tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 23 Sep 2008 15:37:34 -0000 David O'Brien wrote: > On Tue, Sep 23, 2008 at 07:56:11AM -0700, Sam Leffler wrote: > >> David E. O'Brien wrote: >> >>> obrien 2008-09-23 14:45:10 UTC >>> FreeBSD src repository >>> Modified files: >>> sys/conf files sys/fs/tmpfs tmpfs.h >>> tmpfs_subr.c tmpfs_vnops.c sys/i386/i386 bios.c >>> sys/ia64/ia64 efi.c sal.c sys/mips/mips support.S >>> sys/sys libkern.h Added files: >>> sys/libkern memcmp.c Log: >>> SVN rev 183299 on 2008-09-23 14:45:10Z by obrien >>> The kernel implemented 'memcmp' is an alias for 'bcmp'. However, >>> memcmp >>> and bcmp are not the same thing. 'man bcmp' states that the return is >>> "non-zero" if the two byte strings are not identical. Where as, >>> 'man memcmp' states that the return is the "difference between the >>> first two differing bytes (treated as unsigned char values" if the >>> two byte strings are not identical. >>> So provide a proper memcmp(9), but it is a C implementation not a >>> tuned >>> assembly implementation. Therefore bcmp(9) should be preferred over >>> memcmp(9). >>> >>> >> Given the performance difference this change should have been reviewed >> before dumping it into the tree. >> > > >> I do not agree with this; >> > > You do not agree with fixing a bug in our code? > You don't have to "fix a bug in our code" in this way. You could have, for example, fixed the cases where the return result was checked against !{0,1}. Then after having an implementation that was comparable in performance to bcmp switched over completely. > >> did you post it somewhere? >> > > No. > > I suggest you need to get changes of this sort reviewed and/or you need to show you haven't introduced a performance regression. Sam