fix segfault on task removal from list

This commit fixes a segfault which manifests while
processing a received message to update the list of
active processes. This segfault always happens on
removal of a process and sometimes an error
associated with the model size for the TableView is
also displayed on the journal.

The fix includes the following changes:
- add checks on the received ResponseMessage to
discard invalid messages;
- align with other libqtappfw-core clients which use
QJsonObject::value() returning QJsonValue, instead of
QJsonObject::operator[] returning QJsonValueRef;
- create local list of tasks using emplacement
instead of insertion;
- simplify matching & updating old tasklist against
new one; remove flag and use std algorithm find_if
with lambda.

Bug-AGL: SPEC-3402
Signed-off-by: Raquel Medina <raquel.medina@konsulko.com>
Change-Id: Ic6bfaefed3fe91a8d29ef622ba7957b5bed5018a

1 files changed, 46 insertions, 47 deletions

diff --git a/app/taskmanager.cpp b/app/taskmanager.cpp
index 77ef412..a0676cf 100644
--- a/app/taskmanager.cpp
+++ b/app/taskmanager.cpp
@@ -4,6 +4,8 @@
 #include <QString>
 #include <unistd.h>
 #include <iostream>
+#include <vector>
+#include <algorithm>
 #include <QtCore>
 #include <responsemessage.h>
 #include <callmessage.h>
@@ -79,24 +81,31 @@ void TaskManager::ProcessResponse(std::shared_ptr<Message> message)
 {
 	std::shared_ptr<ResponseMessage> rmsg = std::static_pointer_cast<ResponseMessage>(message);
 	qDebug() << "got message ";
-	QString msgType = rmsg->replyData()["msgType"].toString();
+	QString verb = rmsg->requestVerb();
+	if (rmsg->replyStatus() == "failed") {
+		qWarning() << "Reply Failed received for verb:" << verb;
+		return;
+	}
+
+	QJsonObject data = rmsg->replyData();
+	QString msgType = data.value("msgType").toString();
 	if (msgType.isNull())
 		return; // no type supplied, ignoring
 
 	if (QString::compare(msgType, "processList") == 0) {
-		QJsonArray processes = rmsg->replyData()["processes"].toArray();
+		QJsonArray processes = data.value("processes").toArray();
 		ProcessResponseTasklist(processes);
 	}
-	if (QString::compare(msgType, "extraInfo") == 0) {
-		QJsonObject info = rmsg->replyData()["info"].toObject();
+	else if (QString::compare(msgType, "extraInfo") == 0) {
+		QJsonObject info = data.value("info").toObject();
 		ProcessResponseExtraInfo(info);
 	}
-	if (QString::compare(msgType, "loadAvgInfo") == 0) {
-		QJsonObject loadInfo = rmsg->replyData()["loadInfo"].toObject();
+	else if (QString::compare(msgType, "loadAvgInfo") == 0) {
+		QJsonObject loadInfo = data.value("loadInfo").toObject();
 		ProcessResponseLoadAvg(loadInfo);
 	}
-	if (QString::compare(msgType, "netStatInfo") == 0) {
-		QJsonObject netstat = rmsg->replyData()["netstat"].toObject();
+	else if (QString::compare(msgType, "netStatInfo") == 0) {
+		QJsonObject netstat = data.value("netstat").toObject();
 		ProcessResponseNetStat(netstat);
 	}
 	// more response types to follow
@@ -106,54 +115,44 @@ void TaskManager::ProcessResponseTasklist(QJsonArray& processes)
 {
 	std::vector<ProcInfo> procs;
 
-	if (processes.size() == 0) {
-		// this is not a valid process list response
+	if (processes.size() == 0)
 		return;
-	}
 
 	for(auto it = processes.constBegin(); it != processes.constEnd(); ++it) {
-		ProcInfo Proc(it->toObject());
-		procs.push_back(Proc);
+		if (!it->toObject().isEmpty())
+			procs.emplace_back(it->toObject());
 	}
 
-	int flag;
-	if(m_procinfos.empty()){
-		for(auto it = procs.begin(); it != procs.end(); ++it){ // if m_procinfos is empty then this is first call
-			emit addProcess(it->cmd(), it->tid(), it->euid(), it->scpu(), it->ucpu(), it->resident_memory(), it->state()); 
-		}
-	} else {
-		for(auto it = procs.begin(); it != procs.end(); ++it){						// loop through procs, it = procs element (ProcInfo obj)
-			flag = 0;
-			for(auto it2 = m_procinfos.begin(); it2 != m_procinfos.end(); ++it2){	// loop through m_procinfos, it2 m_procinfos element (ProcInfo obj)
-				// if(*it == *it2){  // if the same ID exists in both vectors
-				if(it->tid() == it2->tid()){
-					if(it->cmd() == it2->cmd()){ // if the name is still the same
-						if(!(it == it2)){ // if the same process has new data
-							emit updateProcess(it->cmd(), it->tid(), it->euid(), it->scpu(), it->ucpu(), it->resident_memory(), it->state());
-							m_procinfos.erase(it2);
-							flag = 1;
-							break;
-						}
-					} else { // process in m_procinfos has died and a new one has its ID
-						qDebug() << "The process ID has been reused for another process";
-						emit updateProcess(it->cmd(), it->tid(), it->euid(), it->scpu(), it->ucpu(), it->resident_memory(), it->state());
-						m_procinfos.erase(it2);
-						flag = 1;
-					}
+	if (procs.empty())
+		return;
+
+	for(auto &item : procs) {
+		if(m_procinfos.empty()) {
+			// if old vector is empty, no need to update or remove elements, just add them
+			emit addProcess(item.cmd(), item.tid(), item.euid(), item.scpu(), item.ucpu(), item.resident_memory(), item.state());
+		} else {
+			std::vector<ProcInfo>::iterator it = std::find_if(m_procinfos.begin(), m_procinfos.end(),
+									  [item](const ProcInfo& proc) {
+									    return item.tid() == proc.tid(); });
+			if (it != m_procinfos.end()) {  // the same ID exists in both vectors
+				if (item.cmd() != it->cmd()) {  // but different names:
+					// the process in m_procinfos has died and a new one has its ID
+					qDebug() << "The process ID has been reused for another process";
 				}
-			}
-			if(flag == 0){ // if no ID was found in old vector; that means it's a new process
-				qDebug() << it->cmd() << " process has been added";
-				emit addProcess(it->cmd(), it->tid(), it->euid(), it->scpu(), it->ucpu(), it->resident_memory(), it->state()); 
+				emit updateProcess(item.cmd(), item.tid(), item.euid(), item.scpu(), item.ucpu(), item.resident_memory(), item.state());
+				m_procinfos.erase(it);
+			} else { // if the ID  was not found in old vector, that means it's a new process
+				qDebug() << item.cmd() << " new process has been added";
+				emit addProcess(item.cmd(), item.tid(), item.euid(), item.scpu(), item.ucpu(), item.resident_memory(), item.state());
 			}
 		}
-		for(auto it2 = m_procinfos.begin(); it2 != m_procinfos.end(); ++it2){ // remaining processes in m_procinfos are all dead
-			qDebug() << "Dead processes are removed";
-			qDebug() << it2->cmd();
-			emit removeProcess(it2->tid());
-		}
 	}
-	m_procinfos = procs;
+	// any remaining processes in old vector are all dead, remove them:
+	for(auto &olditem : m_procinfos) {
+		emit removeProcess(olditem.tid());
+	}
+
+	m_procinfos.swap(procs);
 }
 
 void TaskManager::ProcessResponseExtraInfo(QJsonObject &info)