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
obrien at freebsd.org
Tue Sep 23 22:22:46 UTC 2008
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,
>>>> 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
>>>> assembly implementation. Therefore bcmp(9) should be preferred over
>>> 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
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 at FreeBSD.org)
More information about the cvs-all