----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116784/#review52897 -----------------------------------------------------------
The handling of return values from KDateTime::toTime_t() in the existing kio_http code is not correct, because the return value's type is implicitly cast to other types before being checked. For example, in one place it is cast to qint64, which will result in a value of 0xffffffff instead of 0xffffffffffffffff (= -1). This type of error will mask the fact that the error value is being returned. Instead of changing the calling code to detect invalid dates using other methods, it should be fixed to properly cast the uint value returned from KDateTime::toTime_t(). For types other than int, it needs to specifically check for uint(-1) and set the cast value to -1 in that case. For example: uint t = KDateTime::toTime_t(...); // Set the qint64 to be -1 if an error occurred: qint64 result = (t == uint(-1)) ? -1 : t; Note: KDateTime::toTime_t() is *supposed* to return uint(-1) to indicate an error. If it doesn't always do this, *it* should be fixed instead of changing code elsewhere, since kio_http is unlikely to be the only module that will have trouble if that is happening. - David Jarvie On March 13, 2014, 12:49 p.m., Dawit Alemayehu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/116784/ > ----------------------------------------------------------- > > (Updated March 13, 2014, 12:49 p.m.) > > > Review request for kdelibs, Andreas Hartmetz and David Faure. > > > Repository: kdelibs > > > Description > ------- > > The attached patch does the following: > > - It corrects a mistake in assumption that KDateTime.toTime_t() will return > -1 for invalidate dates. It does not. The result is an overflow which is > interpreted in kio_http as a timestamp in the distant future which obviously > is wrong. See https://bugs.kde.org/show_bug.cgi?id=331774 for example. This > assumption also affects the timestamp variables used for cache management. > > - It converts cache management timestamp variables to 64 bits so they can > accomodates dates beyond Feb 7, 2106. > > > Diffs > ----- > > kioslave/http/http.h dd85622 > kioslave/http/http.cpp e4f1eba > > Diff: https://git.reviewboard.kde.org/r/116784/diff/ > > > Testing > ------- > > > Thanks, > > Dawit Alemayehu > >
