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

[Michal Hlavinka]

David Jarvie napsal(a):
> On Wednesday 16 July 2008 14:44, Michal Hlaivnka wrote:
>   
>> 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)
>>     
>
> Actually, I just checked and KDateTime::toTime_t() returns a uint the same
> as QDateTime::toTime_t(). See the apidox (which does correspond to the
> source code :) ). So the return statement should presumably be
>
>     return uint(res) != uint(-1) ? res : 0;
>
> Of course, if time_t is signed (as it is on some systems), the return
> value won't be correct if the m.s. bit is set.
You are right, KDateTime returns uint, my mistake. I don't know where 
I've found it's time_t...
So the right fix would be

from this:
    time_t res = KDateTime::fromString(str, KDateTime::ISODate).toTime_t();
    return res != -1 ? res : 0;

to this:
    uint res = KDateTime::fromString(str, KDateTime::ISODate).toTime_t();
    return res != uint(-1) ? time_t(res) : 0;


...if it's correct to assume QDateTime() (an invalid value) will be 
always converted to uint(-1). I didn't find it anywhere, as I've writen 
already.

_______________________________________________
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/