Subject: Re: Upgrade of `struct vnd_ioctl'
To: Quentin Garnier <cube@cubidou.net>
From: Bill Studenmund <wrstuden@netbsd.org>
List: tech-kern
Date: 02/15/2007 09:26:53
--Fba/0zbH8Xs+Fj9o
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Thu, Feb 15, 2007 at 02:45:53PM +0100, Quentin Garnier wrote:
> On Wed, Feb 14, 2007 at 12:39:58PM -0800, Bill Studenmund wrote:
> [...]
> > > What you need to do in the OVNDIOxxx cases is actually *convert* data
> > > (which points to a struct oldvnd_ioctl) into a struct vnd_ioctl.
> >=20
> > Actually, this time I think we can get away with a little type casting=
=20
> > magic. As I understand it, we are changing the length of the last field=
,=20
> > so we need only adjust how we handle it. Note: I'm also assuming we'll =
not=20
> > add more components in the future; if we were to, we should make the=20
> > above-suggested change for conversion.
> >=20
> > 	vio->vnd_size =3D dbtob(((struct vnd_oioctl *)vnd)->sc_size);
> >=20
> > should work for now.
>=20
> Okay, so first of all, I'm just back from California, I haven't slept
> for 24 hours and spending 11 hours on a plane is not the best way to
> get clear ideas.  So please assume good faith :-)

Ok. I will.

Suggestion #1. Go sleep!

> I *think* you're missing the fact that vio is the vnd_ioctl _or_
> oldvnd_ioctl struct pointer.  vnd is the considered device's softc.

Yes, I was confused about which pointer is which.

> So whatever gross hack you use around the issue won't guarantee that
> you don't end up writing 4 bytes after the end of the actually malloc'd
> struct.

Ok. So here's the place where I assume it was sleep that led you to not=20
realize that I had the pointers backwards. :-)

It's not that gross a hack. It's being explicit about which pointer is=20
which. Here's what I had in mind, with the right pointers in use:

               if (cmd =3D=3D VNDIOOCSET) {
                       struct oldvnd_ioctl *ovio =3D (void *)vio;
                       ovio->vnd_size =3D dbtob(vnd->sc_size);
               } else

> IOW, I'd rather see it done the clean way, which is actually not that
> much a hassle :-)

If Arnaud wants to make the change you suggest, great! If not, however,
what do we do? Are you up for doing it? If so, please do it. If you
aren't, then maybe the change isn't so trivial. ;-)

If we were talking about changing anything other than the tail field or we=
=20
had more than two variants, I'd agree with you that we should have=20
filtering routines to map into what the old calls wanted.

However we don't. Once the other comments are addressed, we have something=
=20
that is ok. It's not wrong, even if it doesn't meet a measure of=20
asthetics.

I'd rather have a good, correct change in-tree than not have a better fix=
=20
not in the tree. :-)

Oh, if we do go with the casting version (what we have so far), then we=20
need a comment indicating that the VNDIOOCSET processing (or OVNDIOCSET if=
=20
we go with that name) assumes that the only difference is in the last=20
component.

Take care,

Bill

--Fba/0zbH8Xs+Fj9o
Content-Type: application/pgp-signature
Content-Disposition: inline

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

iD8DBQFF1JfdWz+3JHUci9cRAt1pAJ9YqbSgKH1jU2VNgyKuu9MfKKGo9ACcD+hr
XIprpFMnoZgcAFHX3lGj0vs=
=eDEu
-----END PGP SIGNATURE-----

--Fba/0zbH8Xs+Fj9o--