Re: [PATCH v2] date.c: Support iso8601 timezone formats
- From
Haitao Li <lihaitao@gmail.com>
- Date
- Sep 8, 2011, 02:53 UTC
- Message-ID
- <CABk5KLhXyQOFLegdpNcHzfwxX4LwjcG_eE3TVW1eDgTP=4kYGQ@mail.gmail.com>
- In-Reply-To
- <7vaaag4794.fsf@alter.siamese.dyndns.org>
Show 9 quoted lines
>> + } else {
>> + /* Only hours specified */
>
> That comment belongs to inside the following if() {...}.
>
>> + if (n == 1 || n == 2) {
>
> ... i.e. here.
>Good catch!
Show 10 quoted lines
>> - if (min < 60 && n > 2) {
>> + if (n > 0 && min < 60 && hour < 25) {
>
> What is this "hour < 25" about? Aren't we talking about the UTC offset
> value that come after the [-+] sign?
>
> I do not mind adding a new check, but I do mind if it adds a check with
> not much value. Even at Pacific/Kiritimati, the offset is 14; the new
> check seems a bit too lenient.
>I think it's not "too" lenient. UTC+14 was "invented" in 1995 [1]. Maybe UTC+15 would be added someday for some reason? How about changing to "hour < 24", this is how ICU checks offset validity [2].
1. http://en.wikipedia.org/wiki/UTC%2B14#History 2. http://bugs.icu-project.org/trac/browser/icu/tags/release-4-8-1/source/i18n/timezone.cpp#L1482