Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 23 Sep 2008 15:22:45 -0700
From:      "David O'Brien" <obrien@freebsd.org>
To:        Sam Leffler <sam@freebsd.org>
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
Message-ID:  <20080923222245.GB6386@dragon.NUXI.org>
In-Reply-To: <48D90D37.4000808@freebsd.org>
References:  <200809231446.m8NEkQev007507@repoman.freebsd.org> <48D9038B.3040000@freebsd.org> <20080923151353.GC50098@dragon.NUXI.org> <48D90D37.4000808@freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, Sep 23, 2008 at 08:37:27AM -0700, Sam Leffler wrote:
> 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}.

Eh?  The bug is memcmp() was unable to produce the proper result.
Anywhere where the POSIX memcmp() is depended on for ordering cannot
easily be changed.

> I suggest you need to get changes of this sort reviewed and/or you need to 
> show you haven't introduced a performance regression.

So performance trumps correctness?  I don't think that's the right goal.

-- 
-- David  (obrien@FreeBSD.org)



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20080923222245.GB6386>