<table><tr><td style="">kossebau added a comment.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D28330">View Revision</a></tr></table><br /><div><div><p>With cross-casting I meant the type of cast where the types involved are not in same inheritance branch</p></div></div><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D28330#inline-161199">View Inline</a><span style="color: #4b4d51; font-weight: bold;">davidre</span> wrote in <span style="color: #4b4d51; font-weight: bold;">breezebutton.cpp:148</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">I don't think it can fail. decoration() is a QPointer (<a href="https://api.kde.org/kdecoration/html/classKDecoration2_1_1DecorationButton.html" class="remarkup-link" target="_blank" rel="noreferrer">https://api.kde.org/kdecoration/html/classKDecoration2_1_1DecorationButton.html</a>) do you mean that with cross-cast?</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Looking at the rest of the code, it always tests for decoration() resolving to an existing <tt style="background: #ebebeb; font-size: 13px;">Decoration</tt> pointer. <br />
While I would have expected it might be ensured that decoration always is of type <tt style="background: #ebebeb; font-size: 13px;">Breeze::Decoration</tt>, but <tt style="background: #ebebeb; font-size: 13px;">Button::create(...)</tt> takes a <tt style="background: #ebebeb; font-size: 13px;">KDecoration2::Decoration</tt>, so at least by code in this class this is not ensured.</p>
<p style="padding: 0; margin: 8px;">Now, <tt style="background: #ebebeb; font-size: 13px;">Button::paint(...)</tt> already has half the check ealier, by the</p>
<div class="remarkup-code-block" style="margin: 12px 0;" data-code-lang="text" data-sigil="remarkup-code-block"><pre class="remarkup-code" style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; padding: 12px; margin: 0; background: rgba(71, 87, 120, 0.08);">if (!decoration()) return;</pre></div>
<p style="padding: 0; margin: 8px;">But this does not ensure we have a <tt style="background: #ebebeb; font-size: 13px;">Breeze::Decoration</tt> type.</p>
<p style="padding: 0; margin: 8px;">And QPointer only ensures that the pointer is updated if the object is deleted elsewhere. It still can be nullptr, and possibly this is done here as the decoration object is not owned by us, so something might delete it behind the button's back.</p>
<p style="padding: 0; margin: 8px;">So IMHO this logic might need another check by someone who can tell if the right type can be still assumed (which ideally then should be reflected in the API, so it is clear no other type could be present.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D28330#inline-161211">View Inline</a><span style="color: #4b4d51; font-weight: bold;">breezebutton.cpp:151</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; "> <span class="n">decoration</span><span class="p">()</span><span style="color: #aa2211">-></span><span class="n">client</span><span class="p">().</span><span class="n">data</span><span class="p">()</span><span style="color: #aa2211">-></span><span class="n">icon</span><span class="p">().</span><span class="n">paint</span><span class="p">(</span><span class="n">painter</span><span class="p">,</span> <span class="n">iconRect</span><span class="p">.</span><span class="n">toRect</span><span class="p">());</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);">
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span class="bright"> </span><span class="n"><span class="bright">KIconLoader</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">::</span></span><span class="bright"></span><span class="n"><span class="bright">global</span></span><span class="bright"></span><span class="p"><span class="bright">()</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">-></span></span><span class="bright"></span><span class="n"><span class="bright">resetPalette</span></span><span class="bright"></span><span class="p"><span class="bright">();</span></span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Assumes there is no other custom palette set before we set ours here. Does that assumption hold? Or could KWin & Co do mess with that palette as well?</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R31 Breeze</div></div></div><br /><div><strong>BRANCH</strong><div><div>icons (branched from master)</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D28330">https://phabricator.kde.org/D28330</a></div></div><br /><div><strong>To: </strong>davidre, Plasma, VDG, ngraham<br /><strong>Cc: </strong>kossebau, ngraham, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart<br /></div>