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<Field>. 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).

So I think making UDSEntry a Hotel California for fields ("you can checkout 
but you can never leave") as you did, is just fine.

> (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.

-- 
David Faure, faure@kde.org, http://www.davidfaure.fr
Working on KDE, in particular KDE Frameworks 5