Re: KPixmapCache in kdelibs4support uses QDateTime in mmap'ed struct
On Tuesday 03 November 2015 11:06:00 Alex Merry wrote: > On 2015-11-03 00:56, Michael Pyne wrote: > > On Tue, November 3, 2015 00:40:58 Albert Astals Cid wrote: > >> Someone went a bit too far in the "port away from time_t to QDateTime" > >> and > >> changed the timestamp member of the KPixmapCacheIndexHeader struct. > >> > >> According to mpyne and thiago this is a big no-no. > > > > The reason it's a big no-no is because KPixmapCacheIndexHeader is meant > > to be > > held and used from mmap()'d shared memory, which generally requires the > > use of > > plain old data (POD) types only, to avoid inadvertent constructor or > > destructor calls once the memory is freed. > > > > QDateTime will (and is) leak applications to crash so I'd +1 the > > recommendation to change back to time_t (or at least some kind of > > integral > > type), and the type is internal-only so there's no API concern. Just > > don't > > forget to bump the defined KPIXMAPCACHE_VERSION so that old caches are > > flushed > > ;). > > Also, in practice, I believe time_t is everywhere we care about - we > just have to take a little care about what API calls we use. Right. On the other hand, it will break in 2039 on 32 bit machines, so nowadays I use "qint64 msecs" and QDateTime::fromMsecsSinceEpoch instead. But anyway, we can hope kdelibs4support won't exist anymore in 2039 ;) -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5 ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: KPixmapCache in kdelibs4support uses QDateTime in mmap'ed struct
On 2015-11-03 00:56, Michael Pyne wrote: On Tue, November 3, 2015 00:40:58 Albert Astals Cid wrote: Someone went a bit too far in the "port away from time_t to QDateTime" and changed the timestamp member of the KPixmapCacheIndexHeader struct. According to mpyne and thiago this is a big no-no. The reason it's a big no-no is because KPixmapCacheIndexHeader is meant to be held and used from mmap()'d shared memory, which generally requires the use of plain old data (POD) types only, to avoid inadvertent constructor or destructor calls once the memory is freed. QDateTime will (and is) leak applications to crash so I'd +1 the recommendation to change back to time_t (or at least some kind of integral type), and the type is internal-only so there's no API concern. Just don't forget to bump the defined KPIXMAPCACHE_VERSION so that old caches are flushed ;). Also, in practice, I believe time_t is everywhere we care about - we just have to take a little care about what API calls we use. Alex ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
KPixmapCache in kdelibs4support uses QDateTime in mmap'ed struct
Someone went a bit too far in the "port away from time_t to QDateTime" and changed the timestamp member of the KPixmapCacheIndexHeader struct. According to mpyne and thiago this is a big no-no. I'm suggesting bringing the code back to using time_t so it still works, this may be a bit less portable, but it's kdelibs4support support anyway, i think we can compromise portability here for "it works" :D Opinions? Cheers, Albert ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: KPixmapCache in kdelibs4support uses QDateTime in mmap'ed struct
On Tue, November 3, 2015 00:40:58 Albert Astals Cid wrote: > Someone went a bit too far in the "port away from time_t to QDateTime" and > changed the timestamp member of the KPixmapCacheIndexHeader struct. > > According to mpyne and thiago this is a big no-no. The reason it's a big no-no is because KPixmapCacheIndexHeader is meant to be held and used from mmap()'d shared memory, which generally requires the use of plain old data (POD) types only, to avoid inadvertent constructor or destructor calls once the memory is freed. QDateTime will (and is) leak applications to crash so I'd +1 the recommendation to change back to time_t (or at least some kind of integral type), and the type is internal-only so there's no API concern. Just don't forget to bump the defined KPIXMAPCACHE_VERSION so that old caches are flushed ;). Regards, - Michael Pyne ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
