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



 <p>Ship it!</p>









<div>




<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
 <thead>
  <tr>
   <th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
    <a href="https://git.reviewboard.kde.org/r/118914/diff/1/?file=284277#file284277line308" style="color: black; font-weight: bold; text-decoration: underline;">klipper/urlgrabber.cpp</a>
    <span style="font-weight: normal;">

     (Diff revision 1)

    </span>
   </th>
  </tr>
 </thead>

 <tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
  <tr>

   <td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void URLGrabber::execute( const ClipAction* action, int cmdIdx ) const</pre></td>

  </tr>
 </tbody>



 
 

 <tbody>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
    <th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">308</font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">            <span class="k">const_cast</span><span class="o"><</span><span class="n">URLGrabber</span><span class="o">*></span><span class="p">(</span><span class="k">this</span><span class="p">)</span><span class="o">-></span><span class="n">m_myClipItem</span> <span class="o">=</span> <span class="n">nullptr</span><span class="p">;</span></pre></td>
  </tr>

 </tbody>

</table>

<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Just make the function not const?</pre>
</div>
<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;">can you null m_historyItem in clipcommandprocess.cpp:68.
It's not a problem right now, but it's still not good to leave things dangling.

Looks like it'll work, but I think it's rather fragile for when someone tries to change it in the future. insert() should't delete arguments without telling you.

If you promise to change it to implicitly shared for 5.x. Ship it.</pre>

<p>- David Edmundson</p>


<br />
<p>On June 24th, 2014, 10:29 a.m. UTC, Martin Gräßlin wrote:</p>








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

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


<p style="color: grey;"><i>Updated June 24, 2014, 10:29 a.m.</i></p>









<div style="margin-top: 1.5em;">
 <b style="color: #575012; font-size: 10pt;">Repository: </b>
plasma-workspace
</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;">[klipper] Fix memory leaks

Klipper is leaking HistoryItems. This can happen in two cases:
* inserting when the item already exists - nobody deleted the new item
* removing items from the history

The ownership of HistoryItems are clearly passed to the History as we
can see by the fact that it deletes all items in the dtor. This means
ownership is passed to History.

For inserting it is relatively simple as most usage is just creating
a new item and inserting it. Removing is more difficult as it takes
the item and the callee might still be using it. This needs some
testing.</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;">valgrind on a unit test I'm writing and all mem leaks fixed. Still need to properly test it for regressions.</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>klipper/history.cpp <span style="color: grey">(24e2ef4cf9784bcf23b8629cf4442fc90324dd8b)</span></li>

 <li>klipper/klipper.h <span style="color: grey">(1dd520fce258e2cb147bcf6a1d83891cc56a9d73)</span></li>

 <li>klipper/klipper.cpp <span style="color: grey">(2d6168e8517a9be23d42bb619e7c85d94d141e1b)</span></li>

 <li>klipper/urlgrabber.cpp <span style="color: grey">(38e4919814fa633c78f3956d94b655c6c23d8fcc)</span></li>

</ul>

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







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








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