Date: Fri, 2 Mar 2018 12:43:34 -0500 From: Pedro Giffuni <pfg@FreeBSD.org> To: Konstantin Belousov <kib@FreeBSD.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Cc: Eitan Adler <lists@eitanadler.com> Subject: Re: svn commit: r330285 - head/sys/sys Message-ID: <dba3b880-58d0-e6df-8f99-4d27919134c9@FreeBSD.org> In-Reply-To: <201803021647.w22Gl2t7092316@repo.freebsd.org> References: <201803021647.w22Gl2t7092316@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
(cc in Eitan as he may be interested in the extra restrict cases) On 02/03/2018 11:47, Konstantin Belousov wrote: > Author: kib > Date: Fri Mar 2 16:47:02 2018 > New Revision: 330285 > URL: https://svnweb.freebsd.org/changeset/base/330285 > > Log: > Remove _Nonnull attributes from user addresses arguments for > copyout(9) family. > > The addresses are user-controllable, and if the process ABI allows > mapping at zero, then the zero address is meaningful, contradicting > the definition of _Nonnull. In any case, it does not require any > special code to handle NULL udaddr. > FWIW, the _Nonnull attributes didn't do much at all beyond producing a warning. They replaced the GNU __nonnull() attributes which were much more dangerous. I am OK with seeing both gone here though. > It is not clear if __restrict makes sense as well, since kaddr and > udaddr point to different address spaces, so equal numeric values of > the pointers do not imply aliasing and a legitimate. But leave it for > later. > > copyinstr(9) does not have its user address argument annotated. I think use of _Nonnull attributes in the threading functions may also be a waste (I introduced them mostly to be compatible with Android). FWIW, Dragonfly sprinkled some restrict there recently: http://gitweb.dragonflybsd.org/dragonfly.git/commit/d33005aaee6af52c80428b59b52aee522c002492 Just in case someone is considering more cleanups. Cheers, Pedro. > Sponsored by: The FreeBSD Foundation > MFC after: 1 week > > Modified: > head/sys/sys/systm.h > > Modified: head/sys/sys/systm.h > ============================================================================== > --- head/sys/sys/systm.h Fri Mar 2 16:31:23 2018 (r330284) > +++ head/sys/sys/systm.h Fri Mar 2 16:47:02 2018 (r330285) > @@ -277,14 +277,14 @@ int copystr(const void * _Nonnull __restrict kfaddr, > int copyinstr(const void * __restrict udaddr, > void * _Nonnull __restrict kaddr, size_t len, > size_t * __restrict lencopied); > -int copyin(const void * _Nonnull __restrict udaddr, > +int copyin(const void * __restrict udaddr, > void * _Nonnull __restrict kaddr, size_t len); > -int copyin_nofault(const void * _Nonnull __restrict udaddr, > +int copyin_nofault(const void * __restrict udaddr, > void * _Nonnull __restrict kaddr, size_t len); > int copyout(const void * _Nonnull __restrict kaddr, > - void * _Nonnull __restrict udaddr, size_t len); > + void * __restrict udaddr, size_t len); > int copyout_nofault(const void * _Nonnull __restrict kaddr, > - void * _Nonnull __restrict udaddr, size_t len); > + void * __restrict udaddr, size_t len); > > int fubyte(volatile const void *base); > long fuword(volatile const void *base); >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?dba3b880-58d0-e6df-8f99-4d27919134c9>