Review Request: fix data leak in dataengines
John Layt
johnlayt at googlemail.com
Wed Jun 9 10:45:19 CEST 2010
> On 2010-06-09 01:06:51, Aaron Seigo wrote:
> > John's commit should just be reverted and the calendar dataengine changed properly. this penalizes the common case and adds more complexity that is uneeded, and the calendar dataengine really ought to list all holidays for the same day in one key/value pair. using QVariant for the values in DataEngines allows us to use lists and other such "fancy" things, and they work very well even in the automatic scripting bridges.
>
> Beat Wolf wrote:
> I'm no expert for the dataengines. so i let the experts sort that out ;) at least i found the problem :) or i can just revert the other commit.
>
> Beat Wolf wrote:
> never mind, didn't see the comment in the other patch.
OK, I'll revert my commit and change the data engine to wrap the data in another level of container.
- John
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/4261/#review6047
-----------------------------------------------------------
On 2010-06-08 20:30:26, Beat Wolf wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/4261/
> -----------------------------------------------------------
>
> (Updated 2010-06-08 20:30:26)
>
>
> Review request for Plasma.
>
>
> Summary
> -------
>
> fix the data "leak" introduced by http://reviewboard.kde.org/r/4235/
> The fix is to use insertMulti if multiple values with the same key are added at once.
>
>
> Diffs
> -----
>
> trunk/KDE/kdelibs/plasma/datacontainer.h 1136034
> trunk/KDE/kdelibs/plasma/datacontainer.cpp 1135137
> trunk/KDE/kdelibs/plasma/dataengine.cpp 1135137
>
> Diff: http://reviewboard.kde.org/r/4261/diff
>
>
> Testing
> -------
>
> Different plasmoids using dataengines
>
>
> Thanks,
>
> Beat
>
>
More information about the Plasma-devel
mailing list