[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-core-devel
Subject:    Re: [PATCH] kdecore: Fix a bug in KDateTime utc offset string parsing.
From:       David Jarvie <djarvie () kde ! org>
Date:       2012-10-08 22:52:44
Message-ID: 201210082352.45023.djarvie () kde ! org
[Download RAW message or body]

On Sunday 07 October 2012 21:23:56 Jon Severinsson wrote:
> The sign of the UTC offset was ignored, and an offset of -0500 (New York) would be \
> treated at +0500 (Pakistan). This commit also adds a unit test for UTC offset \
>                 parsing and comparasion.
> ---
> Hi
> 
> When mucking around in the frameworks branch of kdelibs I found a bug in the
> KDateTime string parsing, which appears to be present in master as well as
> every branch from 4.0 to 4.10. I have, however, only run the updated unittest
> using the frameworks branch and Qt5, so someone else should probably test
> on 4.9, 4.10 and/or master before committing it.
> 
> Best Regards
> Jon Severinsson
> 
> kdecore/date/kdatetime.cpp      |    2 +-
> kdecore/tests/kdatetimetest.cpp |    7 +++++++
> 2 filer ändrade, 8 tillägg(+), 1 borttagning(-)
> 
> diff --git a/kdecore/date/kdatetime.cpp b/kdecore/date/kdatetime.cpp
> index d4f63ff..df1fc3d 100644
> --- a/kdecore/date/kdatetime.cpp
> +++ b/kdecore/date/kdatetime.cpp
> @@ -2962,7 +2962,7 @@ bool getUTCOffset(const QString &string, int &offset, bool \
> colon, int &result) tzmin += tzhour * 60;
> if (result != NO_NUMBER  &&  result != tzmin)
> return false;
> -    result = tzmin;
> +    result = sign * tzmin;
> return true;
> }
> 
> diff --git a/kdecore/tests/kdatetimetest.cpp b/kdecore/tests/kdatetimetest.cpp
> index e79e9f2..812abc5 100644
> --- a/kdecore/tests/kdatetimetest.cpp
> +++ b/kdecore/tests/kdatetimetest.cpp
> @@ -3807,6 +3807,13 @@ void KDateTimeTest::strings_format()
> QVERIFY(!dt.isValid());    // too early
> QVERIFY(dt.outOfRange());
> 
> +    dtutc = KDateTime::fromString(QLatin1String("2000-01-01T00:00:00.000+0000"), \
> QLatin1String("%Y-%m-%dT%H:%M%:S%:s%z")); +    QVERIFY(dtutc.isValid());
> +    dt =    KDateTime::fromString(QLatin1String("2000-01-01T05:00:00.000+0500"), \
> QLatin1String("%Y-%m-%dT%H:%M%:S%:s%z")); +    QVERIFY(dt.isValid());
> +    QVERIFY(dtutc == dt);
> +    dt =    KDateTime::fromString(QLatin1String("1999-12-31T20:30:00.000-0330"), \
> QLatin1String("%Y-%m-%dT%H:%M%:S%:s%z")); +    QVERIFY(dtutc == dt);
> 
> // Restore the original local time zone
> if (originalZone.isEmpty())
> 

The patches look good to commit. However, can you please move the new tests in \
kdatetimetest.cpp to start at line 3740, before the existing group of tests for UTC \
offsets. This will keep related tests grouped together.

-- 
David Jarvie.
KDE developer.
KAlarm author -- http://www.astrojar.org.uk/kalarm


[prev in list] [next in list] [prev in thread] [next in thread]