my own kdatetime patches: toString with InvalidOffset; operator==

[Jon Severinsson]

torsdagen den 25 oktober 2012 19:25:29 skrev du:
> On Thursday 25 October 2012 16:30:20 Jon Severinsson wrote:
> > I triggered this in Qt5 too, and I figured out the underlying reason for
> > it to  happen. If, when KSystemTimeZonesPrivate::setLocalZone() tries to
> > figure out what time zone to use for KSystemTimeZones::local(), the
> > kconfig "ktimezonedrc"/ "TimeZones"/"LocalZone" time zone does not exist
> > in the kconfig  "ktimezonedrc"/"TimeZones"/"ZoneinfoDir" directory, it
> > will create a "valid" KTzTimeZone with garbage data.
> > 
> > This happens because KDateTimeTest::cleanupTestCase() successfully
> > removes the  temporary zoneinfo dir it creates, but used the wrong path
> > when trying to remove the ktimezonedrc kconfig file.
> > 
> > Patches for both KDateTimeTest::cleanupTestCase() and
> > KSystemTimeZonesPrivate::setLocalZone() will be uploaded to review board
> > shortly.
> 
> Well done, I was really wondering what triggered the issue. This is why I
> didn't commit my fixes yet -- I wanted to write a unittest for them, but
> didn't know what to write in the unittest (all my attempts failed).
> 
> Would you be interested in adding a corresponding unittest to reproduceably
> trigger the above issue, and get into an "invalid timezone in a valid
> KDateTime"?

What is it you want me to test?
With my patches there is no way for KSystemTimeZones::local() to create an
valid but garbage time zone, and I will add test to verify that, but that
don't involve KDateTime.

Getting a valid KDateTime with a "valid but garbage" timezone spec is
currently as simple as this:
KTzfileTimeZoneSource source("/invalid/tzinfo/dir");
KDateTime date(QDate(2000,1,1),QTime(0,0,0),KTzfileTimeZone(&source,"Europe/London"));
But what to do whith that?
I could fix KDateTime::Spec::setType(KTimeZone) (and thus the implicit
constructor) to detect "valid but garbage" as an KDateTime::Invalid time zone
spec and verify that the KDateTime is considered invalid. Is that what you
meant or did you have something else in mind?

> I also wonder if isValid() shouldn't return false, instead, in this
> particular case (invalid timezone offset). That would make more sense I
> suppose, and then my patch to toString() is unnecessary since the method
> starts with "return empty if the datetime is invalid" (which doesn't make
> debugging any easier, though...)

This would change the KTimeZone::isValid() from a simple QString::isEmpty()
call into a quite complex operation, possible including reading a file from
disk and parsing it's content, I don't think that is advisable. Not to mention
the infinite loop it would create :(

( This because KTimeZone uses lazy initialization, and don't call
KTimeZoneSource::parse() until the first the call to KTimeZone::data(true),
which is currently used by all getters, but not by isValid(). And as
KTimeZone::data(bool) starts of by calling "if (!isValid()) return 0;", using
any getters in isValid() would be a bad idea... )

Regards
Jon Severinsson
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20121025/b4a688ec/attachment.sig>