Mailing-List: contact dev-help@httpd.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@httpd.apache.org
Received-SPF: pass (nike.apache.org: local policy)
Content-class: urn:content-classes:message
MIME-Version: 1.0
Content-Type: text/plain;
	charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
Subject: RE: svn commit: r952201 - /httpd/httpd/trunk/server/log.c
Date: Tue, 8 Jun 2010 12:11:20 +0200
Message-ID: 
 <99EA83DCDE961346AFA9B5EC33FEC08B0407E432@VF-MBX11.internal.vodafone.com>
In-Reply-To: <4C0E09D1.9010705@kippdata.de>
Thread-Topic: svn commit: r952201 - /httpd/httpd/trunk/server/log.c
Thread-Index: AcsG6wQQkDW1sli8QhOWwO1FPm6egQAB8UHg
References: <20100607122326.630E023889B3@eris.apache.org>
 <20100608082102.GA2795@redhat.com> <4C0E09D1.9010705@kippdata.de>
From: =?iso-8859-1?Q?=22Pl=FCm=2C_R=FCdiger=2C_VF-Group=22?=
 <ruediger.pluem@vodafone.com>
To: <dev@httpd.apache.org>

=20

> -----Original Message-----
> From: Rainer Jung=20
> Sent: Dienstag, 8. Juni 2010 11:14
> To: dev@httpd.apache.org
> Subject: Re: svn commit: r952201 - /httpd/httpd/trunk/server/log.c
>=20
> 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=3D952201&view=3Drev
> >> 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) !=3D APLOG_STARTUP) {
> >>           len +=3D apr_snprintf(errstr + len, MAX_STRING_LEN - len,
> >>                               "[%s] ",=20
> priorities[level_and_mask].t_name);
> >> +
> >> +        len +=3D apr_snprintf(errstr + len, MAX_STRING_LEN - len,
> >> +                            "[%" APR_PID_T_FMT, getpid());
> >> +#if APR_HAS_THREADS
> >> +        {
> >> +            apr_os_thread_t tid =3D apr_os_thread_current();
> >> +            len +=3D apr_snprintf(errstr + len,=20
> MAX_STRING_LEN - len,
> >> +                                ":%pT",&tid);
> >> +        }
> >> +#endif
> >> +        errstr[len++] =3D ']';
> >> +        errstr[len++] =3D ' ';
> >
> > 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? =20
> (Hopefully this
> > function won't start showing up in CPU profiles!)
>=20
> I'm neutral on adding the descriptive prefixes (+0). It's=20
> helpful to get=20
> acquainted with the new format, on the long term it might become=20
> unnecessary. I guess when adding new standard tokens to the=20
> error log we=20
> might end up doing a callback based configurable format like in=20
> mod_log_config.
>=20
> Removing the tid for non-threaded servers is fine. I thought=20
> about that=20
> too. Would it make sense to somehow cache the result of=20
> ap_mpm_query(AP_MPMQ_IS_THREADED,&result) in one of the structs=20
> available from the log function, because it won't change during the=20
> runtime of the instance (at least not during the runtime of=20
> the child;=20
> don't know whether it would be feasible to switch the new dynamically=20
> loaded MPMs during restart, but that's another topic).
>=20

How about a local static variable in the function?

Regards

R=FCdiger