Subject: Re: COPYIN/COPYOUT macro problems Re: IOCTL implementation and kernel/userland addresses
To: Frank van der Linden <fvdl@netbsd.org>
From: Rui Paulo <rpaulo@NetBSD.org>
List: tech-kern
Date: 08/25/2005 17:43:51
--XsQoSWH+UP9D9v3l
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On 2005.08.25 18:35:50 +0000, Reinoud Zandijk wrote:
| Dear folks,
|=20
| picking up an old thread on the added FKIOCTL flag :
|=20
| On Thu, Feb 17, 2005 at 12:26:36AM +0100, Reinoud Zandijk wrote:
| > On Wed, Feb 16, 2005 at 11:37:56PM +0100, Frank van der Linden wrote:
| > > On Wed, Feb 16, 2005 at 11:25:39PM +0100, Reinoud Zandijk wrote:
| > > > --- sys/fcntl.h 3 Feb 2005 19:20:01 -0000       1.29
| > > > +++ sys/fcntl.h 16 Feb 2005 22:21:31 -0000
| > > > @@ -124,11 +124,12 @@
| > > >  #define        FMARK           0x00001000      /* mark during gc()=
 */
| > > >  #define        FDEFER          0x00002000      /* defer for next g=
c pass */
| > > >  #define        FHASLOCK        0x00004000      /* descriptor holds=
 advisory lock */
| > > > +#define FKIOCTL                0x80000000
| > > >  /* bits to save after open(2) */
| > > >  #define        FMASK          =20
| > > > (FREAD|FWRITE|FAPPEND|FASYNC|FFSYNC|FNONBLOCK|FDSYNC|\
| > > > -                        FRSYNC|FALTIO)
| > > > +                        FRSYNC|FALTIO|FKIOCTL)
| > > >  /* bits settable by fcntl(F_SETFL, ...) */
| > > So, just add the value for FKIOCTL, and you're done there.
|=20
|=20
| > still remains the point of defining a COPYOUT() and friends macro's ? o=
r a=20
| > function?
|=20
| I've defined COPYIN and COPYOUT macros wich allways DTRT for ioctls in th=
e=20
| following proposed patch :
|=20
| ------------------
| diff -u -r1.179 systm.h
| --- sys/systm.h 23 Jun 2005 00:30:28 -0000      1.179
| +++ sys/systm.h 25 Aug 2005 16:22:28 -0000
| @@ -240,6 +240,21 @@
|  int    copyin(const void *, void *, size_t);
|  int    copyout(const void *, void *, size_t);
| =20
| +#ifdef _KERNEL
| +#define COPYIN(ioctlflags, uaddr, kaddr, len)\
| +               if ((ioctlflags) & FKIOCTL) {\
| +                       memcpy((kaddr), (uaddr), (len));\
| +               } else {\
| +                       copyin((uaddr), (kaddr), (len));\
| +               };
| +#define COPYOUT(ioctlflags, kaddr, uaddr, len)\
| +               if ((ioctlflags) & FKIOCTL) {\
| +                       memcpy((uaddr), (kaddr), (len));\
| +               } else {\
| +                       copyout((kaddr), (uaddr), (len));\
| +               };
| +#endif /* KERNEL */
| +
|  int    copyin_proc(struct proc *, const void *, void *, size_t);
|  int    copyout_proc(struct proc *, const void *, void *, size_t);
| ----------------------
|=20
| *BUT* a grep in src/sys pointed to the following problems:
|=20
| arch/pc532/fpu/ieee_handler.c
| dist/pf/net/pf_table.c
| dist/ipf/netinet/ip_compat.h
|=20
| all define their own COPYIN() and COPYOUT() macro's.... now what to do?
|    a) don't use COPYIN/COPYOUT but use IOCTL_COPYIN and IOCTL_COPYOUT ?
|    b) patch somehow pc532/pf/ipf to not use such generic names?
|    c) ...
|=20
| (a) would do justice to the fact that they are only usable for IOCTL flag=
s=20
| (b) might cause problems with importing newer pf/ipf or are those imports=
=20
| mainly scripted?
| (c) other suggestions?

I vote for (a).
For (b), you could change pc532, change ipf2netbsd script and create a new
pf2netbsd script, but that's just too overhead for something that could be
done easily with a name change.

		-- Rui Paulo

--XsQoSWH+UP9D9v3l
Content-Type: application/pgp-signature
Content-Disposition: inline

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (NetBSD)

iD8DBQFDDfVHZPqyxs9FH4QRAjeCAJwJ9vu7g/LliObH33cclPf72tIOBQCcCZla
7PkxVCdLc+2bOhL1f2F+xsc=
=z4MP
-----END PGP SIGNATURE-----

--XsQoSWH+UP9D9v3l--