strptime() lacks support for %z

strptime() lacks support for %z

[Jan Včelák]

Hello list.

(Please CC me in replies, I'm not subscribed to the list.)

The strptime() function lacks support for %z (and %Z) format specifier.

I'm mostly interested in '%z' because we are using it the Knot DNS
project and we have been notified by one of our users running Alpine
Linux that the previously written time stamps cannot be parsed.

I want to contribute to musl by implementing the missing specifier.
But honestly I don't know how to start. The strptime() function needs
some refactoring otherwise the change would be ugly (or copy-pasta).

Are you interested in this feature? Please, can you point me to your
coding style guidelines? How can I check I haven't broken anything else?
Do you have unit tests or something?

Cheers,

Jan

Re: strptime() lacks support for %z

[Szabolcs Nagy]

* Jan V??el?k <jan.vcelak@nic.cz> [2015-08-13 20:56:47 +0200]:
> (Please CC me in replies, I'm not subscribed to the list.)
> 
> The strptime() function lacks support for %z (and %Z) format specifier.
> 
> I'm mostly interested in '%z' because we are using it the Knot DNS
> project and we have been notified by one of our users running Alpine
> Linux that the previously written time stamps cannot be parsed.
> 
> I want to contribute to musl by implementing the missing specifier.
> But honestly I don't know how to start. The strptime() function needs
> some refactoring otherwise the change would be ugly (or copy-pasta).
> 
> Are you interested in this feature? Please, can you point me to your
> coding style guidelines? How can I check I haven't broken anything else?
> Do you have unit tests or something?
> 

we have some tests, but not for strptime unfortunately.

the problem with parsing timezones is that it's not posix
so the desired semantics is not clear (struct tm has no
tz field in posix and it is not obvious how that should
be treated in other apis that use struct tm.. glibc does
something but it should be verified to give consistent
behaviour if we add this to musl and there might be parsing
corner cases when %z is not surrounded by spaces..).

Re: strptime() lacks support for %z

[Rich Felker]

On Thu, Aug 13, 2015 at 08:56:47PM +0200, Jan Včelák wrote:
> Hello list.
> 
> (Please CC me in replies, I'm not subscribed to the list.)
> 
> The strptime() function lacks support for %z (and %Z) format specifier.
> 
> I'm mostly interested in '%z' because we are using it the Knot DNS
> project and we have been notified by one of our users running Alpine
> Linux that the previously written time stamps cannot be parsed.
> 
> I want to contribute to musl by implementing the missing specifier.
> But honestly I don't know how to start. The strptime() function needs
> some refactoring otherwise the change would be ugly (or copy-pasta).

It looks to me like it can just reuse numeric_range with a fixup at
the end like what's done for 'want_century'. This should be a very
small change, I think.

> Are you interested in this feature?

It seems reasonable to consider based on the criteria we usually go
by, which are detailed in some old mailing list threads. (These should
probably be polished up and added to the wiki.)

> Please, can you point me to your
> coding style guidelines?

There are some very basic style guides on the wiki, but they mostly
address presentation style, not the actual code content. At some point
we should expand this to a proper coding style doc. For now the best
general principle is to mimic the style of the code you're working on.

> How can I check I haven't broken anything else?
> Do you have unit tests or something?

Yes, but I don't think there's good coverage for strptime. :(
The test repo is here:

http://nsz.repo.hu/git/?p=libc-test

Contributions of tests for strptime or other functionality that
currently lacks coverage would be very welcome too.

Rich

Re: strptime() lacks support for %z

[Jan Včelák]

Hello Szabolcs, 

(Please CC me in replies, I'm not subscribed to the list.)

On Thursday, August 13, 2015 10:06:50 PM Szabolcs Nagy wrote:
> the problem with parsing timezones is that it's not posix
> so the desired semantics is not clear (struct tm has no
> tz field in posix and it is not obvious how that should
> be treated in other apis that use struct tm.. glibc does
> something but it should be verified to give consistent
> behaviour if we add this to musl and there might be parsing
> corner cases when %z is not surrounded by spaces..).

I know it's not specified in POSIX strptime, however it is specified in 
strftime. I'm not sure how strictly do you want to stick to POSIX, but it 
seems reasonable to me to have the equivalent format support in both 
functions, so you can write the time stamp and parse it back.

Anyway, the format in strftime is simple and on fixed width, +hhmm or -hhmm.

http://git.musl-libc.org/cgit/musl/tree/src/time/strftime.c#n175

So what do you suggest? Should we make a workaround for this in our project 
code? Or will you consider supporting non-POSIX format specifier in strptime 
in musl?

Regards,

Jan

Re: strptime() lacks support for %z

[Szabolcs Nagy]

* Jan V??el?k <jan.vcelak@nic.cz> [2015-08-14 10:00:23 +0200]:
> On Thursday, August 13, 2015 10:06:50 PM Szabolcs Nagy wrote:
> > the problem with parsing timezones is that it's not posix
> > so the desired semantics is not clear (struct tm has no
> > tz field in posix and it is not obvious how that should
> > be treated in other apis that use struct tm.. glibc does
> > something but it should be verified to give consistent
> > behaviour if we add this to musl and there might be parsing
> > corner cases when %z is not surrounded by spaces..).
> 
> I know it's not specified in POSIX strptime, however it is specified in 
> strftime. I'm not sure how strictly do you want to stick to POSIX, but it 
> seems reasonable to me to have the equivalent format support in both 
> functions, so you can write the time stamp and parse it back.
> 
> Anyway, the format in strftime is simple and on fixed width, +hhmm or -hhmm.
> 

strftime uses tzset (timezone from TZ) but strptime cannot
set TZ so it must put the timezone somewhere else.

so strptime can't be consistent with strftime.
with tz in struct tm, mktime/localtime no longer roundtrip.

> So what do you suggest? Should we make a workaround for this in our project 
> code? Or will you consider supporting non-POSIX format specifier in strptime 
> in musl?

if you want portable code then you cannot rely on the
libc supporting %z, but otherwise it seems to be a
reasonable extension, just make sure the implementation
does not break any conforming code that does not use %z.

Re: strptime() lacks support for %z

[Jan Včelák]

On Friday, August 14, 2015 11:34:32 AM Szabolcs Nagy wrote:
> if you want portable code then you cannot rely on the
> libc supporting %z, but otherwise it seems to be a
> reasonable extension, just make sure the implementation
> does not break any conforming code that does not use %z.

OK then. We won't relay on '%z' in strptime and will fix that in our 
implementation. Anyway, thank you for useful input.

Regards,

Jan

Re: strptime() lacks support for %z

[Rich Felker]

On Fri, Aug 14, 2015 at 11:34:32AM +0200, Szabolcs Nagy wrote:
> * Jan V??el?k <jan.vcelak@nic.cz> [2015-08-14 10:00:23 +0200]:
> > On Thursday, August 13, 2015 10:06:50 PM Szabolcs Nagy wrote:
> > > the problem with parsing timezones is that it's not posix
> > > so the desired semantics is not clear (struct tm has no
> > > tz field in posix and it is not obvious how that should
> > > be treated in other apis that use struct tm.. glibc does
> > > something but it should be verified to give consistent
> > > behaviour if we add this to musl and there might be parsing
> > > corner cases when %z is not surrounded by spaces..).
> > 
> > I know it's not specified in POSIX strptime, however it is specified in 
> > strftime. I'm not sure how strictly do you want to stick to POSIX, but it 
> > seems reasonable to me to have the equivalent format support in both 
> > functions, so you can write the time stamp and parse it back.
> > 
> > Anyway, the format in strftime is simple and on fixed width, +hhmm or -hhmm.
> > 
> 
> strftime uses tzset (timezone from TZ) but strptime cannot
> set TZ so it must put the timezone somewhere else.
> 
> so strptime can't be consistent with strftime.
> with tz in struct tm, mktime/localtime no longer roundtrip.

I don't follow. strftime uses the extended fields from struct tm. I
don't see anywhere it depends on tzset, nor reasons why strptime would
need tzset to be called. Am I missing something?

Rich

Re: strptime() lacks support for %z

[Szabolcs Nagy]

* Rich Felker <dalias@libc.org> [2015-08-14 10:15:40 -0400]:
> On Fri, Aug 14, 2015 at 11:34:32AM +0200, Szabolcs Nagy wrote:
> > * Jan V??el?k <jan.vcelak@nic.cz> [2015-08-14 10:00:23 +0200]:
> > > On Thursday, August 13, 2015 10:06:50 PM Szabolcs Nagy wrote:
> > > > the problem with parsing timezones is that it's not posix
> > > > so the desired semantics is not clear (struct tm has no
> > > > tz field in posix and it is not obvious how that should
> > > > be treated in other apis that use struct tm.. glibc does
> > > > something but it should be verified to give consistent
> > > > behaviour if we add this to musl and there might be parsing
> > > > corner cases when %z is not surrounded by spaces..).
> > > 
> > > I know it's not specified in POSIX strptime, however it is specified in 
> > > strftime. I'm not sure how strictly do you want to stick to POSIX, but it 
> > > seems reasonable to me to have the equivalent format support in both 
> > > functions, so you can write the time stamp and parse it back.
> > > 
> > > Anyway, the format in strftime is simple and on fixed width, +hhmm or -hhmm.
> > > 
> > 
> > strftime uses tzset (timezone from TZ) but strptime cannot
> > set TZ so it must put the timezone somewhere else.
> > 
> > so strptime can't be consistent with strftime.
> > with tz in struct tm, mktime/localtime no longer roundtrip.
> 
> I don't follow. strftime uses the extended fields from struct tm. I
> don't see anywhere it depends on tzset, nor reasons why strptime would
> need tzset to be called. Am I missing something?
> 

it seems %z does not use it in musl, %Z does

__tm_to_tzname calls do_tzset()

posix says
"Local timezone information is used as though strftime() called tzset()"

Re: strptime() lacks support for %z

[Szabolcs Nagy]

* Szabolcs Nagy <nsz@port70.net> [2015-08-14 17:00:39 +0200]:
> * Rich Felker <dalias@libc.org> [2015-08-14 10:15:40 -0400]:
> > On Fri, Aug 14, 2015 at 11:34:32AM +0200, Szabolcs Nagy wrote:
> > > * Jan V??el?k <jan.vcelak@nic.cz> [2015-08-14 10:00:23 +0200]:
> > > > On Thursday, August 13, 2015 10:06:50 PM Szabolcs Nagy wrote:
> > > > > the problem with parsing timezones is that it's not posix
> > > > > so the desired semantics is not clear (struct tm has no
> > > > > tz field in posix and it is not obvious how that should
> > > > > be treated in other apis that use struct tm.. glibc does
> > > > > something but it should be verified to give consistent
> > > > > behaviour if we add this to musl and there might be parsing
> > > > > corner cases when %z is not surrounded by spaces..).
> > > > 
> > > > I know it's not specified in POSIX strptime, however it is specified in 
> > > > strftime. I'm not sure how strictly do you want to stick to POSIX, but it 
> > > > seems reasonable to me to have the equivalent format support in both 
> > > > functions, so you can write the time stamp and parse it back.
> > > > 
> > > > Anyway, the format in strftime is simple and on fixed width, +hhmm or -hhmm.
> > > > 
> > > 
> > > strftime uses tzset (timezone from TZ) but strptime cannot
> > > set TZ so it must put the timezone somewhere else.
> > > 
> > > so strptime can't be consistent with strftime.
> > > with tz in struct tm, mktime/localtime no longer roundtrip.
> > 
> > I don't follow. strftime uses the extended fields from struct tm. I
> > don't see anywhere it depends on tzset, nor reasons why strptime would
> > need tzset to be called. Am I missing something?
> > 
> 
> it seems %z does not use it in musl, %Z does
> 
> __tm_to_tzname calls do_tzset()
> 
> posix says
> "Local timezone information is used as though strftime() called tzset()"

ah ok, the tz name and offset are independent

i expected both of those would be used from TZ in
strftime, but musl already uses the tm struct offset.

then implementing %z in strptime is less intrusive
than i thought.

Re: strptime() lacks support for %z

[Rich Felker]

On Fri, Aug 14, 2015 at 05:08:23PM +0200, Szabolcs Nagy wrote:
> > > > strftime uses tzset (timezone from TZ) but strptime cannot
> > > > set TZ so it must put the timezone somewhere else.
> > > > 
> > > > so strptime can't be consistent with strftime.
> > > > with tz in struct tm, mktime/localtime no longer roundtrip.
> > > 
> > > I don't follow. strftime uses the extended fields from struct tm. I
> > > don't see anywhere it depends on tzset, nor reasons why strptime would
> > > need tzset to be called. Am I missing something?
> > > 
> > 
> > it seems %z does not use it in musl, %Z does
> > 
> > __tm_to_tzname calls do_tzset()
> > 
> > posix says
> > "Local timezone information is used as though strftime() called tzset()"
> 
> ah ok, the tz name and offset are independent
> 
> i expected both of those would be used from TZ in
> strftime, but musl already uses the tm struct offset.

Because we support zoneinfo files, printing this information with
strftime does not seem to be possible without using the data from the
struct tm. There can be multiple different offsets and zone names
depending on the date, and strftime does not seem to require a
canonical-form struct tm, so trying to derive the zone/offset in
effect from TZ and the standard struct tm members does not seem like
it could be done reliably. Unfortunately this is all a bit of a mess
because POSIX is in some sort of denial that zoneinfo is necessary...

Rich