httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Plüm, Rüdiger, VF-Group" <ruediger.pl...@vodafone.com>
Subject RE: svn commit: r952201 - /httpd/httpd/trunk/server/log.c
Date Tue, 08 Jun 2010 10:11:20 GMT
 

> -----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, rjung@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


Mime
View raw message