Hi, 2013/9/30 David Faure: > On Thursday 26 September 2013 22:48:28 Frank Reininghaus wrote: >> (a) With my patch, UDSEntry::remove(uint field) removes the "field" >> from the QHash, but it neither frees the memory held by the >> corresponding QString, nor does it remove the entry from the >> QVector. I'm not sure how often it happens that fields are >> removed and re-added to a UDSEntry, but if that is done repeatedly, >> then the memory usage will grow indefinitely, which is not good. >> Erasing the "Field" from the QVector every time might not be optimal >> either because it makes "remove" a O(N) operation. Maybe one could >> check if at least half the entries have been deleted and reorganize >> the internal structure if that is the case. I'll look into that. > > I wouldn't worry too much about it. AFAICS remove is never called. > (I removed remove - haha - locally and all of kio, kfile, kioslaves, libkonq > and dolphin compiled just fine). Maybe remove could be removed from the frameworks branch then ;-) >> (b) Maybe I should add a unit test that stores multiple UDSEntries to >> a stream, changes the order of the fields every time, re-loads them, >> and verifies that everything is OK. Just to make sure that the code >> really works even if not all UDSEntries have the same format inside >> the stream. > > Sounds good. Don't use random though, to "change the order of the fields every > time", since that would lead to errors that would be very hard to reproduce. Did that, fixed a bug that was uncovered by the tests, and changed a few more little things: https://git.reviewboard.kde.org/r/113355/ Cheers, Frank