<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/107324/">http://git.reviewboard.kde.org/r/107324/</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;">First off, good catch on the memory leak, I agree the current code is not handling it correctly.

I would propose a different fix instead though. Usually we try to avoid performing manual memory management if we can avoid it, and there's no reason we can't have the compiler do the deallocation for us here. In fact, if you look at the return value of the createSignal() and createSlot() functions (which are being passed to qstrdup()) you will see that they already have a proper type: QByteArray.

So instead I would recommend changing the types of the signal and slot variables to QByteArray. You would have to use signal.constData() or slot.constData() in order to use the QMetaObject::indexOfSignal and QMetaObject::indexOfSlot methods, and also for QObject::connect, but no other changes should be needed, and QByteArray will make sure the memory is properly deallocated when the function returns.</pre>

 <br />

<p>- Michael</p>

<br />

<p>On November 14th, 2012, 11:39 a.m., Daniel Calviño Sánchez 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 kdelibs.</div>

<div>By Daniel Calviño Sánchez.</div>

<p style="color: grey;"><i>Updated Nov. 14, 2012, 11:39 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;">A char* is used to store the signal name, and another char* is used to store the slot name. Each name is a deep copy of the data of a QByteArray, made with qstrdup. Thus, the copies must be deleted using delete[].

Before, the function callConnect returned without deleting the signal and slot names. Now, the return value is got, the strings are deleted, and then the return value is returned. The strings are needed to get the return value, so they can not be deleted before getting it.</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;">Memory test for class ScriptingTest in KTutorial project (that is where I discovered the leak, as it uses KJSEmbed through Kross), and unit test for ScriptingTest (to ensure that everything still works as expected after the changes).</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>kjsembed/kjsembed/qobject_binding.cpp <span style="color: grey">(afc1989)</span></li>

</ul>

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

  </td>

 </tr>

</table>

  </div>

 </body>

</html>