Review Request 129995: Fix KillRunner Memory leak

[David Edmundson]

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


Fix it, then Ship it!





runners/kill/killrunner.cpp (line 185)
<https://git.reviewboard.kde.org/r/129995/#comment68398>

    just put it on the stack, that way you don't have to worry about any manual deletions.
    
    Or use the static QProcess::execute which wraps it all anyway.


- David Edmundson


On March 7, 2017, 7:15 a.m., Leslie Zhai wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129995/
> -----------------------------------------------------------
> 
> (Updated March 7, 2017, 7:15 a.m.)
> 
> 
> Review request for Plasma, Aleix Pol Gonzalez and Kai Uwe Broulik.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> -------
> 
> Hi KDE developers,
> 
> Bug reported by the clang static analyzer.
> 
> Description: Potential leak of memory pointed to by 'process'
> File: plasma-workspace/runners/kill/killrunner.cpp
> Line: 186
> 
> ```
> 168	void KillRunner::run(const Plasma::RunnerContext &context, const Plasma::QueryMatch &match)
> 169	{
> 170	    Q_UNUSED(context)
> 171	 
> 172	    QVariantList data = match.data().value<QVariantList>();
> 173	    quint64 pid = data[0].toUInt();
> 174	//     QString user = data[1].toString();
> 175	 
> 176	    int signal;
> 177	    if (match.selectedAction() != NULL) {
> 
> Assuming the condition is false	
> ?
> 
> ?
> Taking false branch	
> ?
> 178	        signal = match.selectedAction()->data().toInt();
> 179	    } else {
> 180	        signal = 9; //default: SIGKILL
> 181	    }
> 182	 
> 183	    QStringList args;
> 184	    args << QStringLiteral("-%1").arg(signal) << QStringLiteral("%1").arg(pid);
> 185	    KProcess *process = new KProcess(this);
> 
> ?
> Memory is allocated	
> ?
> 186	    int returnCode = process->execute(QStringLiteral("kill"), args);
> 
> ?
> Within the expansion of the macro 'QStringLiteral':
> a
> Potential leak of memory pointed to by 'process'
> 
> 187	 
> 188	    if (returnCode == 0)
> ```
> 
> So I simply add ```delete process``` to free the allocated memory.
> 
> Regards,
> Leslie Zhai
> 
> 
> Diffs
> -----
> 
>   runners/kill/killrunner.cpp 5c2e8529 
> 
> Diff: https://git.reviewboard.kde.org/r/129995/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Leslie Zhai
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20170307/3a27c9b6/attachment.html>