'Re: Review Request: Verify pointer is valid when calculating the longest caption'

[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kwin
Subject:    Re: Review Request: Verify pointer is valid when calculating the longest caption
From:       "Commit Hook" <null () kde ! org>
Date:       2012-07-22 17:24:28
Message-ID: 20120722172428.4416.42568 () vidsolbach ! de
[Download RAW message or body]

[Attachment #2 (multipart/alternative)]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105645/#review16212
-----------------------------------------------------------

This review has been submitted with commit ff0f879b66e275030570d9d36909e7fecd01b3ec \
by Martin Gräßlin to branch KDE/4.9.

- Commit Hook

On July 21, 2012, 9:36 a.m., Martin Gräßlin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105645/
> -----------------------------------------------------------
> 
> (Updated July 21, 2012, 9:36 a.m.)
> 
> 
> Review request for kwin.
> 
> 
> Description
> -------
> 
> Verify pointer is valid when calculating the longest caption
> 
> The method was missing a check whether the weak pointers in the
> internal list got deleted. This could in very unlikely cases
> lead to a crash.
> 
> In order to verify that adding the null pointer check fixes the
> crash a unit test is added to simulate the situation of a
> pointer being deleted. This required to add a mock a few
> classes of TabBox. A MockTabBoxHandler and MockTabBoxClient are
> added implementing the specific interfaces. The DeclarativeView
> is completely mocked to make the linker happy. Including the
> actual implementation is not possible as it pulls in half of
> KWin core.
> 
> BUG: 303840
> FIXED-IN: 4.9.0
> 
> 
> This addresses bug 303840.
> http://bugs.kde.org/show_bug.cgi?id=303840
> 
> 
> Diffs
> -----
> 
> kwin/tabbox/CMakeLists.txt e222b6a63f3550781f271675592cad8ba2ebdfcf 
> kwin/tabbox/clientmodel.cpp 06a0c1a869330407033640f171d465e5e7cb8b53 
> kwin/tabbox/tests/CMakeLists.txt PRE-CREATION 
> kwin/tabbox/tests/mock_declarative.cpp PRE-CREATION 
> kwin/tabbox/tests/mock_tabboxclient.h PRE-CREATION 
> kwin/tabbox/tests/mock_tabboxclient.cpp PRE-CREATION 
> kwin/tabbox/tests/mock_tabboxhandler.h PRE-CREATION 
> kwin/tabbox/tests/mock_tabboxhandler.cpp PRE-CREATION 
> kwin/tabbox/tests/test_tabbox_clientmodel.h PRE-CREATION 
> kwin/tabbox/tests/test_tabbox_clientmodel.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/105645/diff/
> 
> 
> Testing
> -------
> 
> without change: unit test fails/crashes
> with change: unit test succeeds
> 
> 
> Thanks,
> 
> Martin Gräßlin
> 
> 

[Attachment #5 (text/html)]

<html>
 <body>
  <div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
   <table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 \
solid;">  <tr>
     <td>
      This is an automatically generated e-mail. To reply, visit:
      <a href="http://git.reviewboard.kde.org/r/105645/">http://git.reviewboard.kde.org/r/105645/</a>
  </td>
    </tr>
   </table>
   <br />

 <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This review has been \
submitted with commit ff0f879b66e275030570d9d36909e7fecd01b3ec by Martin Gräßlin to \
branch KDE/4.9.</pre>  <br />

<p>- Commit</p>

<br />
<p>On July 21st, 2012, 9:36 a.m., Martin Gräßlin wrote:</p>

<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" \
style="background-image: \
url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); \
background-position: left top; background-repeat: repeat-x; border: 1px black \
solid;">  <tr>
  <td>

<div>Review request for kwin.</div>
<div>By Martin Gräßlin.</div>

<p style="color: grey;"><i>Updated July 21, 2012, 9:36 a.m.</i></p>

<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;">Verify pointer is valid when calculating the longest caption

The method was missing a check whether the weak pointers in the
internal list got deleted. This could in very unlikely cases
lead to a crash.

In order to verify that adding the null pointer check fixes the
crash a unit test is added to simulate the situation of a
pointer being deleted. This required to add a mock a few
classes of TabBox. A MockTabBoxHandler and MockTabBoxClient are
added implementing the specific interfaces. The DeclarativeView
is completely mocked to make the linker happy. Including the
actual implementation is not possible as it pulls in half of
KWin core.

BUG: 303840
FIXED-IN: 4.9.0</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;">without change: unit test fails/crashes with change: unit test \
succeeds</pre>  </td>
 </tr>
</table>

<div style="margin-top: 1.5em;">
 <b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>

 <a href="http://bugs.kde.org/show_bug.cgi?id=303840">303840</a>

</div>

<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">

 <li>kwin/tabbox/CMakeLists.txt <span style="color: \
grey">(e222b6a63f3550781f271675592cad8ba2ebdfcf)</span></li>

 <li>kwin/tabbox/clientmodel.cpp <span style="color: \
grey">(06a0c1a869330407033640f171d465e5e7cb8b53)</span></li>

 <li>kwin/tabbox/tests/CMakeLists.txt <span style="color: \
grey">(PRE-CREATION)</span></li>

 <li>kwin/tabbox/tests/mock_declarative.cpp <span style="color: \
grey">(PRE-CREATION)</span></li>

 <li>kwin/tabbox/tests/mock_tabboxclient.h <span style="color: \
grey">(PRE-CREATION)</span></li>

 <li>kwin/tabbox/tests/mock_tabboxclient.cpp <span style="color: \
grey">(PRE-CREATION)</span></li>

 <li>kwin/tabbox/tests/mock_tabboxhandler.h <span style="color: \
grey">(PRE-CREATION)</span></li>

 <li>kwin/tabbox/tests/mock_tabboxhandler.cpp <span style="color: \
grey">(PRE-CREATION)</span></li>

 <li>kwin/tabbox/tests/test_tabbox_clientmodel.h <span style="color: \
grey">(PRE-CREATION)</span></li>

 <li>kwin/tabbox/tests/test_tabbox_clientmodel.cpp <span style="color: \
grey">(PRE-CREATION)</span></li>

</ul>

<p><a href="http://git.reviewboard.kde.org/r/105645/diff/" style="margin-left: \
3em;">View Diff</a></p>

  </td>
 </tr>
</table>

  </div>
 </body>
</html>

_______________________________________________
kwin mailing list
kwin@kde.org
https://mail.kde.org/mailman/listinfo/kwin

[prev in list] [next in list] [prev in thread] [next in thread] 