Mailing List Archive

Re: svn commit: r1908380 - in /httpd/httpd/trunk: changes-entries/errorlogformat.txt include/util_time.h server/log.c server/util_time.c
On 3/14/23 12:11 PM, ylavic@apache.org wrote:
> Author: ylavic
> Date: Tue Mar 14 11:11:24 2023
> New Revision: 1908380
>
> URL: http://svn.apache.org/viewvc?rev=1908380&view=rev
> Log:
> core: Add formats %{z} and %{strftime-format} to ErrorLogFormat. PR 62161.
>
> %{z} prints the timezone offset (i.e. "[+-]nnnn") and %{strftime-format} allows
> any %-format handled by [apr_]strftime().
>
> * include/util_time.h():
> Define new AP_CTIME_OPTION_GMTOFF option for ap_recent_ctime_ex().
>
> * server/util_time.c(ap_recent_ctime_ex):
> Handle AP_CTIME_OPTION_GMTOFF to print "[+-]nnnn" timezone.
>
> * server/log.c(log_ctime):
> If the format contains a '%' it's for strftime(), otherwise it's builtin
> with new 'z' as AP_CTIME_OPTION_GMTOFF.
>
>
> Added:
> httpd/httpd/trunk/changes-entries/errorlogformat.txt
> Modified:
> httpd/httpd/trunk/include/util_time.h
> httpd/httpd/trunk/server/log.c
> httpd/httpd/trunk/server/util_time.c
>
> Added: httpd/httpd/trunk/changes-entries/errorlogformat.txt
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/changes-entries/errorlogformat.txt?rev=1908380&view=auto
> ==============================================================================
> --- httpd/httpd/trunk/changes-entries/errorlogformat.txt (added)
> +++ httpd/httpd/trunk/changes-entries/errorlogformat.txt Tue Mar 14 11:11:24 2023
> @@ -0,0 +1,3 @@
> + *) core: Add formats %{z} and %{strftime-format} to ErrorLogFormat.
> + PR 62161. [Yann Ylavic]
> +
>
> Modified: httpd/httpd/trunk/include/util_time.h
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/include/util_time.h?rev=1908380&r1=1908379&r2=1908380&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/include/util_time.h (original)
> +++ httpd/httpd/trunk/include/util_time.h Tue Mar 14 11:11:24 2023
> @@ -47,6 +47,8 @@ extern "C" {
> #define AP_CTIME_OPTION_USEC 0x1
> /* Use more compact ISO 8601 format */
> #define AP_CTIME_OPTION_COMPACT 0x2
> +/* Add timezone offset from GMT ([+-]hhmm) */
> +#define AP_CTIME_OPTION_GMTOFF 0x4
>
>
> /**
>
> Modified: httpd/httpd/trunk/server/log.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/log.c?rev=1908380&r1=1908379&r2=1908380&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/server/log.c (original)
> +++ httpd/httpd/trunk/server/log.c Tue Mar 14 11:11:24 2023
> @@ -573,14 +573,32 @@ static int log_ctime(const ap_errorlog_i
> int time_len = buflen;
> int option = AP_CTIME_OPTION_NONE;
>
> - while (arg && *arg) {
> - switch (*arg) {
> - case 'u': option |= AP_CTIME_OPTION_USEC;
> - break;
> - case 'c': option |= AP_CTIME_OPTION_COMPACT;
> - break;
> + if (arg) {
> + if (arg[0] == 'u' && !arg[1]) { /* no ErrorLogFormat (fast path) */
> + option |= AP_CTIME_OPTION_USEC;
> + }
> + else if (!ap_strchr_c(arg, '%')) { /* special "%{cuz}t" formats */
> + while (*arg) {
> + switch (*arg++) {
> + case 'u':
> + option |= AP_CTIME_OPTION_USEC;
> + break;
> + case 'c':
> + option |= AP_CTIME_OPTION_COMPACT;
> + break;
> + case 'z':
> + option |= AP_CTIME_OPTION_GMTOFF;
> + break;
> + }
> + }
> + }
> + else { /* "%{strftime %-format}t" */
> + apr_size_t len = 0;
> + apr_time_exp_t expt;
> + ap_explode_recent_localtime(&expt, apr_time_now());
> + apr_strftime(buf, &len, buflen, arg, &expt);
> + return (int)len;
> }
> - arg++;
> }
>
> ap_recent_ctime_ex(buf, apr_time_now(), option, &time_len);
>
> Modified: httpd/httpd/trunk/server/util_time.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/util_time.c?rev=1908380&r1=1908379&r2=1908380&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/server/util_time.c (original)
> +++ httpd/httpd/trunk/server/util_time.c Tue Mar 14 11:11:24 2023
> @@ -16,6 +16,7 @@
>
> #include "util_time.h"
> #include "apr_env.h"
> +#include "apr_strings.h"
>
>
>
> @@ -27,6 +28,8 @@
> /* Length of ISO 8601 date/time */
> #define AP_CTIME_COMPACT_LEN 20
>
> +/* Length of timezone offset from GMT ([+-]hhmm) plus leading space */
> +#define AP_CTIME_GMTOFF_LEN 6
>
> /* Cache for exploded values of recent timestamps
> */
> @@ -183,6 +186,10 @@ AP_DECLARE(apr_status_t) ap_recent_ctime
> needed += AP_CTIME_USEC_LENGTH;
> }
>
> + if (option & AP_CTIME_OPTION_GMTOFF) {
> + needed += AP_CTIME_GMTOFF_LEN;
> + }
> +
> /* Check the provided buffer length */
> if (len && *len >= needed) {
> *len = needed;
> @@ -195,9 +202,10 @@ AP_DECLARE(apr_status_t) ap_recent_ctime
> }
>
> /* example without options: "Wed Jun 30 21:49:08 1993" */
> - /* 123456789012345678901234 */
> /* example for compact format: "1993-06-30 21:49:08" */
> - /* 1234567890123456789 */
> + /* example for compact+usec+gmtoff format:
> + * "1993-06-30 22:49:08.123456 +0100"
> + */
>
> ap_explode_recent_localtime(&xt, t);
> real_year = 1900 + xt.tm_year;
> @@ -251,7 +259,19 @@ AP_DECLARE(apr_status_t) ap_recent_ctime
> *date_str++ = real_year % 100 / 10 + '0';
> *date_str++ = real_year % 10 + '0';
> }
> - *date_str++ = 0;
> + if (option & AP_CTIME_OPTION_GMTOFF) {
> + int off = xt.tm_gmtoff;
> + char sign = '+';
> + if (off < 0) {
> + off = -off;
> + sign = '-';
> + }
> + apr_snprintf(date_str, AP_CTIME_GMTOFF_LEN + 1, " %c%.2d%.2d",
> + sign, off / 3600, (off % 3600) / 60);

The code above does not use apr_snprintf but converts the numbers to strings explicitly (I guess for performance reasons).
Should we use the same approach here?

Regards

Rüdiger
Re: svn commit: r1908380 - in /httpd/httpd/trunk: changes-entries/errorlogformat.txt include/util_time.h server/log.c server/util_time.c [ In reply to ]
On Tue, Mar 14, 2023 at 1:30?PM Ruediger Pluem <rpluem@apache.org> wrote:
>
> On 3/14/23 12:11 PM, ylavic@apache.org wrote:
> > Author: ylavic
> > Date: Tue Mar 14 11:11:24 2023
> > New Revision: 1908380
> >
> > URL: http://svn.apache.org/viewvc?rev=1908380&view=rev
> > Log:
> > core: Add formats %{z} and %{strftime-format} to ErrorLogFormat. PR 62161.
> >
> > %{z} prints the timezone offset (i.e. "[+-]nnnn") and %{strftime-format} allows
> > any %-format handled by [apr_]strftime().
> >
> > * include/util_time.h():
> > Define new AP_CTIME_OPTION_GMTOFF option for ap_recent_ctime_ex().
> >
> > * server/util_time.c(ap_recent_ctime_ex):
> > Handle AP_CTIME_OPTION_GMTOFF to print "[+-]nnnn" timezone.
> >
> > * server/log.c(log_ctime):
> > If the format contains a '%' it's for strftime(), otherwise it's builtin
> > with new 'z' as AP_CTIME_OPTION_GMTOFF.
[]
> >
> > +++ httpd/httpd/trunk/server/util_time.c Tue Mar 14 11:11:24 2023
[]
> > @@ -251,7 +259,19 @@ AP_DECLARE(apr_status_t) ap_recent_ctime
> > *date_str++ = real_year % 100 / 10 + '0';
> > *date_str++ = real_year % 10 + '0';
> > }
> > - *date_str++ = 0;
> > + if (option & AP_CTIME_OPTION_GMTOFF) {
> > + int off = xt.tm_gmtoff;
> > + char sign = '+';
> > + if (off < 0) {
> > + off = -off;
> > + sign = '-';
> > + }
> > + apr_snprintf(date_str, AP_CTIME_GMTOFF_LEN + 1, " %c%.2d%.2d",
> > + sign, off / 3600, (off % 3600) / 60);
>
> The code above does not use apr_snprintf but converts the numbers to strings explicitly (I guess for performance reasons).
> Should we use the same approach here?

Thanks, done in r1908389.


Regards;
Yann.
Re: svn commit: r1908380 - in /httpd/httpd/trunk: changes-entries/errorlogformat.txt include/util_time.h server/log.c server/util_time.c [ In reply to ]
On 3/14/23 4:56 PM, Yann Ylavic wrote:
> On Tue, Mar 14, 2023 at 1:30?PM Ruediger Pluem <rpluem@apache.org> wrote:
>>
>> On 3/14/23 12:11 PM, ylavic@apache.org wrote:
>>> Author: ylavic
>>> Date: Tue Mar 14 11:11:24 2023
>>> New Revision: 1908380
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1908380&view=rev
>>> Log:
>>> core: Add formats %{z} and %{strftime-format} to ErrorLogFormat. PR 62161.
>>>
>>> %{z} prints the timezone offset (i.e. "[+-]nnnn") and %{strftime-format} allows
>>> any %-format handled by [apr_]strftime().
>>>
>>> * include/util_time.h():
>>> Define new AP_CTIME_OPTION_GMTOFF option for ap_recent_ctime_ex().
>>>
>>> * server/util_time.c(ap_recent_ctime_ex):
>>> Handle AP_CTIME_OPTION_GMTOFF to print "[+-]nnnn" timezone.
>>>
>>> * server/log.c(log_ctime):
>>> If the format contains a '%' it's for strftime(), otherwise it's builtin
>>> with new 'z' as AP_CTIME_OPTION_GMTOFF.
> []
>>>
>>> +++ httpd/httpd/trunk/server/util_time.c Tue Mar 14 11:11:24 2023
> []
>>> @@ -251,7 +259,19 @@ AP_DECLARE(apr_status_t) ap_recent_ctime
>>> *date_str++ = real_year % 100 / 10 + '0';
>>> *date_str++ = real_year % 10 + '0';
>>> }
>>> - *date_str++ = 0;
>>> + if (option & AP_CTIME_OPTION_GMTOFF) {
>>> + int off = xt.tm_gmtoff;
>>> + char sign = '+';
>>> + if (off < 0) {
>>> + off = -off;
>>> + sign = '-';
>>> + }
>>> + apr_snprintf(date_str, AP_CTIME_GMTOFF_LEN + 1, " %c%.2d%.2d",
>>> + sign, off / 3600, (off % 3600) / 60);
>>
>> The code above does not use apr_snprintf but converts the numbers to strings explicitly (I guess for performance reasons).
>> Should we use the same approach here?
>
> Thanks, done in r1908389.

I guess the

#include "apr_strings.h"

added in r1908380 is no longer needed as well?

Regards

Rüdiger
Re: svn commit: r1908380 - in /httpd/httpd/trunk: changes-entries/errorlogformat.txt include/util_time.h server/log.c server/util_time.c [ In reply to ]
On 3/14/23 12:11 PM, ylavic@apache.org wrote:
> Author: ylavic
> Date: Tue Mar 14 11:11:24 2023
> New Revision: 1908380
>
> URL: http://svn.apache.org/viewvc?rev=1908380&view=rev
> Log:
> core: Add formats %{z} and %{strftime-format} to ErrorLogFormat. PR 62161.
>
> %{z} prints the timezone offset (i.e. "[+-]nnnn") and %{strftime-format} allows
> any %-format handled by [apr_]strftime().
>
> * include/util_time.h():
> Define new AP_CTIME_OPTION_GMTOFF option for ap_recent_ctime_ex().
>
> * server/util_time.c(ap_recent_ctime_ex):
> Handle AP_CTIME_OPTION_GMTOFF to print "[+-]nnnn" timezone.
>
> * server/log.c(log_ctime):
> If the format contains a '%' it's for strftime(), otherwise it's builtin
> with new 'z' as AP_CTIME_OPTION_GMTOFF.
>
>
> Added:
> httpd/httpd/trunk/changes-entries/errorlogformat.txt
> Modified:
> httpd/httpd/trunk/include/util_time.h
> httpd/httpd/trunk/server/log.c
> httpd/httpd/trunk/server/util_time.c
>
>
> Modified: httpd/httpd/trunk/include/util_time.h
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/include/util_time.h?rev=1908380&r1=1908379&r2=1908380&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/include/util_time.h (original)
> +++ httpd/httpd/trunk/include/util_time.h Tue Mar 14 11:11:24 2023
> @@ -47,6 +47,8 @@ extern "C" {
> #define AP_CTIME_OPTION_USEC 0x1
> /* Use more compact ISO 8601 format */
> #define AP_CTIME_OPTION_COMPACT 0x2
> +/* Add timezone offset from GMT ([+-]hhmm) */
> +#define AP_CTIME_OPTION_GMTOFF 0x4

Not really important for trunk, but doesn't this require a minor bump for the backport?

>
>
> /**
>

Regards

Rüdiger
Re: svn commit: r1908380 - in /httpd/httpd/trunk: changes-entries/errorlogformat.txt include/util_time.h server/log.c server/util_time.c [ In reply to ]
On Fri, Mar 17, 2023 at 9:36?AM Ruediger Pluem <rpluem@apache.org> wrote:
>
> I guess the
>
> #include "apr_strings.h"
>
> added in r1908380 is no longer needed as well?

Yep, removed in r1908556.


Regards;
Yann.
Re: svn commit: r1908380 - in /httpd/httpd/trunk: changes-entries/errorlogformat.txt include/util_time.h server/log.c server/util_time.c [ In reply to ]
On Fri, Mar 17, 2023 at 9:37?AM Ruediger Pluem <rpluem@apache.org> wrote:
>
> On 3/14/23 12:11 PM, ylavic@apache.org wrote:
> >
> > +++ httpd/httpd/trunk/include/util_time.h Tue Mar 14 11:11:24 2023
> > @@ -47,6 +47,8 @@ extern "C" {
> > #define AP_CTIME_OPTION_USEC 0x1
> > /* Use more compact ISO 8601 format */
> > #define AP_CTIME_OPTION_COMPACT 0x2
> > +/* Add timezone offset from GMT ([+-]hhmm) */
> > +#define AP_CTIME_OPTION_GMTOFF 0x4
>
> Not really important for trunk, but doesn't this require a minor bump for the backport?

And that one in r1908557.

Thanks;
Yann.