[PATCH] unload input modules when they are no longer used

[Peter Hutterer]

On Mon, Jul 02, 2012 at 03:30:02PM +0200, Michal Suchanek wrote:
> Hello
> 
> On 22 June 2012 03:55, Peter Hutterer <peter.hutterer at who-t.net> wrote:
> > On Fri, Jun 08, 2012 at 04:19:17PM +0200, Michal Suchanek wrote:
> >> Hello,
> >>
> >> sending the patch witch should fix issue with unloading sibling
> >> modules along with a couple of patches that allow actually unloading
> >> modules.
> >
> > in the future, please send patches out separately. it's a bit of a pain to
> > review and comment otherwise, especially as  I have to edit the mimetype for
> > from application/octet-stream to text/plain all of them.
> 
> The joys of Mozilla client software.
> 
> I found there is an unix server that is allowed to send mail which
> should address this issue.
> 
> > urgh, no. why does the caller need to care about the module count at all? the
> > input code should just call unload and let the loader sort it out.
> 
> urgh, yes.
> 
> The input code needs to know when to delete the driver from its list
> and the loader does not return anything.
> 
> I could possibly make it return something different when this check
> fails but I am not sure it has any nice semantics. The close on the
> actual object handle can fail also and then the module is already
> uninitialized, and unloading child submodules can fail, too.

the bits your changing already change the ABI, so you've got (more or less)
free range with the API as well. shape it into something that makes sense.
would be a novel thing for this bit of code, anyways ;)
 
> >>      if (xf86InputDriverList[drvIndex] && xf86InputDriverList[drvIndex]->module)
> >>          UnloadModule(xf86InputDriverList[drvIndex]->module);
> >>      free(xf86InputDriverList[drvIndex]);
> >>      xf86InputDriverList[drvIndex] = NULL;
> >> +    if (drvIndex + 1 < xf86NumInputDrivers)
> >> +        memmove(xf86InputDriverList[drvIndex], xf86InputDriverList[drvIndex+1],
> >> +                sizeof(xf86InputDriverList[drvIndex]) * (xf86NumInputDrivers - drvIndex - 1));
> >> +    xf86NumInputDrivers--;
> >> +    xf86InputDriverList[xf86NumInputDrivers] = NULL;
> >
> > this sounds like a prime target for a xorg_list switch to avoid this code.
> >
> 
> The loader sibling list even more so.
> 
> Still the input device list is substantially simpler structure than
> xorg_list with the only downside that deletion is somewhat hairy.
> Perhaps another one is that xorg_list has tests but the input device
> list does not.

that's my main argument. we've found quite a few bugs with various
open-coded lists (or manual list resizing) that just using the macros is
much safer. the nt_list_* macros for already existing, null-terminated lists
and the xorg_list_* macros for anything new.
 
> I will be sending updated version of the patches which should
> hopefully address the concerns stripped off this email ;-)

thanks. sorry it took me so long to get to them.

Cheers,
  Peter