This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH 1/2] time: in strptime(), make %z accept Z as a time zone [BZ #17886]
- From: Vincent Bernat <Vincent dot Bernat at exoscale dot ch>
- To: James Perkins <james at loowit dot net>
- Cc: libc-alpha at sourceware dot org, Mike Frysinger <vapier at gentoo dot org>
- Date: Wed, 16 Sep 2015 09:57:45 +0200
- Subject: Re: [PATCH 1/2] time: in strptime(), make %z accept Z as a time zone [BZ #17886]
- Authentication-results: sourceware.org; auth=none
- References: <cover dot 1441380709 dot git dot Vincent dot Bernat at exoscale dot ch> <87si6ujiyq dot fsf at zoro dot exoscale dot ch> <20150911172155 dot GA2802 at gmail dot com>
â 11 septembre 2015 10:21 -0700, James Perkins <james@loowit.net>Â:
>> - /* We recognize two formats: if two digits are given, these
>> + /* We recognize three formats: if two digits are given, these
>> specify hours. If fours digits are used, minutes are
>> - also specified. */
>> + also specified. 'Z' is equivalent to +0000. */
>> {
>> val = 0;
>> while (ISSPACE (*rp))
>> ++rp;
>
> OK.
>
> However, note that val is not used until the loop that accumulates a
> sum in val. As an optimization/localization it could be moved down past
> the tests for Z, + and -. e.g.
>
> + val = 0;
> while (n < 4 && *rp >= '0' && *rp <= '9')
> .... accumulate digits in val ...
It is unrelated to the patch and this is also how this is done in
get_number() macro. I prefer to leave this as is if you don't mind.
>> + if (*rp == 'Z')
>> + {
>> + ++rp;
>> + break;
>> + }
>> if (*rp != '+' && *rp != '-')
>> return NULL;
>> bool neg = *rp++ == '-';
>
>
> I've thought about this early break a bit more and I think it's
> incomplete. strptime will not set tm fields if they are not valid
> input. However, it *does* set the tm fields if it parses them
> correctly. So for completeness, you probably want to do this:
>
> + if (*rp == 'Z')
> + {
> + ++rp;
> + tm->tm_gmtoff = 0;
> + break;
> + }
Agreed.
Updated patchset will come shortly.
--
Vincent Bernat â Vincent.Bernat@exoscale.ch
ââ http://www.exoscale.ch