+1 On Mar 11, 2011, at 4:47 PM, Jeff Trawick wrote:
> On Fri, Mar 11, 2011 at 4:18 PM, Stefan Fritsch <[email protected]> wrote: >> On Friday 11 March 2011, Jeff Trawick wrote: >>> I was hoping that someone who knows something would respond, but >>> what the hey... >>> >>> On Fri, Mar 11, 2011 at 9:27 AM, Eric Covener <[email protected]> >> wrote: >>>> server/log.c discards anything higher than WARNING if the >>>> server_rec is NULL. >> >> FWIW, the WARNING can be changed to something else with the -e command >> line option. But -e does not allow to set per-module levels. >> >>>> But, when adding new traceN messages deep down in modules, >>>> request/conn/server recs might not be handy. >>>> >>>> Is it safe to use extern server_rec* ap_server_conf in random >>>> modules for trace messages? >>> >>> yes >> >> Not always. Between destruction of the previous pconf and finishing >> config parsing, ap_server_conf will usually point to invalid data. > > sure (my mind is stuck on those evil calls to ap_log_error at steady > state with NULL server_rec) > >> But >> this could be changed by registering a pool cleanup on pconf that sets >> ap_server_conf to NULL. > > good idea... > >> >>>> Or should server/log.c know the difference between "still in >>>> startup" and "stupid module couldn't find a server-rec to pass >>>> but really wants info in main server errorlog via stderr? >>>> >>>> (or, should server/log.c be willing to log anything < WARNING and >>>>> TRACE1 to stderr via s==NULL? as long as it matches the >>>> LogLevel >>> >>> Something I wondered when fixing some server-rec=NULL issues >>> recently was whether we should have an especially ugly warning >>> logged in maintainer mode when APLOG_STARTUP isn't included in the >>> level flags while server-rec is NULL. >>> >>> either-it-is-startup-or-you-pass-server_rec works for me. >> >> If we ensured that ap_server_conf is either valid or NULL, then >> ap_log_error() could do that and modules wouldn't have to care about >> it. Unless I have missed something, this looks like the better >> solution to me. If it leads to wrong behaviour, ap_log_error could >> also use ap_state_query() to check what to do. > > I guess if it is safe to blindly add ap_server_rec then there's no > need to try to flag bad calls at steady state > >> >> APLOG_STARTUP only determins if the timestamp/severity-level/module >> prefix is printed. I would be ok with making that a no-op and letting >> ap_log_error() decide by itself when httpd is in the startup phase >
