svn commit: r333265 - head/sys/sys

[Bruce Evans]

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