Hello Paul-san,
Thank you for your porting Glibc strftime changes to the
implementation in Gnulib.
Let me just make a few comments to make it better code.
From: Paul Eggert <egg...@cs.ucla.edu>
Subject: [PATCH] nstrftime: merge glibc strftime changes
Date: Thu, 21 Feb 2019 20:08:46 -0800
> [...]
> diff --git a/lib/nstrftime.c b/lib/nstrftime.c
> index fd731e1c4..a45d809b2 100644
> --- a/lib/nstrftime.c
> +++ b/lib/nstrftime.c
> @@ -418,7 +418,7 @@ iso_week_days (int yday, int wday)
>
> static size_t __strftime_internal (STREAM_OR_CHAR_T *, STRFTIME_ARG (size_t)
> const CHAR_T *, const struct tm *,
> - bool, bool *
> + int, bool, bool *
For the implementation of nstrftime.c in Gnulib, upcase is added as an
argument of __strftime_internal function compared to strftime_l.c in
Glibc.
This (upcase) argument is used mainly for the process described in the
beginning of format_char, such as %a, %A, %[bh], %B, etc.
On the other hand, tzset_called argument is used in the process
described at the end of format_char, i.e. %z.
Therefore, I think these arguments are in this order.
Since yr_spec argument to be added this time is used in %EY and %Ey of
format_char, it is better to make the argument order the same.
Recommend instead:
| @@ -418,7 +418,7 @@ iso_week_days (int yday, int wday)
|
| static size_t __strftime_internal (STREAM_OR_CHAR_T *, STRFTIME_ARG (size_t)
| const CHAR_T *, const struct tm *,
| - bool, bool *
| + bool, int, bool *
| extra_args_spec LOCALE_PARAM);
|
| /* Write information from TP into S according to the format
> [...]
> @@ -433,7 +433,7 @@ my_strftime (STREAM_OR_CHAR_T *s, STRFTIME_ARG (size_t
> maxsize)
> const struct tm *tp extra_args_spec LOCALE_PARAM)
> {
> bool tzset_called = false;
> - return __strftime_internal (s, STRFTIME_ARG (maxsize) format, tp,
> + return __strftime_internal (s, STRFTIME_ARG (maxsize) format, tp, 0,
Ditto. Recommend instead:
| @@ -434,7 +434,7 @@ my_strftime (STREAM_OR_CHAR_T *s, STRFTIME_ARG (size_t
maxsize)
| {
| bool tzset_called = false;
| return __strftime_internal (s, STRFTIME_ARG (maxsize) format, tp,
| - false, &tzset_called extra_args LOCALE_ARG);
| + false, 0, &tzset_called extra_args LOCALE_ARG);
| }
| #if defined _LIBC && ! FPRINTFTIME
| libc_hidden_def (my_strftime)
> [...]
> @@ -446,7 +446,8 @@ libc_hidden_def (my_strftime)
> static size_t
> __strftime_internal (STREAM_OR_CHAR_T *s, STRFTIME_ARG (size_t maxsize)
> const CHAR_T *format,
> - const struct tm *tp, bool upcase, bool *tzset_called
> + const struct tm *tp, int yr_spec,
> + bool upcase, bool *tzset_called
Ditto. Recommend instead:
| @@ -446,8 +446,8 @@ libc_hidden_def (my_strftime)
| static size_t
| __strftime_internal (STREAM_OR_CHAR_T *s, STRFTIME_ARG (size_t maxsize)
| const CHAR_T *format,
| - const struct tm *tp, bool upcase, bool *tzset_called
| - extra_args_spec LOCALE_PARAM)
| + const struct tm *tp, bool upcase, int yr_spec,
| + bool *tzset_called extra_args_spec LOCALE_PARAM)
| {
| #if defined _LIBC && defined USE_IN_EXTENDED_LOCALE_MODEL
| struct __locale_data *const current = loc->__locales[LC_TIME];
> [...]
> @@ -863,13 +864,13 @@ __strftime_internal (STREAM_OR_CHAR_T *s, STRFTIME_ARG
> (size_t maxsize)
> subformat:
> {
> size_t len = __strftime_internal (NULL, STRFTIME_ARG ((size_t)
> -1)
> - subfmt,
> - tp, to_uppcase, tzset_called
> + subfmt, tp, yr_spec,
> + to_uppcase, tzset_called
> extra_args LOCALE_ARG);
> add (len, __strftime_internal (p,
> STRFTIME_ARG (maxsize - i)
> - subfmt,
> - tp, to_uppcase, tzset_called
> + subfmt, tp, yr_spec,
> + to_uppcase, tzset_called
Ditto. Recommend instead:
| @@ -864,13 +864,15 @@ __strftime_internal (STREAM_OR_CHAR_T *s, STRFTIME_ARG
(size_t maxsize)
| {
| size_t len = __strftime_internal (NULL, STRFTIME_ARG ((size_t)
-1)
| subfmt,
| - tp, to_uppcase, tzset_called
| - extra_args LOCALE_ARG);
| + tp, to_uppcase, yr_spec,
| + tzset_called extra_args
| + LOCALE_ARG);
| add (len, __strftime_internal (p,
| STRFTIME_ARG (maxsize - i)
| subfmt,
| - tp, to_uppcase, tzset_called
| - extra_args LOCALE_ARG));
| + tp, to_uppcase, yr_spec,
| + tzset_called extra_args
| + LOCALE_ARG));
| }
| break;
|
Thank you for your consideration.
Regards,
TAMUKI Shoichi
