>> +/* we need to hook a few internal things before APR_HOOK_REALLY_FIRST */
>> +#define MODPERL_HOOK_REALLY_FIRST (-20)
> 
> 
> The name is confusing, since APR_REALLY_FIRST is -10 and you kept the
> rest of the MODPERL_HOOK_ names matching . Call it REALLY_REALLY_FIRST?
> or anything else which is not _REALLY_FIRST?

I was leaning toward MODPERL_HOOK_REALLY_REALLY_FIRST at first, so ok.

> please s/order/hook_order/ or similar, order on its own is too vague of
> a name.

well, the rest are vague too, but ok :)

> I think it's a waste. If it's used only by the main server and relevant
> only at the startup, just use some static variable and don't blow the
> size of scfg for vhosts.

the whole idea of static C variables/structs and threaded mpms really,
really confuses me.  but I guess it doesn't matter, since it's not a
per-request thing and all this happens before ap_mpm_run is called anyway.

>> +
>> +/* from here down is support for dynamic hook ordering.  this is mostly
>> + * stolen from mod_info.c, so see also the logic and descriptions there.
>> + */
> 
> 
> That's a bad idea to point to description in another module. What
> happens if it gets nuked/rewritten and it won't match the code below any
> longer. Just copy it over while it's correct.

ok, I guess so.

> 
> The whole sorting code doesn't belong to modperl_util.c, IMHO. It's a
> one-time use code and should be static in where it is used, i.e. mod_perl.c
> 

I started out doing exactly that.  however, I didn't feel as though clogging
up mod_perl.c with this little feature was worth it - I'd be adding, what,
200 lines of obscure-feature code to the main module?  so, I moved it over
to modperl_util.c (after also debating modperl_config.c).

but if you think it's better in mod_perl.c that's fine with me.  I was just
trying to keep that as clutter-free as possible.

> And finally, if there are already two modules who need to copy so much
> cruft to resort hooks, perhaps it should become an Apache API, so that
> you don't have to worry about win32 users and what not. This stuff is
> really not urgent to put in, IMHO. So if it gets added in 2.0.49 that's
> still cool with us, no?

yeah, I tried to see how to abstract some of the stuff out, but really it's
only the hook_lookup stuff that is duplicated.  everyone else would have
their own needs.  little bits of logic here and there are the same, but
mod_perl and mod_info both have their exceptional cases.

--Geoff


---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to