[Kde-pim] parseRFCDate/parseISODate - wrong bad date handling (PATCH)

[Michal Hlaivnka]

Allen Winter napsal(a):
> On Wednesday 16 July 2008 07:48:32 David Jarvie wrote:
>   
>> On Wednesday 16 July 2008 12:27, masp01 at centrum.cz wrote:
>>     
>>> Hi,
>>>
>>> I've found bug in kdepimlibs/syndication/tools.cpp (bugs.kde.org - 166721)
>>>
>>> description: if string contains date in wrong format, parseRFCDate (or
>>> parseISODate) returns 2^32-1
>>>
>>>  result: some apps (akregator) don't know about error, because returned
>>> value uint32_t(-1) is not the expected time_t(-1) ( = uint64_t(-1) at
>>> least on my computer). This leads to wrong dates being displayed
>>>
>>>  Problem is located between Qt 4 and KDateTime.
>>>
>>>  KDateTime::toTime_t() returns time_t (64bit at least on my computer)
>>>  but
>>>  QDateTime::toTime_t() returns uint (32bit)
>>>
>>>  QDateTime's -1 is converted to 4294967295, but error is tested for -1
>>>
>>>  solution: QDateTime's return type should be time_t, not uint
>>>
>>>  fix: Don't test for -1 but do QDateTime/KDateTime::isValid() tests
>>>
>>>
>>> patch to fix this is attached
>>>
>>>
>>>  time_t parseISODate(const QString& str)
>>>  {
>>> -    time_t res = KDateTime::fromString(str,
>>>       
>> KDateTime::ISODate).toTime_t();
>>     
>>> -    return res != -1 ? res : 0;
>>> +    const KDateTime kdt = KDateTime::fromString(str, KDateTime::ISODate);
>>> +    return kdt.isValid() ? kdt.toTime_t() : 0;
>>>  }
>>>
>>>  time_t parseRFCDate(const QString& str)
>>>  {
>>> -    time_t res = KDateTime::fromString(str,
>>>       
>> KDateTime::RFCDate).toTime_t();
>>     
>>> -    return res != -1 ? res : 0;
>>> +    const KDateTime kdt = KDateTime::fromString(str, KDateTime::RFCDate);
>>> +    return kdt.isValid() ? kdt.toTime_t() : 0;
>>>  }
>>>
>>>  time_t parseDate(const QString& str, DateFormat hint)
>>>       
>> How about casting the -1 to time_t in the return statements instead:
>>
>>    return res != (time_t)(-1) ? res : 0;
>>     
It won't work. res is time_t( uint(-1) ), so '-1' in the statement is 
already time_t(-1)
> I'll take care of committing this fix.
> These methods should use uint's instead of time_t's, but I don't think we can
> change that now, due to the API freeze.  drat.
>   
IMHO Qt  is the one who is wrong here. Why is uint on 64bit computer 
with x86_64 version of Qt only 32bit? And why result of 
QDateTime::toTime_t() isn't time_t ???

PS: I've not been looking too hard but I can't se where is written that 
invalid KDateTime value represented by just KDateTime() or QDateTime() 
must be equal to -1. KDateTime::toTime_t() uses QDateTime::toTime_t() 
and there is only this:
"uint QDateTime::toTime_t () const

Returns the datetime as the number of seconds that have passed since 
1970-01-01T00:00:00, Coordinated Universal Time (Qt::UTC).

On systems that do not support timezones, this function will behave as 
if local time were Qt::UTC."

So no information about return value on error (invalid datetime) at all.

_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/