Review Request: Fix memory leak in KJSEmbed when connecting signals and slots through the QObject binding

Michael Pyne mpyne at kde.org
Fri Nov 16 03:57:22 GMT 2012


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

Ship it!


Looks good (even a bit better than my initial try). Assuming it still passes your Kross and script tests then please commit (since this is kdelibs, it should go to the KDE/4.9 branch only, it will be forward-ported for you).

- Michael Pyne


On Nov. 15, 2012, 10:42 a.m., Daniel Calviño Sánchez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107324/
> -----------------------------------------------------------
> 
> (Updated Nov. 15, 2012, 10:42 a.m.)
> 
> 
> Review request for kdelibs.
> 
> 
> Description
> -------
> 
> 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.
> 
> 
> Diffs
> -----
> 
>   kjsembed/kjsembed/qobject_binding.cpp afc1989 
> 
> Diff: http://git.reviewboard.kde.org/r/107324/diff/
> 
> 
> Testing
> -------
> 
> 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).
> 
> 
> Thanks,
> 
> Daniel Calviño Sánchez
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20121116/4144fe02/attachment.htm>


More information about the kde-core-devel mailing list