[RFC][PATCH v9 2/6] Implement alternative month names (bug 10871).
Joseph Myers
joseph@codesourcery.com
Fri Oct 27 17:04:00 GMT 2017
On Fri, 27 Oct 2017, Zack Weinberg wrote:
> The code changes look good to me, except for a few small concerns
> listed below. Also, before landing I would like Joseph Myers (cc:ed)
> to positively confirm that this patch introduces no new ISO C or POSIX
> conformance issues, and will not tie our hands if this or a similar
> feature is standardized in the future.
I believe this patch properly handles namespace issues and avoids any
problems with improperly changing enum values or making them improperly
depend on feature test macros defined. I believe it properly deals with
allowing existing locales using existing POSIX features to continue to
work by copying the month values as needed.
There should be a GCC bug filed, if there isn't one already, for allowing
%OB / %Ob in strftime formats (with appropriate warnings in ISO C pedantic
modes about being an extension). Tests may need to disable errors for
format warnings when built with a GCC version without that support.
> > @@ -1015,6 +1035,10 @@ __strptime_internal (const char *rp, const char *fmt,
> > struct tm *tmp,
> > case 'O':
> > switch (*fmt++)
> > {
> > + case 'B':
> > + /* Undo the increment and continue. */
> > + fmt--;
> > + break;
>
> This is subtly wrong, and I had to read a big chunk of the entire
> function to understand what it was meant to do and how it doesn't
> actually work. Please change to
>
> ] case 'O':
> ] switch (*fmt++)
> ] {
> ] + case 'B':
> ] + /* Match month name. Reprocess as plain 'B'. */
> ] + fmt--;
> ] + goto start_over;
Also, the POSIX proposal accepted for issue 8 has strptime %Ob and
strptime %OB handled the same, despite that POSIX proposal not including
strftime %Ob. (This is only an argument for %Ob handling for strptime to
go in this patch rather than the later one adding alternative abbreviated
months, however.)
--
Joseph S. Myers
joseph@codesourcery.com
More information about the Libc-alpha
mailing list