Date: Mon, 5 Jun 2017 21:55:20 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Bryan Drewery <bdrewery@FreeBSD.org> Cc: Ed Maste <emaste@FreeBSD.org>, Bruce Evans <brde@optusnet.com.au>, "src-committers@freebsd.org" <src-committers@FreeBSD.org>, "svn-src-all@freebsd.org" <svn-src-all@FreeBSD.org>, "svn-src-head@freebsd.org" <svn-src-head@FreeBSD.org> Subject: Re: svn commit: r319507 - head/sys/fs/msdosfs Message-ID: <20170605212605.B929@besplex.bde.org> In-Reply-To: <721fdeb3-b9d6-b8f4-9025-a5ae7bfd295f@FreeBSD.org> References: <201706021839.v52IdrZJ074478@repo.freebsd.org> <20170603161546.D939@besplex.bde.org> <543a4144-4f08-8742-5455-0103569d9f4f@FreeBSD.org> <CAPyFy2BZR62MtuckgrC6crRNDgbfMDvd0d5mjONHpxbCgqTxxg@mail.gmail.com> <721fdeb3-b9d6-b8f4-9025-a5ae7bfd295f@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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@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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20170605212605.B929>