Date: Mon, 8 Mar 2004 17:24:38 -0800 From: Brooks Davis <brooks@one-eyed-alien.net> To: Stefan `Sec` Zehl <sec@42.org> Cc: freebsd-bugs@freebsd.org Subject: Re: kern/63772: tap device / exclusive open problem Message-ID: <20040309012438.GA20988@Odin.AC.HMC.Edu> In-Reply-To: <200403090050.i290oEuG015002@freefall.freebsd.org> References: <200403090050.i290oEuG015002@freefall.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--wRRV7LY7NUeQGEoC
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On Mon, Mar 08, 2004 at 04:50:14PM -0800, Stefan `Sec` Zehl wrote:
> The following reply was made to PR kern/63772; it has been noted by GNATS.
>=20
> From: Stefan `Sec` Zehl <sec@42.org>
> To: Brooks Davis <brooks@one-eyed-alien.net>
> Cc: FreeBSD-gnats-submit@freebsd.org, freebsd-bugs@freebsd.org
> Subject: Re: kern/63772: tap device / exclusive open problem
> Date: Tue, 9 Mar 2004 01:46:45 +0100
>=20
> On Fri, Mar 05, 2004 at 10:55 -0800, Brooks Davis wrote:
> =20
> [...]
> =20
> > > KASSERT(!(tp->tap_flags & TAP_OPEN),=20
> > > ("%s flags is out of sync", tp->tap_if.if_xname));
> > > =20
> > > + if (tp->tap_flags & TAP_OPEN)
> > > + return (EBUSY);
> > > +
> [...]
> =20
> =20
> > There's obviously something complicated going on since with INVARIANTS
> > enabled this would either return EBUSY or panic. This should definite=
ly
> > not be committed as is.
> =20
> I must admit, I don't understand the problem. this is an really simple
> if. Nothing is modified, and tp is not NULL, or the KASSERT above would
> be broken already.
> =20
> How do you get it to panic?
> =20
> It works fine for me.
The problem is very simple. Since !(tp->tap_flags & TAP_OPEN) should
never happen (and results in a panic in the KASSERT if INVARIANTS is
enabled) your "fix" effectivly removes the entire rest of this
function. It may be that the rest of that code is junk, but if that's
the case, it should be removed not bypassed by a bogus conditional.
With this patch and INVARIANTS, you end up with:
function()
{
...
if (!A)
panic;
if (A)
return (EBUSY)
lots
of
unreachable
code
...
}
This demonstrates a lack of sufficent understanding of the code so while
I believe that it works for you, it should not be commited as is.
Creating unreachable code is not a good idea.
-- Brooks
--=20
Any statement of the form "X is the one, true Y" is FALSE.
PGP fingerprint 655D 519C 26A7 82E7 2529 9BF0 5D8E 8BE9 F238 1AD4
--wRRV7LY7NUeQGEoC
Content-Type: application/pgp-signature
Content-Disposition: inline
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.1 (GNU/Linux)
iD8DBQFATRzVXY6L6fI4GtQRAjhqAJ9ZS2bA24PIFkxnCY1tJ5R6ALapoACgpwqY
E2slgrX5asTc5WLE/FnnK9o=
=dFFY
-----END PGP SIGNATURE-----
--wRRV7LY7NUeQGEoC--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20040309012438.GA20988>