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

[Sam Leffler]

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