Avoid creating duplicate property entries in the cache
ClosedPublic

Authored by bruns on Apr 11 2018, 10:31 PM.

Details

Summary

Properties are associated with a specific interface, although the Solid
UDisks2 backend merges properties from all interfaces into a single
namespace.
Fortunately most properties have either unique names accross interfaces,
or are consistent (e.g. "Size" in org.fd.UDisks2.{Block,Partition}), thus
this poses no problem in practice.
QMap<>::unite(other) behaves like QMap<>::insertMulti(item), i.e. the
map may contain multiple values per key, while QMap<>::insert(item)
updates the value for existing keys.

Test Plan

make
solid-hardware list details

Diff Detail

Repository
R245 Solid
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
bruns created this revision.Apr 11 2018, 10:31 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptApr 11 2018, 10:31 PM
bruns requested review of this revision.Apr 11 2018, 10:31 PM
bruns updated this revision to Diff 31929.Apr 11 2018, 10:35 PM

update, one line was missing

broulik accepted this revision.Apr 12 2018, 12:39 PM
broulik added a subscriber: broulik.

Wouldn't make a big difference, would it?

If there are multiple items for key in the map, the value of the most recently inserted one is returned.

It's not like we actually check for the property we're overriding with having a saner value.

This revision is now accepted and ready to land.Apr 12 2018, 12:39 PM
bruns added a comment.Apr 12 2018, 6:05 PM

Wouldn't make a big difference, would it?

If there are multiple items for key in the map, the value of the most recently inserted one is returned.

It's not like we actually check for the property we're overriding with having a saner value.

As allproperties is currently called directly from Manager::updateBackend (which is IMHO wrong and needs some cleanup), we end up with values from multiple GetAll calls for the same interface. Without the "cleanup after InterfaceRemoved" (D12126), the propertyCache for e.g. Loop interfaces contained each key 4 times, after D12126 its only twice without this one.

Fortunately QMap guarantees to use the last inserted value when there are multiple values for a key [1], so this is likely only annoying when debugging and a slight waste of memory/cpu time.
[1] http://doc.qt.io/qt-5/qmap.html#find

This revision was automatically updated to reflect the committed changes.