On Sun, Nov 20, 2011 at 3:41 AM, Stefan Fritsch <[email protected]> wrote: > On Fri, 18 Nov 2011, [email protected] wrote: > >> Author: trawick >> Date: Fri Nov 18 13:10:06 2011 >> New Revision: 1203634 >> >> URL: http://svn.apache.org/viewvc?rev=1203634&view=rev >> Log: >> add conn_rec to error log hook > >> URL: >> http://svn.apache.org/viewvc/httpd/httpd/trunk/server/log.c?rev=1203634&r1=1203633&r2=1203634&view=diff >> >> ============================================================================== >> --- httpd/httpd/trunk/server/log.c (original) >> +++ httpd/httpd/trunk/server/log.c Fri Nov 18 13:10:06 2011 >> @@ -1264,8 +1264,8 @@ static void log_error_core(const char *f >> if (!log_format) { > > Not your change, but since you seem to use the error_log hook: Is this the > correct behavior? It seems the hook is only called if there is no custom log > format set. I think it should actually be "if (done)", i.e. call it only for > the real message, not for the per-req/per-conn messages.
wow, I didn't notice any of that! (I actually have the no-conn_rec problem with code that runs only with 2.2 at present) call it only for the real message; if the module needs the per-request or per-connection information for some reason, it is already available from c or r IIUC > > >> /* only pass the real error string to the hook */ >> errstr[errstr_end] = '\0'; >> - ap_run_error_log(file, line, module_index, level, status, s, >> r, pool, >> - errstr + errstr_start); >> + ap_run_error_log(file, line, module_index, level, status, s, >> c, r, >> + pool, errstr + errstr_start); >> } > > Would it make sense to pass the ap_errorlog_info struct instead? It has > contains most of the args and is extensible with only a minor MMN bump. (shrug) it might be less aggravating to existing callers to simply add the conn_rec parm prior to 2.4.x GA vs. changing the entire interface (not many existing callers, dunno if we are entertaining API changes prior to GA anyway) > > >> *errstr = '\0'; >> @@ -1761,9 +1761,9 @@ AP_DECLARE(const char *) ap_parse_log_le >> AP_IMPLEMENT_HOOK_VOID(error_log, >> (const char *file, int line, int module_index, int >> level, >> apr_status_t status, const server_rec *s, >> - const request_rec *r, apr_pool_t *pool, >> - const char *errstr), (file, line, module_index, >> level, >> - status, s, r, pool, errstr)) >> + const conn_rec *c, const request_rec *r, >> + apr_pool_t *pool, const char *errstr), (file, >> line, >> + module_index, level, status, s, c, r, pool, >> errstr)) >> >> AP_IMPLEMENT_HOOK_RUN_FIRST(int, generate_log_id, >> (const conn_rec *c, const request_rec *r, >> >> >> > -- Born in Roswell... married an alien...
