'Re: Review Request 126506: Fix improper destruction of non-virtual KDirModel subclasses'

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

List:       kde-frameworks-devel
Subject:    Re: Review Request 126506: Fix improper destruction of non-virtual KDirModel subclasses
From:       "David Faure" <faure () kde ! org>
Date:       2015-12-25 9:17:28
Message-ID: 20151225091728.29845.47920 () mimi ! kde ! org
[Download RAW message or body]

--===============5361969903913265116==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit

> On Dec. 25, 2015, 9:14 a.m., David Faure wrote:
> > Ship It!

withdrawn, see above

- David

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126506/#review90085
-----------------------------------------------------------

On Dec. 25, 2015, 12:12 a.m., Michael Pyne wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126506/
> -----------------------------------------------------------
> 
> (Updated Dec. 25, 2015, 12:12 a.m.)
> 
> 
> Review request for KDE Frameworks and David Faure.
> 
> 
> Repository: kio
> 
> 
> Description
> -------
> 
> Noted by Coverity (CID 1019869), and could result in a set of
> partially-destructed objects. Which, in this case, would probably leak
> memory in the event of multiple levels of filesystem hierarchy from the
> root KDirModel, but wouldn't cause any worse problems than that.
> 
> The 'proper' fix would be to add a virtual dtor at the base class but I
> didn't want to add a vtable just for this, so I mirrored the rest of the
> code and utilized the fact that item().isDir() is true for all derived
> classes.
> 
> 
> Diffs
> -----
> 
> src/widgets/kdirmodel.cpp af56a06 
> 
> Diff: https://git.reviewboard.kde.org/r/126506/diff/
> 
> 
> Testing
> -------
> 
> Builds, kdirmodeltest passes.
> 
> 
> Thanks,
> 
> Michael Pyne
> 
> 

--===============5361969903913265116==
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/126506/">https://git.reviewboard.kde.org/r/126506/</a>
  </td>
    </tr>
   </table>
   <br />

<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;">  <p style="margin-top: 0;">On December 25th, 2015, 9:14 a.m. UTC, <b>David \
Faure</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;">Ship It!</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;"><p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">withdrawn, see above</p></pre> <br />

<p>- David</p>

<br />
<p>On December 25th, 2015, 12:12 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 Frameworks and David Faure.</div>
<div>By Michael Pyne.</div>

<p style="color: grey;"><i>Updated Dec. 25, 2015, 12:12 a.m.</i></p>

<div style="margin-top: 1.5em;">
 <b style="color: #575012; font-size: 10pt;">Repository: </b>
kio
</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;">Noted by Coverity (CID 1019869), and could result in a \
set of partially-destructed objects. Which, in this case, would probably leak
memory in the event of multiple levels of filesystem hierarchy from the
root KDirModel, but wouldn't cause any worse problems than that.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">The 'proper' fix would be to add a virtual dtor at the \
base class but I didn't want to add a vtable just for this, so I mirrored the rest of \
the code and utilized the fact that item().isDir() is true for all derived
classes.</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;">Builds, kdirmodeltest passes.</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>src/widgets/kdirmodel.cpp <span style="color: grey">(af56a06)</span></li>

</ul>

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

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

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

--===============5361969903913265116==--

[Attachment #3 (text/plain)]

_______________________________________________
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

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