svn commit: r319507 - head/sys/fs/msdosfs

[Bruce Evans]

On Sun, 4 Jun 2017, Bryan Drewery wrote:

> On 6/4/17 4:27 PM, Ed Maste wrote:
>> On 4 June 2017 at 14:06, Bryan Drewery <bdrewery at freebsd.org> wrote:
>>>
>>> In r189170. It seems to me we should change systm.h back to a macro
>>> #define memmove(dst, src, len) bcopy((src), (dst), (len))
>>
>> Note that they're not quite equivalent: memmove returns dst, while
>> bcopy has no return value.
>>
>
> #define memmove(dst, src, len) (bcopy((src), (dst), (len)), dst)
>
> Obscure comma operator,

memmove() is a function (possibly implemented as a function without a
backing function in the kernel, although its man page doesn't document
this
   (memmove(9) doesn't exist, and memmove(3) doesn't document this)
but the above is an unsafe macro, even after adding the missing
parentheses.  Statement-expressions with inner variables named with
underscores would be needed to implement memmove() as a macro.

Some broken compilers call mem*() for struct copying even with
-ffreestanding.  This probably affects memcpy() but not memmove().

> or we just add an inline memmove in systm.h too.

That moves the namespace problems.  Arches can reasonably have 
optimized MD variants of it.  Except it is a style bug to have it
at all, and worse to optimize it.  We use HAVE_INLINE_XXX idefs
to handle this problem.  For memset(), we actually have the reverse
problem.  memset() is both extern in the non-library libkern and
static inline in <sys/libkern.h>.  It can't be both.  A mess of ifdefs
involving LIBKERN_INLINE and LIBKERN_BODY is used to get both at
different times.

In some trees, I use macros like the following to recover some
builtins:

XX #define	bzero(p, n) ({						\
XX 	if (__builtin_constant_p(n) && (n) <= 32)		\
XX 		__builtin_memset((p), 0, (n));			\
XX 	else							\
XX 		(bzero)((p), (n));				\
XX })
XX 
XX #define	memcpy(to, from, n)	__builtin_memcpy((to), (from), (n))

Note that __builtin_memcpy() often ends up calling memcpy(), even in the
-ffreestanding case when memcpy() shouldn't exist.

These functions may be implemented in either libkern.h or systm.h, but
should be in only 1.  It is unclear which of these places is a style
bug.  Clearly bzero() should be in systm.h since it is never in libkern.
I put the others there too.

Bruce