Message ID | 1417573661-9902-1-git-send-email-james@loowit.net |
---|---|
State | Superseded |
Headers |
Received: (qmail 26575 invoked by alias); 3 Dec 2014 02:28:40 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <libc-alpha.sourceware.org> List-Unsubscribe: <mailto:libc-alpha-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:libc-alpha-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 25841 invoked by uid 89); 3 Dec 2014 02:28:00 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.6 required=5.0 tests=BAYES_00, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-ig0-f177.google.com X-Received: by 10.50.66.162 with SMTP id g2mr6278234igt.40.1417573675313; Tue, 02 Dec 2014 18:27:55 -0800 (PST) From: James Perkins <james@loowit.net> To: libc-alpha@sourceware.org Subject: [PATCH v2 1/2] [BZ #16141] strptime: extend %z range limits Date: Tue, 2 Dec 2014 18:27:40 -0800 Message-Id: <1417573661-9902-1-git-send-email-james@loowit.net> |
Commit Message
James Perkins
Dec. 3, 2014, 2:27 a.m. UTC
This is a fix for [BZ #16141] strptime %z offset restriction. strptime supports the parsing of a timezone offset from UTC time into the broken-out time field tm_gmtoff. It supports timezone offsets between UTC-12:00 and UTC+12:00, returning an error (NULL) for values outside that range. However, time zone offsets outside the current range limits exist both currently and historically: * Present day: - Pacific/Auckland (New Zealand) summer time +13:00 - Pacific/Kiritimati (Christmas Island) +14:00 - Pacific/Apia (Samoa) summer time +14:00 * Historical offsets exceeded +15:00/-15:00 * POSIX supports -24:00 to +25:00 * Paul Eggert notes: - https://sourceware.org/ml/libc-alpha/2014-12/msg00068.html So, extend the timezone offset range to UTC-24:00 to UTC+25:00. James 2014-12-02 James Perkins james@loowit.net [BZ #16141] * time/strptime_l.c (__strptime_internal): Extend %z timezone offset range limits to UTC-24:00 to UTC+25:00 to parse current and historical uses. --- time/strptime_l.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
Comments
On 3 December 2014 at 02:27, James Perkins <james@loowit.net> wrote: > This is a fix for [BZ #16141] strptime %z offset restriction. > > strptime supports the parsing of a timezone offset from UTC time into the > broken-out time field tm_gmtoff. It supports timezone offsets between > UTC-12:00 and UTC+12:00, returning an error (NULL) for values outside > that range. > > However, time zone offsets outside the current range limits exist both > currently and historically: > > * Present day: > - Pacific/Auckland (New Zealand) summer time +13:00 > - Pacific/Kiritimati (Christmas Island) +14:00 > - Pacific/Apia (Samoa) summer time +14:00 > * Historical offsets exceeded +15:00/-15:00 > * POSIX supports -24:00 to +25:00 > * Paul Eggert notes: > - https://sourceware.org/ml/libc-alpha/2014-12/msg00068.html > > So, extend the timezone offset range to UTC-24:00 to UTC+25:00. > > James > > 2014-12-02 James Perkins james@loowit.net > > [BZ #16141] > * time/strptime_l.c (__strptime_internal): Extend %z timezone > offset range limits to UTC-24:00 to UTC+25:00 to parse current > and historical uses. > --- > time/strptime_l.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) The patch looks ok modulo the nit below. It would be good to add a test however. > diff --git a/time/strptime_l.c b/time/strptime_l.c > index b3a612e..1cdce41 100644 > --- a/time/strptime_l.c > +++ b/time/strptime_l.c > @@ -777,7 +777,10 @@ __strptime_internal (rp, fmt, tmp, statep LOCALE_PARAM) > return NULL; > val = (val / 100) * 100 + ((val % 100) * 50) / 30; > } > - if (val > 1200) > + /* valid range UTC-24 to +25, ala POSIX */ The comment should be a sentence so start with a capital and end with a full-stop and a couple of spaces. > + if (neg && val > 2400) > + return NULL; > + if (!neg && val > 2500) > return NULL; > tm->tm_gmtoff = (val * 3600) / 100; > if (neg) > -- > 1.7.9.5 >
On Wed, Dec 3, 2014 at 1:41 AM, Will Newton <will.newton@linaro.org> wrote: > On 3 December 2014 at 02:27, James Perkins <james@loowit.net> wrote: >> This is a fix for [BZ #16141] strptime %z offset restriction. > The patch looks ok modulo the nit below. It would be good to add a test however. I'm reworking it to add tests. >> + /* valid range UTC-24 to +25, ala POSIX */ > > The comment should be a sentence so start with a capital and end with > a full-stop and a couple of spaces. Thanks for the feedback, Will. In my rework I will also drop the range limit per discussion with Paul Eggert, to honor a -9959 to +9959 input offset range, so the comment will go away. Cheers, James
On 12/03/2014 05:28 PM, James Perkins wrote: > On Wed, Dec 3, 2014 at 1:41 AM, Will Newton <will.newton@linaro.org> wrote: >> On 3 December 2014 at 02:27, James Perkins <james@loowit.net> wrote: >>> This is a fix for [BZ #16141] strptime %z offset restriction. >> The patch looks ok modulo the nit below. It would be good to add a test however. > > I'm reworking it to add tests. Please keep in mind that as your change gets more complicated it will eventually require copyright assignment. Please review: https://sourceware.org/glibc/wiki/Contribution%20checklist All glibc developers should remind new contributors to look through the contribution checklist :-) Cheers, Carlos.
On Wed, Dec 3, 2014 at 9:41 PM, Carlos O'Donell <carlos@redhat.com> wrote: > Please keep in mind that as your change gets more complicated it will > eventually require copyright assignment. Thank you, Carlos. I would agree that a Copyright assignment is required if I contribute any creative work. As it stands I've added a few lines to an array initializer in one file and removed more lines than I added in another, so I can safely say none of my patch content is creative. > Please review: > https://sourceware.org/glibc/wiki/Contribution%20checklist I am glad to say I read through once to determine how best to submit my patches. It was very helpful in explaining the patch format and precise expected content. I will make sure to read through it again before post my next submission. > All glibc developers should remind new contributors to look through > the contribution checklist :-) Agreed. As this patch series represents my first contribution to glibc, I regret there are preferences of the community I missed the first pass through. I'm grateful for the constructive reviews I have received from Paul Eggert, Will Newton, and yourself. I feel I've had an encouraging introduction. Cheers, James
On 12/04/2014 04:23 PM, James Perkins wrote: > As this patch series represents my first contribution to glibc, I regret there > are preferences of the community I missed the first pass through. I'm > grateful for the constructive reviews I have received from Paul Eggert, Will > Newton, and yourself. I feel I've had an encouraging introduction. I'm very glad to hear that. I look forward to your contributions. Cheers, Carlos.
diff --git a/time/strptime_l.c b/time/strptime_l.c index b3a612e..1cdce41 100644 --- a/time/strptime_l.c +++ b/time/strptime_l.c @@ -777,7 +777,10 @@ __strptime_internal (rp, fmt, tmp, statep LOCALE_PARAM) return NULL; val = (val / 100) * 100 + ((val % 100) * 50) / 30; } - if (val > 1200) + /* valid range UTC-24 to +25, ala POSIX */ + if (neg && val > 2400) + return NULL; + if (!neg && val > 2500) return NULL; tm->tm_gmtoff = (val * 3600) / 100; if (neg)