> -----Original Message----- > From: Rainer Jung > Sent: Dienstag, 8. Juni 2010 11:14 > To: dev@httpd.apache.org > Subject: Re: svn commit: r952201 - /httpd/httpd/trunk/server/log.c > > On 08.06.2010 10:21, Joe Orton wrote: > > On Mon, Jun 07, 2010 at 12:23:26PM -0000, rj...@apache.org wrote: > >> Author: rjung > >> Date: Mon Jun 7 12:23:26 2010 > >> New Revision: 952201 > >> > >> URL: http://svn.apache.org/viewvc?rev=952201&view=rev > >> Log: > >> Add process id and thread id (if APR has thread support) > >> to the error log. > > ... > >> @@ -620,6 +621,18 @@ static void log_error_core(const char *f > >> if ((level& APLOG_STARTUP) != APLOG_STARTUP) { > >> len += apr_snprintf(errstr + len, MAX_STRING_LEN - len, > >> "[%s] ", > priorities[level_and_mask].t_name); > >> + > >> + len += apr_snprintf(errstr + len, MAX_STRING_LEN - len, > >> + "[%" APR_PID_T_FMT, getpid()); > >> +#if APR_HAS_THREADS > >> + { > >> + apr_os_thread_t tid = apr_os_thread_current(); > >> + len += apr_snprintf(errstr + len, > MAX_STRING_LEN - len, > >> + ":%pT",&tid); > >> + } > >> +#endif > >> + errstr[len++] = ']'; > >> + errstr[len++] = ' '; > > > > These numbers made me double-take when reading error_log since they > > don't have a descriptive prefix. Also the tid is not much use in a > > non-threaded server. Could we do something like this? > (Hopefully this > > function won't start showing up in CPU profiles!) > > I'm neutral on adding the descriptive prefixes (+0). It's > helpful to get > acquainted with the new format, on the long term it might become > unnecessary. I guess when adding new standard tokens to the > error log we > might end up doing a callback based configurable format like in > mod_log_config. > > Removing the tid for non-threaded servers is fine. I thought > about that > too. Would it make sense to somehow cache the result of > ap_mpm_query(AP_MPMQ_IS_THREADED,&result) in one of the structs > available from the log function, because it won't change during the > runtime of the instance (at least not during the runtime of > the child; > don't know whether it would be feasible to switch the new dynamically > loaded MPMs during restart, but that's another topic). >
How about a local static variable in the function? Regards Rüdiger