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
> 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