[PATCH 0/37] Extension handling cleanup, death to extmod

[Daniel Stone]

Hi Jamey,

On Thu, Jun 30, 2011 at 04:06:02PM -0700, Jamey Sharp wrote:
> I sure like the results of this series. I've reviewed the patches as I
> found them on your personal branch. The abbreviated commit hashes that I
> reviewed are listed together with their commit summaries below.

Thanks a heap for the review! Here's a slightly belated update.
on its way to the list shortly.

So TTBOMK I've addressed everything you've mentioned, but:

> 9601c02 Move DGA from extmod to built-in
> da1bfbf Move XFree86-VidMode from extmod to built-in
> 0b40ab8 Remove the last remnants of extmod
> - Some re-ordering needed; details in the thread for "Move
>   XFree86-VidMode from extmod".
> 
> - Anyway, I'd be tempted to leave these two extensions in extmod. All
>   the other de-modularizings make sense because they're trivial module
>   wrappers around code that was linked into the server core anyway.
>   These two aren't.

Ultimately though, there was still so much code in the server core to
support these that I think it's not quite as clear-cut as that.  I think
if people want to enable or disable it, they should be doing it at build
time.

> 5396316 GLX: Remove extension init dependencies
> - This doesn't look right, off-hand. My reading of the code is that
>   extensions will be initialized in the order that LoadExtension was
>   called, and that static extensions will have LoadExtension called from
>   InitExtensions *after* all modules are loaded. So GLX would init
>   before its dependencies, right?

As it turns out, yes. :) I did notice and fix this when Evolution failed
to start because it couldn't find a GLX visual/fbconfig for the
Composite visual.

> e2ea4a6 Remove separate ExtensionToggle list from miinitext [WIP]
> - Duh, it's WIP. :-) But I think it's easy to fix: Either do your
>   lookups in both staticExtensions and ExtensionModuleList, or arrange
>   to LoadExtension for all staticExtensions before calling anything else
>   in miinitext.c, and then use only ExtensionModuleList.

The problem though is that ExtensionModuleList might get added to later,
e.g. drivers calling LoadModule 'glx' at an arbitrary stage in startup,
long after argument parsing.  We could just accept arbitrary extension
names in argument parsing and try to match them later, but yeah.

Thanks again for the review!

Cheers,
Daniel