[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-core-devel
Subject: Re: Review Request 127866: Oxygen: Fix QCache usage
From: Michael Pyne <mpyne () kde ! org>
Date: 2016-05-21 21:26:37
Message-ID: 20160521212637.15344.32614 () mimi ! kde ! org
[Download RAW message or body]
--===============0783103058877689815==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
> On May 15, 2016, 8:51 a.m., Hugo Pereira Da Costa wrote:
> > To be honest, I am quite puzzle by this whole thing.
> > Now, every insertion in the cache requires at least two searches in there and (in \
> > many case) at least one copy constructor being called. This is quite expansive \
> > ... (even though this happens only if the object is not found in the cache).
> > Also: not sure I understand what issue we are trying to fix and how: why is it \
> > that if the object inserted in the cache is immediately deleted, just retrying an \
> > indefinite amount of time will "fix" the issue. Are we not just transforming a \
> > crash into a freeze (infinite loop) ?
> > The Qt documentation is very vague about cases where the object is deleted \
> > immediately, and the only case it mentions is: " In particular, if cost is \
> > greater than maxCost(), the object will be deleted immediately." Well, in such \
> > cases (that should not appear here), the infinite loop will not help. Right ? \
> > Since we have no idea on how "predictible" the other deletion cases are, I don't \
> > think the fix is a good fix.
> > Does that mean that we should change the code in order to use references rather \
> > than pointer everywhere ? (as you did in the first patch on this topic) ?
> > Or get rid of using QCache (because this absence of guarantee at the insertion \
> > stage is too much of a pain to handle) ?
> > Or just commit and wait for bug reports about freezes ? (but with a happy \
> > coverty) ?
>
> Michael Pyne wrote:
> For the most part the requirements are determined by the current return type within \
> the code. If we return a pointer then currently it has to have come directly out of \
> the QCache. Since QCache is assumed to be the owner, the calling code won't delete \
> the pointer ; but if the caller won't delete the pointer then we'll have a memory \
> leak if we return a pointer to something that had been new'd instead.
> References (i.e. QColor&) are a similar issue; it's safe enough to return a \
> reference to something held within the QCache, but we can't return a reference to a \
> local variable since that reference will invalidate as soon as we return from the \
> function. Of course a reference to a cached QColor may *also* invalidate with the \
> next call to an insert method of that cache, but that's a separate story.
> It is unfortunate that the Qt docs are vague about this, since if the **only** \
> thing we had to worry about was cost being >maxCost(), we could pretty much just \
> mark 'ignore' for all the Coverity issues associated with this (and I'd be fine \
> doing that). The docs do kind of hint at that but don't make it clear if is the \
> only way that an entry would be deleted immediately.
> I think you're right that a loop is not a good idea... I was figuring that \
> eventually QCache would remove enough other items to make it work but then I \
> suppose QCache::insert() would have done that with the very first attempt.
> As far as other options, I would definitely recommend against QCache for the \
> QColors: I'd say just hold onto specific QColors directly (perhaps in a QHash) and, \
> if possible, return them as values instead of references.
> I'm not sure if we could get away with the same for TileSets, but if so it would \
> again make things easier. If we can't we could look into making TileSet an \
> implicitly shared class so that we can return it by value cheaply.
> I wouldn't recommend a commit only to make Coverity happy. I've marked other \
> reports as "False Positive" and even "That's a bug, but we're ignoring it" before. \
> But it does seem to me that if a crash *is* possible (especially in underlying \
> library code) we should do something to avoid it.
> Either way I'll see if I can work up a revised patch but I'd still appreciate \
> advice on what's workable or not within Oxygen.
> Mark Gaiser wrote:
> Disclaimer: i'm not an oxygen dev! I just read the patch and want to share my \
> opinion :)
> I'm also quite puzzeled with this change..
>
> A cache is "usually" being used to store the result of a "heavy" operation and use \
> that result the next time to speed things up. That principle sounds great to me and \
> should be used in places if needed. A better approach would be to speed up the slow \
> operation to make caching simply not needed, but that's a different story.
> The changes you've made now make QCache usage heavier and most certainly much more \
> difficult to follow because of the added while loops to keep an hypothetical case \
> from happening. If QCache requires measures like this then perhaps we should not be \
> using QCache at all.
> I understand you're trying to do your best, Michael Pyne, but a patch like this \
> feels like heading in the wrong direction to me..
> I don't know if it's usefull, but there is als QPixmapCache [1] (which uses a \
> QCache internally, but seems to behave as a normal sane LRU cache [2])
> Cheers,
> Mark
>
> [1] http://doc.qt.io/qt-5/qpixmapcache.html
> [2] https://en.wikipedia.org/wiki/Cache_algorithms#LRU
>
> Frank Reininghaus wrote:
> > If we can't we could look into making TileSet an implicitly shared class so that \
> > we can return it by value cheaply.
>
> Or alternatively, one could return not raw pointers, but shared pointers (like \
> `std::shared_ptr<TileSet>` or `QSharedPtr<TileSet>`). Then it would be 100% clear \
> that the caller, who gets the pointer from the cache, is now a co-owner of the \
> object and can be sure that it won't be deleted as long as they use it.
> Having a `QCache<QSharedPointer<TileSet>>` would then store pointers to shared \
> pointers, unfortunately, but if we made `TileSet` implicitly shared, then it would \
> essentially be the same, even if one of the pointers would be hidden inside the \
> implementation.
> I have the feeling that we need a replacement for `QCache` that works with shared \
> pointers :-) `QCache` was probably designed in a time when there were only raw \
> pointers, and no clear ways to express ownership (which is one of the reasons why \
> tools like Coverity are needed).
> One could argue that a cache that deals with shared pointers adds a small overhead \
> (due to the reference counting), but since a cache is supposed to be used with \
> objects that are expensive to create, this should not matter at all.
It would probably not be too difficult to make a simple cache that held shared \
pointers directly (to avoid pointers to shared pointers). If we embed an assumption \
that it's a FIFO then we already have QQueue<T> and it's simple. Even QCache itself \
is not very big, in fact I was able to verify directly that there are no other ways \
to delete a pointer upon ::insert other than exceeding the maxCost, which is already \
an error. It would be nice if Qt would document that this is the *only* failure mode \
(instead of that this is a *particular* failure mode), in that case we could just add \
an assert that the cost going in is less than maxCost and I /think/ that should \
satisfy Coverity (which is pretty smart in its static analysis).
However I've also started on tweaking the patch to return QColor directly (instead of \
a reference) which simplifies much of this right off the bat, and leaves TileSet \
alone as the ugly case. I do think that we need to do something different in here \
though (probably shared pointers!)... right now I'm not sure if the cache is really \
needed to reduce memory usage or if it's simply a way to share common data without \
duplicating it in memory everywhere.
- Michael
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127866/#review95480
-----------------------------------------------------------
On May 8, 2016, 5:03 a.m., Michael Pyne wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127866/
> -----------------------------------------------------------
>
> (Updated May 8, 2016, 5:03 a.m.)
>
>
> Review request for kde-workspace and Hugo Pereira Da Costa.
>
>
> Repository: oxygen
>
>
> Description
> -------
>
> This should mostly complete the QCache fixes I kicked off in a previous RR, 127837. \
> Hugo noted there were many other similar usages, and boy he wasn't kidding! ;) The \
> long story short is that these usages can theoretically cause use-after-free \
> behavior (which can lead to crashes and even undefined behavior if the compiler \
> ever gets smart).
> *NOTE* It is -much- easier to review if you download the diff to your git \
> repository for oxygen and then run "git diff -b" to ignore whitespace changes, \
> particularly for the QPixmap changes.
> For QPixmaps we return values instead of pointers, so we simply make a separate \
> copy to be cached when we do insert. For QColor we return references to values so \
> we *must* return pointers, and those have to be owned by a QCache to avoid \
> memleaks. So I added a helper function to loop until the cache accepts the new \
> entry. TileSets are a similar concern, except those have manual loops since I was \
> uncertain about whether TileSet's copy constructor was the best idea or not.
> This fixes a ton of Coverity issues (59717 - 259733, 259739, 259742 - 259752, \
> 1336154, 1336155) and might be associated with Qt bug 38142 and KDE bug 219055 \
> (which doesn't actually appear to be a dupe of a different bug to me...).
>
> Diffs
> -----
>
> kstyle/oxygenstylehelper.cpp 612ba37
> liboxygen/oxygenhelper.h a6453a0
> liboxygen/oxygenhelper.cpp 4843604
> liboxygen/oxygenshadowcache.cpp 907e586
>
> Diff: https://git.reviewboard.kde.org/r/127866/diff/
>
>
> Testing
> -------
>
> Compiled without warnings, installed and ran `oxygen-demo5 -style oxygen`. Used the \
> GUI Benchmark feature to automatically cycle through all the listed features -- no \
> crashes or obvious rendering errors.
>
> Thanks,
>
> Michael Pyne
>
>
--===============0783103058877689815==
MIME-Version: 1.0
Content-Type: text/html; charset="utf-8"
Content-Transfer-Encoding: 7bit
<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 \
solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;"> \
<tr> <td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/127866/">https://git.reviewboard.kde.org/r/127866/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <p style="margin-top: 0;">On May 15th, 2016, 8:51 a.m. UTC, <b>Hugo Pereira \
Da Costa</b> wrote:</p> <blockquote style="margin-left: 1em; border-left: 2px solid \
#d0d0d0; padding-left: 10px;"> <pre style="white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">To be honest, I am quite puzzle by this whole thing. \
Now, every insertion in the cache requires at least two searches in there and (in \
many case) at least one copy constructor being called. This is quite expansive ... \
(even though this happens only if the object is not found in the cache).</p> <p \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">Also: not sure I understand what issue we are trying to fix and how: why is \
it that if the object inserted in the cache is immediately deleted, just retrying an \
indefinite amount of time will "fix" the issue. Are we not just transforming a crash \
into a freeze (infinite loop) ? </p> <p style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: inherit;">The Qt documentation is \
very vague about cases where the object is deleted immediately, and the only case it \
mentions is: " In particular, if cost is greater than maxCost(), the object will be \
deleted immediately." Well, in such cases (that should not appear here), the infinite \
loop will not help. Right ? Since we have no idea on how "predictible" the other \
deletion cases are, I don't think the fix is a good fix. </p> <p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Does \
that mean that we should change the code in order to use references rather than \
pointer everywhere ? (as you did in the first patch on this topic) ? </p> <p \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">Or get rid of using QCache (because this absence of guarantee at the \
insertion stage is too much of a pain to handle) ? </p> <p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Or \
just commit and wait for bug reports about freezes ? (but with a happy coverty) \
?</p></pre> </blockquote>
<p>On May 15th, 2016, 2:21 p.m. UTC, <b>Michael Pyne</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">For the most part the \
requirements are determined by the current return type within the code. If we return \
a pointer then currently it has to have come directly out of the QCache. Since QCache \
is assumed to be the owner, the calling code won't delete the pointer ; but if \
the caller won't delete the pointer then we'll have a memory leak if we \
return a pointer to something that had been new'd instead.
References (i.e. QColor&) are a similar issue; it's safe enough to return a \
reference to something held within the QCache, but we can't return a reference to \
a local variable since that reference will invalidate as soon as we return from the \
function. Of course a reference to a cached QColor may *also* invalidate with the \
next call to an insert method of that cache, but that's a separate story.
It is unfortunate that the Qt docs are vague about this, since if the **only** thing \
we had to worry about was cost being >maxCost(), we could pretty much just mark \
'ignore' for all the Coverity issues associated with this (and I'd be \
fine doing that). The docs do kind of hint at that but don't make it clear if is \
the only way that an entry would be deleted immediately.
I think you're right that a loop is not a good idea... I was figuring that \
eventually QCache would remove enough other items to make it work but then I suppose \
QCache::insert() would have done that with the very first attempt.
As far as other options, I would definitely recommend against QCache for the QColors: \
I'd say just hold onto specific QColors directly (perhaps in a QHash) and, if \
possible, return them as values instead of references.
I'm not sure if we could get away with the same for TileSets, but if so it would \
again make things easier. If we can't we could look into making TileSet an \
implicitly shared class so that we can return it by value cheaply.
I wouldn't recommend a commit only to make Coverity happy. I've marked other \
reports as "False Positive" and even "That's a bug, but we're \
ignoring it" before. But it does seem to me that if a crash *is* possible \
(especially in underlying library code) we should do something to avoid it.
Either way I'll see if I can work up a revised patch but I'd still appreciate \
advice on what's workable or not within Oxygen.</pre> </blockquote>
<p>On May 15th, 2016, 8:57 p.m. UTC, <b>Mark Gaiser</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">Disclaimer: i'm not an oxygen dev! I just read the patch and want to share \
my opinion :)</p> <p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">I'm also quite puzzeled with this change..</p> <p \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">A cache is "usually" being used to store the result of a "heavy" operation \
and use that result the next time to speed things up. That principle sounds great to \
me and should be used in places if needed. A better approach would be to speed up the \
slow operation to make caching simply not needed, but that's a different story.</p> \
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">The changes you've made now make QCache usage heavier \
and most certainly much more difficult to follow because of the added while loops to \
keep an hypothetical case from happening. If QCache requires measures like this then \
perhaps we should not be using QCache at all.</p> <p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I \
understand you're trying to do your best, Michael Pyne, but a patch like this feels \
like heading in the wrong direction to me..</p> <p style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: inherit;">I don't know if it's \
usefull, but there is als QPixmapCache [1] (which uses a QCache internally, but seems \
to behave as a normal sane LRU cache [2])</p> <p style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: inherit;">Cheers, Mark</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">[1] http://doc.qt.io/qt-5/qpixmapcache.html [2] \
https://en.wikipedia.org/wiki/Cache_algorithms#LRU</p></pre> </blockquote>
<p>On May 21st, 2016, 6:04 a.m. UTC, <b>Frank Reininghaus</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><blockquote \
style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid \
#bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;"> <p \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">If we can't we could look into making TileSet an implicitly shared class so \
that we can return it by value cheaply.</p> </blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">Or alternatively, one could return not raw pointers, \
but shared pointers (like <code style="text-rendering: inherit;color: \
#4444cc;padding: 0;white-space: normal;margin: 0;line-height: \
inherit;">std::shared_ptr<TileSet></code> or <code style="text-rendering: \
inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: \
inherit;">QSharedPtr<TileSet></code>). Then it would be 100% clear that the \
caller, who gets the pointer from the cache, is now a co-owner of the object and can \
be sure that it won't be deleted as long as they use it.</p> <p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">Having a <code style="text-rendering: inherit;color: #4444cc;padding: \
0;white-space: normal;margin: 0;line-height: \
inherit;">QCache<QSharedPointer<TileSet>></code> would then store \
pointers to shared pointers, unfortunately, but if we made <code \
style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: \
0;line-height: inherit;">TileSet</code> implicitly shared, then it would essentially \
be the same, even if one of the pointers would be hidden inside the \
implementation.</p> <p style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: inherit;">I have the feeling that we need a \
replacement for <code style="text-rendering: inherit;color: #4444cc;padding: \
0;white-space: normal;margin: 0;line-height: inherit;">QCache</code> that works with \
shared pointers :-) <code style="text-rendering: inherit;color: #4444cc;padding: \
0;white-space: normal;margin: 0;line-height: inherit;">QCache</code> was probably \
designed in a time when there were only raw pointers, and no clear ways to express \
ownership (which is one of the reasons why tools like Coverity are needed).</p> <p \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">One could argue that a cache that deals with shared pointers adds a small \
overhead (due to the reference counting), but since a cache is supposed to be used \
with objects that are expensive to create, this should not matter at all.</p></pre> \
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">It would probably not be \
too difficult to make a simple cache that held shared pointers directly (to avoid \
pointers to shared pointers). If we embed an assumption that it's a FIFO then we \
already have QQueue<T> and it's simple. Even QCache itself is not very big, \
in fact I was able to verify directly that there are no other ways to delete a \
pointer upon ::insert other than exceeding the maxCost, which is already an error. It \
would be nice if Qt would document that this is the *only* failure mode (instead of \
that this is a *particular* failure mode), in that case we could just add an assert \
that the cost going in is less than maxCost and I /think/ that should satisfy \
Coverity (which is pretty smart in its static analysis).
However I've also started on tweaking the patch to return QColor directly \
(instead of a reference) which simplifies much of this right off the bat, and leaves \
TileSet alone as the ugly case. I do think that we need to do something different in \
here though (probably shared pointers!)... right now I'm not sure if the cache is \
really needed to reduce memory usage or if it's simply a way to share common data \
without duplicating it in memory everywhere.</pre> <br />
<p>- Michael</p>
<br />
<p>On May 8th, 2016, 5:03 a.m. UTC, Michael Pyne wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: \
1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; \
-webkit-border-radius: 6px;"> <tr>
<td>
<div>Review request for kde-workspace and Hugo Pereira Da Costa.</div>
<div>By Michael Pyne.</div>
<p style="color: grey;"><i>Updated May 8, 2016, 5:03 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
oxygen
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" \
style="border: 1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">This should mostly complete the QCache fixes I kicked \
off in a previous RR, 127837. Hugo noted there were many other similar usages, and \
boy he wasn't kidding! ;) The long story short is that these usages can theoretically \
cause use-after-free behavior (which can lead to crashes and even undefined behavior \
if the compiler ever gets smart).</p> <p style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: inherit;"><em style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
normal;">NOTE</em> It is -much- easier to review if you download the diff to your git \
repository for oxygen and then run "git diff -b" to ignore whitespace changes, \
particularly for the QPixmap changes.</p> <p style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: inherit;">For QPixmaps we return \
values instead of pointers, so we simply make a separate copy to be cached when we do \
insert. For QColor we return references to values so we <em style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
normal;">must</em> return pointers, and those have to be owned by a QCache to avoid \
memleaks. So I added a helper function to loop until the cache accepts the new entry. \
TileSets are a similar concern, except those have manual loops since I was uncertain \
about whether TileSet's copy constructor was the best idea or not.</p> <p \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">This fixes a ton of Coverity issues (59717 - 259733, 259739, 259742 - \
259752, 1336154, 1336155) and might be associated with Qt bug 38142 and KDE bug \
219055 (which doesn't actually appear to be a dupe of a different bug to \
me...).</p></pre> </td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: \
1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">Compiled without warnings, installed and ran <code \
style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: \
0;line-height: inherit;">oxygen-demo5 -style oxygen</code>. Used the GUI Benchmark \
feature to automatically cycle through all the listed features -- no crashes or \
obvious rendering errors.</p></pre> </td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>kstyle/oxygenstylehelper.cpp <span style="color: grey">(612ba37)</span></li>
<li>liboxygen/oxygenhelper.h <span style="color: grey">(a6453a0)</span></li>
<li>liboxygen/oxygenhelper.cpp <span style="color: grey">(4843604)</span></li>
<li>liboxygen/oxygenshadowcache.cpp <span style="color: grey">(907e586)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/127866/diff/" style="margin-left: \
3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>
--===============0783103058877689815==--
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic