Re: oskit driver for i8042 keyboard controller

[Marcus Brinkmann]

On Sun, Nov 10, 2002 at 02:05:20AM +0100, Marcus Brinkmann wrote:
> On Fri, Nov 08, 2002 at 10:15:51PM -0500, Roland McGrath wrote:
> > > Ok, I can try to find out where the references come from.
> > 
> > Please do.
> 
> ds_notify does a dev_port_lookup that acuires a reference.  This reference
> is not released.  So when the last user goes away, and the notification is
> sent, there is one reference.  Then the refcount is increased to 2 by
> dev_port_lookup, and then lowered to 1.  The device is never actually
> released.
> 
> Now, I added a second device_deallocate to this function.  This makes sure
> the device is actually destroyed.  However, this breaks stuff: The minimal
> console doesn't seem to work anymore, and the problem that I can not open
> the kbd device for readonly once and then for writing the second time still
> persists.  So I am not going to check in this change right now.  This requires
> more debugging and/or code staring.

Hi,

your changes to the code don't have a net effect on what happens.  Although
you now deallocate twice in the notification handler, we have three
references for a device after open.  That means that the device is never
deallocated, and a subsequent open with different open modes fails.  In the
new version of the code you get an extra reference for the hash entry, but
the device is removed in the hash only after all references went away, so
that looks definitely bogus to me.

BTW, you write (mode &~ dev->mode) in one place.  This confused me a bit,
(mode & ~dev->mode) looks more readable to me.  The code as written seems to
check if the mode is a subset of dev->mode.  That means if I open for
reading I might also be allowed to write (as subsequent checks are done on
dev->mode).  That seems a bit broken to me (ie, what if I pass on the device
port to less privileged users).  Do you think we should keep the open mode
in an extra field?





-- 
`Rhubarb is no Egyptian god.' GNU      http://www.gnu.org    marcus@gnu.org
Marcus Brinkmann              The Hurd http://www.gnu.org/software/hurd/
Marcus.Brinkmann@ruhr-uni-bochum.de
http://www.marcus-brinkmann.de/