You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Ruediger Pluem <rp...@apache.org> on 2023/03/14 15:05:56 UTC

Re: svn commit: r1908388 - in /httpd/httpd/trunk/server: core.c log.c


On 3/14/23 3:37 PM, ylavic@apache.org wrote:
> Author: ylavic
> Date: Tue Mar 14 14:37:00 2023
> New Revision: 1908388
> 
> URL: http://svn.apache.org/viewvc?rev=1908388&view=rev
> Log:
> core: Use the main ErrorLogFormat for ap_log_perror() and while loading vhosts.
> 
> * server/core.c(create_core_server_config):
>   Init sconf->error_log_format early so that it applies while the vhost
>   is loading.
> 
> * server/log.c(log_error_core):
>   Get the core_server_config from the main server if no server/config is
>   provided.
> 
> 
> Modified:
>     httpd/httpd/trunk/server/core.c
>     httpd/httpd/trunk/server/log.c
> 
> Modified: httpd/httpd/trunk/server/core.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/core.c?rev=1908388&r1=1908387&r2=1908388&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/server/core.c (original)
> +++ httpd/httpd/trunk/server/core.c Tue Mar 14 14:37:00 2023
> @@ -493,6 +493,11 @@ static void *create_core_server_config(a
>          conf->flush_max_pipelined = AP_FLUSH_MAX_PIPELINED;
>      }
>      else {
> +        /* Use main ErrorLogFormat while the vhost is loading */
> +        core_server_config *main_conf =
> +            ap_get_core_module_config(ap_server_conf->module_config);
> +        conf->error_log_format = main_conf->error_log_format;
> +
>          conf->flush_max_pipelined = -1;
>      }
>  
> 
> Modified: httpd/httpd/trunk/server/log.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/log.c?rev=1908388&r1=1908387&r2=1908388&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/server/log.c (original)
> +++ httpd/httpd/trunk/server/log.c Tue Mar 14 14:37:00 2023
> @@ -1098,6 +1098,9 @@ static void log_error_core(const char *f
>              errorlog_provider = ap_server_conf->errorlog_provider;
>              errorlog_provider_handle = ap_server_conf->errorlog_provider_handle;
>          }
> +
> +        /* Use the main ErrorLogFormat if any */
> +        sconf = ap_get_core_module_config(ap_server_conf->module_config);

The code above checks for ap_server_conf != NULL. Should we apply this here as well or can we be sure that ap_server_conf is
always non NULL?

>      }
>      else {
>          int configured_level = r ? ap_get_request_module_loglevel(r, module_index)        :
> @@ -1145,6 +1148,10 @@ static void log_error_core(const char *f
>                  }
>              }
>          }
> +        else {
> +            /* Use the main ErrorLogFormat if any */
> +            sconf = ap_get_core_module_config(ap_server_conf->module_config);
> +        }
>      }
>  
>      if (!logf && !(errorlog_provider && errorlog_provider_handle)) {
> @@ -1215,7 +1222,7 @@ static void log_error_core(const char *f
>              info.file         = file;
>              info.line         = line;
>              info.status       = status;
> -            log_format = sconf ? sconf->error_log_format : NULL;
> +            log_format = sconf->error_log_format;

If we would follow my comment above and check for ap_server_conf != NULL. this check would still be needed.

Regards

Rüdiger

Re: svn commit: r1908388 - in /httpd/httpd/trunk/server: core.c log.c

Posted by Yann Ylavic <yl...@gmail.com>.
On Tue, Mar 14, 2023 at 4:06 PM Ruediger Pluem <rp...@apache.org> wrote:
>
> On 3/14/23 3:37 PM, ylavic@apache.org wrote:
> > Author: ylavic
> > Date: Tue Mar 14 14:37:00 2023
> > New Revision: 1908388
> >
> > URL: http://svn.apache.org/viewvc?rev=1908388&view=rev
> > Log:
> > core: Use the main ErrorLogFormat for ap_log_perror() and while loading vhosts.
> >
> > * server/core.c(create_core_server_config):
> >   Init sconf->error_log_format early so that it applies while the vhost
> >   is loading.
> >
> > * server/log.c(log_error_core):
> >   Get the core_server_config from the main server if no server/config is
> >   provided.
[]
> >
> > +++ httpd/httpd/trunk/server/log.c Tue Mar 14 14:37:00 2023
> > @@ -1098,6 +1098,9 @@ static void log_error_core(const char *f
> >              errorlog_provider = ap_server_conf->errorlog_provider;
> >              errorlog_provider_handle = ap_server_conf->errorlog_provider_handle;
> >          }
> > +
> > +        /* Use the main ErrorLogFormat if any */
> > +        sconf = ap_get_core_module_config(ap_server_conf->module_config);
>
> The code above checks for ap_server_conf != NULL. Should we apply this here as well or can we be sure that ap_server_conf is
> always non NULL?

Yes, can still be NULL here, thanks! Fixed in r1908390.
Also in r1908393 I tried to set ap_server_conf ASAP, for anything
logged during ap_read_config()..


>
> >      }
> >      else {
> >          int configured_level = r ? ap_get_request_module_loglevel(r, module_index)        :
> > @@ -1145,6 +1148,10 @@ static void log_error_core(const char *f
> >                  }
> >              }
> >          }
> > +        else {
> > +            /* Use the main ErrorLogFormat if any */
> > +            sconf = ap_get_core_module_config(ap_server_conf->module_config);
> > +        }
> >      }
> >
> >      if (!logf && !(errorlog_provider && errorlog_provider_handle)) {
> > @@ -1215,7 +1222,7 @@ static void log_error_core(const char *f
> >              info.file         = file;
> >              info.line         = line;
> >              info.status       = status;
> > -            log_format = sconf ? sconf->error_log_format : NULL;
> > +            log_format = sconf->error_log_format;
>
> If we would follow my comment above and check for ap_server_conf != NULL. this check would still be needed.

Restored in r1908390 too.


Regards;
Yann.