From patchwork Mon Aug 17 20:48:38 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: James Perkins X-Patchwork-Id: 8248 Received: (qmail 18824 invoked by alias); 17 Aug 2015 20:48:51 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 18693 invoked by uid 89); 17 Aug 2015 20:48:50 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.5 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-ob0-f173.google.com X-Received: by 10.182.246.41 with SMTP id xt9mr2925626obc.55.1439844527193; Mon, 17 Aug 2015 13:48:47 -0700 (PDT) From: James Perkins To: GNU libc , Carlos O'Donell , Mike Frysinger , Paul Eggert Subject: [PATCH v6 1/2] [BZ #16141] strptime %z: fix rounding, extend range to +/-9959 Date: Mon, 17 Aug 2015 13:48:38 -0700 Message-Id: <1439844519-17370-2-git-send-email-james@loowit.net> In-Reply-To: <1439844519-17370-1-git-send-email-james@loowit.net> References: <1439844519-17370-1-git-send-email-james@loowit.net> Fixes [BZ #16141] strptime %z offset restriction. Topic: strptime supports a %z input field descriptor, which parses a time zone offset from UTC time into the broken-out time field tm_gmtoff. Problems: 1) In the current implementation, the minutes portion calculation is correct only for minutes evenly divisible by 3. This is because the minutes value is converted to decimal time, but inadequate precision leads to rounding which calculates results that are too low for some values. For example, due to rounding, a +1159 offset string results in an incorrect tm_gmtoff of 43128 (== 11 * 3600 + 58.8 * 60) seconds, instead of 43140 (== 11 * 3600 + 59 * 60) seconds. In contrast, a +1157 offset (minutes divisible by 3) does not cause the bug, and results in a correct tm_gmtoff of 43020. 2) strptime's %z specifier will not parse time offsets less than -1200 or greater than +1200, or if only hour digits are present, less than -12 or greater than +12. It will return NULL for offsets outside that range. These limits do not meet historical and modern use cases: * Present day exceeds the +1200 limit: - Pacific/Auckland (New Zealand) summer time is +1300. - Pacific/Kiritimati (Christmas Island) is +1400. - Pacific/Apia (Samoa) summer time is +1400. * Historical offsets exceeded +1500/-1500. * POSIX supports -2459 to +2559. * Offsets up to +/-9959 may occasionally be useful. * Paul Eggert's notes provide additional detail: - https://sourceware.org/ml/libc-alpha/2014-12/msg00068.html - https://sourceware.org/ml/libc-alpha/2014-12/msg00072.html 3) tst-strptime2, part of the 'make check' test suite, does not test for the above problems. Corrective actions: 1) In time/strptime_l.c, calculate the offset from the hour and minute portions directly, without the rounding errors introduced by decimal time. 2) Remove the +/-1200 range limit, permitting strptime to parse offsets from -9959 through +9959. 3) Add zone offset values to time/tst-strptime2.c. * Test minutes evenly divisible by three (+1157) and not evenly divisible by three (+1158 and +1159). * Test offsets near the old and new range limits (-1201, -1330, -2459, -2500, -99, -9959, +1201, +1330, +1400, +1401, +2559, +2600, +99, and +9959) The revised strptime passes all old and new tst-strptime2 tests. 2015-08-14 James Perkins [BZ #16141] * time/strptime_l.c (__strptime_internal): Fix %z minutes calculation, removing incorrect decimal time rounding, so that all minute values result in a valid seconds value. * time/strptime_l.c (__strptime_internal): Extend %z time zone offset range limits to UTC-99:59 through UTC+99:59 to parse current and historical use cases. * time/tst-strptime2.c (tests): Modify and add tests for the strptime %z input field descriptor, specifically conversion of minutes to seconds and validating an offset range of -9959 to +9959. Signed-off-by: James Perkins --- time/strptime_l.c | 12 +++--------- time/tst-strptime2.c | 21 +++++++++++++++++++-- 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/time/strptime_l.c b/time/strptime_l.c index 5640cce..4203ad8 100644 --- a/time/strptime_l.c +++ b/time/strptime_l.c @@ -770,16 +770,10 @@ __strptime_internal (rp, fmt, tmp, statep LOCALE_PARAM) else if (n != 4) /* Only two or four digits recognized. */ return NULL; - else - { - /* We have to convert the minutes into decimal. */ - if (val % 100 >= 60) - return NULL; - val = (val / 100) * 100 + ((val % 100) * 50) / 30; - } - if (val > 1200) + else if (val % 100 >= 60) + /* Minutes valid range is 0 through 59. */ return NULL; - tm->tm_gmtoff = (val * 3600) / 100; + tm->tm_gmtoff = (val / 100) * 3600 + (val % 100) * 60; if (neg) tm->tm_gmtoff = -tm->tm_gmtoff; } diff --git a/time/tst-strptime2.c b/time/tst-strptime2.c index a08e6d7..4db8321 100644 --- a/time/tst-strptime2.c +++ b/time/tst-strptime2.c @@ -17,8 +17,25 @@ static const struct { "1113472456 -1030", -37800 }, { "1113472456 +0030", 1800 }, { "1113472456 -0030", -1800 }, - { "1113472456 -1330", LONG_MAX }, - { "1113472456 +1330", LONG_MAX }, + { "1113472456 +1157", 43020 }, + { "1113472456 +1158", 43080 }, + { "1113472456 +1159", 43140 }, + { "1113472456 +1200", 43200 }, + { "1113472456 -1200", -43200 }, + { "1113472456 +1201", 43260 }, + { "1113472456 -1201", -43260 }, + { "1113472456 +1330", 48600 }, + { "1113472456 -1330", -48600 }, + { "1113472456 +1400", 50400 }, + { "1113472456 +1401", 50460 }, + { "1113472456 -2459", -89940 }, + { "1113472456 -2500", -90000 }, + { "1113472456 +2559", 93540 }, + { "1113472456 +2600", 93600 }, + { "1113472456 -99", -356400 }, + { "1113472456 +99", 356400 }, + { "1113472456 -9959", -359940 }, + { "1113472456 +9959", 359940 }, { "1113472456 -1060", LONG_MAX }, { "1113472456 +1060", LONG_MAX }, { "1113472456 1030", LONG_MAX },