On Fri, 4 May 2018, Mateusz Guzik wrote:

Log:
 Allow the compiler to use __builtin_memcpy

 In particular this allows the compiler to avoid heavy-handed machinery
 if the to be copied buffer is small.

 Reviewed by:   jhb

Bugs in this:

Modified: head/sys/sys/systm.h
==============================================================================
--- head/sys/sys/systm.h        Fri May  4 21:17:29 2018        (r333264)
+++ head/sys/sys/systm.h        Fri May  4 22:33:54 2018        (r333265)
@@ -275,6 +275,7 @@ void        bzero(void * _Nonnull buf, size_t len);
void    explicit_bzero(void * _Nonnull, size_t);

void    *memcpy(void * _Nonnull to, const void * _Nonnull from, size_t len);
+#define memcpy(to, from, len) __builtin_memcpy(to, from, len)

- space instead of tab after #define.  systm.h has rotted to have several
  other instances of this bug
- space instead of tab between macro and macro expansion.  This bug is less
  common.
- args not parenthesized.

I wonder how memcpy() handles attributes like _Nonnull.  FreeBSD doesn't
declare any attributes for builtins (is this possible?) so it gets the ones
that the compiler defaults to, if any.  I haven't noticed any documentation
of these defaults even for compilers like gcc that have documentation.

void    *memmove(void * _Nonnull dest, const void * _Nonnull src, size_t n);

int     copystr(const void * _Nonnull __restrict kfaddr,

Modified: head/sys/sys/zutil.h
==============================================================================
--- head/sys/sys/zutil.h        Fri May  4 21:17:29 2018        (r333264)
+++ head/sys/sys/zutil.h        Fri May  4 22:33:54 2018        (r333265)
@@ -32,7 +32,6 @@
#include <sys/param.h>
#include <sys/kernel.h>
#  define HAVE_MEMCPY
-#  define memcpy(d, s, n)      bcopy((s), (d), (n))
#  define memset(d, v, n)       bzero((d), (n))
#  define memcmp                bcmp

Example of parenthesizing args in old code.  Example of other style bugs
for #define (spaces after '#' to indent).

Reduction of portability here.  I think zutil.h tries to provide its own
wrappers for everything.  It still does this for memset() although this
is wrong when (n) != 0.

I think you removed memcpy() since memcpy() is now implemented as an
inconsistent macro, while memset() is still a function in systm.h so there
is no conflict for memset().  Since there is no conflict, the bug is not
even detected.

memcmp() here is even more broken that memset().  memcmp() cannot be
implemented using bcmp(), since it is tri-state while bcmp() is boolean.
The system memcmp() had the same bug for a long time.

Bruce
_______________________________________________
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Reply via email to