Re: svn commit: r1209766 [9/12] - in /httpd/httpd/trunk: docs/log-message-tags/ modules/aaa/ modules/apreq/ modules/arch/netware/ modules/arch/unix/ modules/arch/win32/ modules/cache/ modules/cluster/ modules/core/ modules/database/ modules/dav/fs/ modules...

[Kaspar Brand <ht...@velox.ch>]

On 03.12.2011 00:02, sf@apache.org wrote:
> Modified: httpd/httpd/trunk/modules/ssl/ssl_engine_log.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_engine_log.c?rev=1209766&r1=1209765&r2=1209766&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/ssl/ssl_engine_log.c (original)
> +++ httpd/httpd/trunk/modules/ssl/ssl_engine_log.c Fri Dec  2 23:02:04 2011
> @@ -94,7 +94,7 @@ void ssl_log_ssl_error(const char *file,
>          annotation = ssl_log_annotation(err);
>  
>          ap_log_error(file, line, APLOG_MODULE_INDEX, level, 0, s,
> -                     "SSL Library Error: %s%s%s%s%s%s",
> +                     APLOGNO(02021) "SSL Library Error: %s%s%s%s%s%s",
>                       /* %s */
>                       err,
>                       /* %s%s%s */
> @@ -135,7 +135,7 @@ static void ssl_log_cert_error(const cha
>               */
>              int maxdnlen = (HUGE_STRING_LEN - msglen - 300) / 2;
>  
> -            BIO_puts(bio, " [subject: ");
> +            BIO_puts(bio, APLOGNO(02022) " [subject: ");
>              name = SSL_X509_NAME_to_string(p, X509_get_subject_name(cert),
>                                             maxdnlen);
>              if (!strIsEmpty(name)) {
> @@ -174,7 +174,7 @@ static void ssl_log_cert_error(const cha
>      }
>      else {
>          apr_snprintf(buf + msglen, sizeof buf - msglen,
> -                     " [certificate: -not available-]");
> +                     APLOGNO(02023) " [certificate: -not available-]");
>      }
>  
>      if (r) {
> 

These changes aren't doing the right thing, I think... both
ssl_log_ssl_error() and ssl_log_cert_error() are basically wrappers for
ap_log_*(), and are therefore called from various places in mod_ssl -
i.e. the messages triggering them should get different tags. (Also, in
the case of ssl_log_cert_error, the error tag is currently inserted
somewhere in the middle of the message, because ssl_log_cert_error
appends to a message already stored in buf.)

ssl_log_ssl_error is (virtually?) always called in combination with
ap_log_*, so we might do without a tag for these messages - or
otherwise, modify ssl_log_ssl_error to allow a tag to be passed in, and
then use the same tag in both log calls (?). Finally, as far as
ssl_log_xerror/ssl_log_cxerror/ssl_log_rxerror are concerned, would it
be possible to extend the Coccinelle patch file so that these are also
recognized?

Kaspar

Re: svn commit: r1209766 [9/12] - in /httpd/httpd/trunk: docs/log-message-tags/ modules/aaa/ modules/apreq/ modules/arch/netware/ modules/arch/unix/ modules/arch/win32/ modules/cache/ modules/cluster/ modules/core/ modules/database/ modules/dav/fs/ modules...

[Stefan Fritsch <sf...@sfritsch.de>]

On Wednesday 07 December 2011, Kaspar Brand wrote:
> These changes aren't doing the right thing, I think... both
> ssl_log_ssl_error() and ssl_log_cert_error() are basically wrappers
> for ap_log_*(), and are therefore called from various places in
> mod_ssl - i.e. the messages triggering them should get different
> tags. (Also, in the case of ssl_log_cert_error, the error tag is
> currently inserted somewhere in the middle of the message, because
> ssl_log_cert_error appends to a message already stored in buf.)
> 
> ssl_log_ssl_error is (virtually?) always called in combination with
> ap_log_*, so we might do without a tag for these messages - or
> otherwise, modify ssl_log_ssl_error to allow a tag to be passed in,
> and then use the same tag in both log calls (?). Finally, as far
> as ssl_log_xerror/ssl_log_cxerror/ssl_log_rxerror are concerned,
> would it be possible to extend the Coccinelle patch file so that
> these are also recognized?

Good point. Fixed/added in r1211680. I have not modified 
ssl_log_ssl_error() besides removing the tag. It's really only called 
together with some other log message, so the tag doesn't add any 
value.