From owner-svn-src-all@freebsd.org Mon Jun 5 11:55:34 2017 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 6B595BFD2D0; Mon, 5 Jun 2017 11:55:34 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail105.syd.optusnet.com.au (mail105.syd.optusnet.com.au [211.29.132.249]) by mx1.freebsd.org (Postfix) with ESMTP id 2C3227B5B7; Mon, 5 Jun 2017 11:55:33 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from besplex.bde.org (c122-106-153-191.carlnfd1.nsw.optusnet.com.au [122.106.153.191]) by mail105.syd.optusnet.com.au (Postfix) with ESMTPS id 092F5104AC59; Mon, 5 Jun 2017 21:55:21 +1000 (AEST) Date: Mon, 5 Jun 2017 21:55:20 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Bryan Drewery cc: Ed Maste , Bruce Evans , "src-committers@freebsd.org" , "svn-src-all@freebsd.org" , "svn-src-head@freebsd.org" Subject: Re: svn commit: r319507 - head/sys/fs/msdosfs In-Reply-To: <721fdeb3-b9d6-b8f4-9025-a5ae7bfd295f@FreeBSD.org> Message-ID: <20170605212605.B929@besplex.bde.org> References: <201706021839.v52IdrZJ074478@repo.freebsd.org> <20170603161546.D939@besplex.bde.org> <543a4144-4f08-8742-5455-0103569d9f4f@FreeBSD.org> <721fdeb3-b9d6-b8f4-9025-a5ae7bfd295f@FreeBSD.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.2 cv=WvBbCZXv c=1 sm=1 tr=0 a=Tj3pCpwHnMupdyZSltBt7Q==:117 a=Tj3pCpwHnMupdyZSltBt7Q==:17 a=kj9zAlcOel0A:10 a=6I5d2MoRAAAA:8 a=C1beX9IksOr_2a5XRB4A:9 a=CjuIK1q_8ugA:10 a=IjZwj45LgO3ly-622nXo:22 X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 05 Jun 2017 11:55:34 -0000 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 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 . 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