Re: Changes in nl_langinfo() and strftime() API in glibc

[Rafal Luzynski]

24.01.2018 10:03 Bruno Haible <address@hidden> wrote:
>
>
> Rafal Luzynski wrote:
> > Unfortunately,
> > I'm afraid it will not even compile without the changes in nl_langinfo()
> > because ALTMON_1 and _NL_ABALTMON_1 symbols are undefined.
>
> No, Paul's patch was correct. Paul would never push a commit that does not
> compile.

I didn't mean it does not compile, I just wasn't sure because I did not
test this update.

> Paul Eggert wrote:
> > I don't know the nl_langinfo code as well, though,
>
> Done as follows:

Thank you, Bruno. I'm sorry I don't have time to compile and test
it now. But just at first sight: I can't see _NL_ABALTMON_* here.
Isn't it necessary?

Please also see the comments below:

>
> [...]
> diff --git a/doc/posix-functions/nl_langinfo.texi
> b/doc/posix-functions/nl_langinfo.texi
> index cd9e523..529b0e9 100644
> --- a/doc/posix-functions/nl_langinfo.texi
> +++ b/doc/posix-functions/nl_langinfo.texi
> @@ -15,6 +15,10 @@ Minix 3.1.8, mingw, MSVC 14, BeOS.
> The constant @code{CODESET} is not supported on some platforms:
> glibc 2.0.6, OpenBSD 3.8.
> @item
> +The constants @code{ALTMON_1} to @code{ALTMON_12} are not defined on some
> +platforms:
> +glibc 2.26 and many others.
> address@hidden

To be more precise: we are adding ALTMON_1 since glibc 2.27 (as GNU
extension) so it is not defined on glibc 2.26 and older.
(This is to avoid confusion: glibc 2.26 and newer? just glibc 2.26?)

> [...]
> diff --git a/lib/nl_langinfo.c b/lib/nl_langinfo.c
> index 725ccf6..b93f7be 100644
> --- a/lib/nl_langinfo.c
> +++ b/lib/nl_langinfo.c
> [...]
> @@ -228,28 +246,49 @@ nl_langinfo (nl_item item)
> return (char *) abdays[item - ABDAY_1];
> return nlbuf;
> }
> - case MON_1:
> - case MON_2:
> - case MON_3:
> - case MON_4:
> - case MON_5:
> - case MON_6:
> - case MON_7:
> - case MON_8:
> - case MON_9:
> - case MON_10:
> - case MON_11:
> - case MON_12:
> - {
> - static char const months[][sizeof "September"] = {
> - "January", "February", "March", "April", "May", "June", "July",
> - "September", "October", "November", "December"
> - };
> + {
> + static char const months[][sizeof "September"] = {
> + "January", "February", "March", "April", "May", "June", "July",
> + "September", "October", "November", "December"
> + };
> + case MON_1:
> + case MON_2:
> + case MON_3:
> + case MON_4:
> + case MON_5:
> + case MON_6:
> + case MON_7:
> + case MON_8:
> + case MON_9:
> + case MON_10:
> + case MON_11:
> + case MON_12:
> tmm.tm_mon = item - MON_1;
> if (!strftime (nlbuf, sizeof nlbuf, "%B", &tmm))
> return (char *) months[item - MON_1];
> return nlbuf;

This looks correct: if nl_langinfo(MON_*) is not supported then you
try to retrieve the month name with stftime("%B"). Although I don't
understand why "case MON_*" has been removed and added. Formatting
changes maybe?

> - }
> + case ALTMON_1:
> + case ALTMON_2:
> + case ALTMON_3:
> + case ALTMON_4:
> + case ALTMON_5:
> + case ALTMON_6:
> + case ALTMON_7:
> + case ALTMON_8:
> + case ALTMON_9:
> + case ALTMON_10:
> + case ALTMON_11:
> + case ALTMON_12:
> + tmm.tm_mon = item - ALTMON_1;
> + /* The platforms without nl_langinfo() don't support strftime with %OB.
> + We don't even need to try. */
> + #if 0
> + if (!strftime (nlbuf, sizeof nlbuf, "%OB", &tmm))
> + #endif

Not really, I think that this removed implementation would be useful
sometimes. As far as I know OS X does support strftime("%OB") and
does support nl_langinfo() but does not support ALTMON_* series.

> + if (!strftime (nlbuf, sizeof nlbuf, "%B", &tmm))
> + return (char *) months[item - ALTMON_1];
> + return nlbuf;
> + }

Otherwise OK: if neither ALTMON_* nor strftime("%OB") is available
on a particular platform then falling back to MON_* series is correct.

I'm sorry for this short review. I'm afraid I will not have time
to review more thoroughly this week.

Thank you for your support.

Regards,

Rafal