Fw: [PATCH 1/2] strftime.c(__strftime): add %i, %q, %v, tests; tweak %Z docs

[Corinna Vinschen]

On Sep 19 11:51, C Howland wrote:
> >
> > ------------------------------
> > *From:* Newlib <newlib-bounces+craig.howland=caci.com@sourceware.org> on
> > behalf of Brian Inglis <Brian.Inglis@SystematicSW.ab.ca>
> > *Sent:* Saturday, September 17, 2022 1:00 AM
> > *To:* Newlib <newlib@Sourceware.org>
> > *Subject:* [PATCH 1/2] strftime.c(__strftime): add %i, %q, %v, tests;
> > tweak %Z docs
> >
> >
> >
> > newlib/libc/time/strftime.c(__strftime):
> > %i year in century [00..99] Synonym for "%y". Non-POSIX extension.
> > [tm_year]
> > %q GNU quarter of the year (from `<<1>>' to `<<4>>') [tm_mon]
> > %v OSX/Ruby VMS/Oracle date "%d-%b-%Y". Non-POSIX extension. [tm_mday,
> > tm_mon, tm_year]
> > add %i %q %v tests
> > %Z clarify current time zone *abbreviation* not "name" [tm_isdst]
> > ---
> >  newlib/libc/time/strftime.c | 67 +++++++++++++++++++++++++++++++++++--
> >  1 file changed, 64 insertions(+), 3 deletions(-)
> >
> >
> While the additions themselves nominally look good, all being extensions
> they ought to be gated by the appropriate ifdefs, and the manual would best
> mention what gates are needed to get them.  %q would be __GNU_VISIBLE as
> the gate

You're misunderstanding how these foo_VISIBLE macros are supposed to be
used.  They guard definitions and prototypes in header files depending
on the application layer environment in terms of portability.  They are
not supposed to be used inside newlib code to create conditionalized
library code.

Of course you can argue that the code should not go in unconditionalized
based on target preferences, but that should use one of the documented
macros for newlib builds, e. g., _ELIX_LEVEL, _GLIBC_EXTENSION, or
whatever.

You can also argue that they should not go in at all.  I'm doing this
now for the sake of discussion:

- %i: Where is used and documented?  I don't see this in glibc, not even
  in the latest from the glibc git repo.

- %q: Ditto. For a GNU extension, it's surprisingly absent from the
  most recent glibc code, or do I miss something?

- %v: OSX/Ruby?  Isn't that already gone?  Also, it introduces another
  ambiguous date format where %F or equivalent should be used instead.

It's nice to add features, but it wouldn't hurt to have some
clarification and justification if it's non-standard stuff.
Availability in glibc or in the BSDs is typically justification enough,
but I don't see this here.


Thanks,
Corinna