Fix indentation, cleanup, refactoring, deletions

* Remove unused methods for adding hostname and user to xml
* Avoid copying/(de)serializing XML a thousand times
* Fix Session::isValid() or rather make it a bit more usable
  (although it's unused currently)
* baseDir is global, not per VSession, which doesn't make any
  sense without the legacy approach of recusively loading
  one xml file per entry

1 files changed, 59 insertions, 130 deletions

diff --git a/src/vsession.cpp b/src/vsession.cpp
index c22e1bd..6344ec8 100644
--- a/src/vsession.cpp
+++ b/src/vsession.cpp
@@ -7,21 +7,21 @@
 #include <QStringList>
 #include <QIcon>
 #include <QMessageBox>
-#if 0
 #include <QHostInfo> // available since Qt 4.7
-#endif
-#include <limits> // for HOST_NAME_MAX
-#include <unistd.h> // for gethostname(), getuid()
-#include <sys/types.h> // for getuid, getpwuid
-#include <pwd.h> // for getpwuid
+#include <unistd.h> // for getuid()
+#include <sys/types.h> // for getuid(), getpwuid()
+#include <pwd.h> // for getpwuid()
 #include "globals.h"
 #include "vsession.h"
 #include "sessionsiconholder.h"
 
-bool VSession::init(const QString& xml, const QString& baseDirPath) {
-    this->baseDirPath_ = baseDirPath;
-    _process = new QProcess();
-    return this->doc_.setContent(xml);
+static QProcess _process;
+
+bool VSession::init(const QDomElement& xml) {
+    QDomElement settingsNode = this->doc_.createElement("settings");
+    this->doc_.appendChild(settingsNode);
+    eintrag_ = this->doc_.importNode(xml, true).toElement();
+    return !settingsNode.appendChild(eintrag_).isNull() && !settingsNode.isNull();
 }
 
 void VSession::addNodeWithAttribute(const QString& nodeName,
@@ -29,12 +29,12 @@ void VSession::addNodeWithAttribute(const QString& nodeName,
                                  const QString& attribute,
                                  bool replace) {
     QDomElement node =
-            this->doc_.namedItem("eintrag").namedItem(nodeName).toElement();
+            eintrag_.firstChildElement(nodeName);
 
     if (replace == false || node.isNull()) {
         // create a new node
         node = this->doc_.createElement(nodeName);
-        this->doc_.namedItem("eintrag").appendChild(node);
+        eintrag_.appendChild(node);
     }
 
     node.setAttribute(attribute, value);
@@ -94,24 +94,29 @@ QIcon VSession::icon() const {
 }
 
 QString VSession::toXml() const {
-    QDomDocument doc;
-    doc.appendChild(doc.createElement("settings"));
-
-    doc.firstChild().appendChild(doc.importNode(doc_.documentElement(), true));
+    QDomDocument doc(doc_);
 
     QDomNode xmlNode = doc.createProcessingInstruction(
             "xml", "version=\"1.0\" encoding=\"UTF-8\"");
     doc.insertBefore(xmlNode, doc.firstChild());
-
+    QString image(this->getAttribute("image_name"));
+    QDomElement path = doc.firstChildElement("settings")
+            .firstChildElement("eintrag")
+            .appendChild(doc.createElement("image_path"))
+            .toElement();
+    if (QFileInfo(image).isRelative()) {
+        // make path to image absolute
+        path.setAttribute("param", basePath + "/" + image);
+    } else {
+        path.setAttribute("param", image);
+    }
     return doc.toString();
 }
 
 QString VSession::getAttribute(const QString &nodeName,
                                 const QString &attribute) const {
-    QDomDocument doc = this->doc_;
-    QDomNode n = doc.namedItem("eintrag").namedItem(nodeName);
-    return this->doc_.namedItem("eintrag").namedItem(nodeName).toElement()
-            .attribute(attribute);
+    QDomNode n = eintrag_.firstChildElement(nodeName);
+    return eintrag_.firstChildElement(nodeName).attribute(attribute);
 }
 
 QList<QString> VSession::keywords() const {
@@ -119,7 +124,7 @@ QList<QString> VSession::keywords() const {
 }
 
 void VSession::readKeywords() {
-    QDomNode keywordsNode = this->doc_.namedItem("eintrag").namedItem("keywords");
+    QDomNode keywordsNode = eintrag_.namedItem("keywords");
     for (QDomElement el(keywordsNode.firstChildElement("keyword"));
             !el.isNull();
             el = el.nextSiblingElement("keyword")) {
@@ -216,8 +221,7 @@ bool VSession::isLocked() const {
 }
 
 bool VSession::isValid() const {
-    // default to false
-    return QFile::exists(QString(this->baseDirPath_).append("/").append(getAttribute("image_name")));
+    return !getAttribute("image_name").isEmpty();
 }
 
 int VSession::priority() const {
@@ -231,90 +235,21 @@ int VSession::priority() const {
     return prio;
 }
 
-void VSession::addUserAndHostname() {
-    QString username(getpwuid(geteuid())->pw_name);
-    this->addNodeWithAttribute("username", username);
-
-    // Qt >= 4.7 has <QHostInfo>
-    // QString hostname(QHostInfo::localHostName());
-    char hname[HOST_NAME_MAX + 1];
-    QString hostname;
-    if (gethostname(hname, HOST_NAME_MAX) == 0) {
-        hostname = QString::fromUtf8(hname);
-    } else {
-        hostname = QString::fromUtf8("unknown-%1").arg(qrand());
-    }
-    this->addNodeWithAttribute("hostname", hostname);
-
-    QString image(this->getAttribute("image_name"));
-    if (QFileInfo(image).isRelative()) {
-        // make path to image absolute
-        this->addNodeWithAttribute("image_path",
-                                   this->baseDirPath_ + "/" + image);
-    } else {
-        this->addNodeWithAttribute("image_path", image);
-    }
-
-    // insert computername as the first child of <eintrag>
-    // bootpgm needs computername within the first 500 bytes
-    QDomElement computername(doc_.createElement("computername"));
-    computername.setAttribute("param", hostname.append("-%1").arg(qrand()));
-    this->doc_.namedItem("eintrag").insertBefore(computername, QDomNode());
-}
-
-void VSession::mergePoolXml() {
-    QDomDocument doc;
-
-    QString poolXmlFile = etcPath + "/vmchooser-" + pool + ".xml";
-
-    QFile file(poolXmlFile);
-    if (!file.open(QIODevice::ReadOnly)) {
-        return;
-    }
-    if (!doc.setContent(&file)) {
-        file.close();
-        return;
-    }
-    file.close();
-
-    for (QDomElement envNode(doc.firstChildElement("environment"));
-    !envNode.isNull();
-    envNode = envNode.nextSiblingElement()) {
-        if (envNode.attribute("param") != pool) continue;
-
-        for (QDomElement typeNode(envNode.firstChildElement());
-        !typeNode.isNull();
-        typeNode = typeNode.nextSiblingElement()) {
-            QString type = typeNode.nodeName();
-            if (type != "shared_folders" &&
-                type != "printers" &&
-                type != "scanners") continue;
-
-            QDomElement destinationNode =
-                    this->doc_.namedItem("eintrag").namedItem(type).toElement();
-
-            if (destinationNode.isNull()) {
-                // create new node
-                destinationNode = this->doc_.createElement(type);
-                this->doc_.namedItem("eintrag").appendChild(destinationNode);
-            }
-
-            for (QDomElement el(typeNode.firstChildElement());
-            !el.isNull();
-            el = el.nextSiblingElement()) {
-                destinationNode.appendChild(this->doc_.importNode(el, true));
-            }
-        }
-    }
-}
-
 bool VSession::run() const {
+    if (_process.state() != QProcess::NotRunning) {
+        qDebug() << "Cannot start vsession while old one is still running";
+        return false;
+    }
     if (debugMode) {
         qDebug() << "Sarting session " << this->getAttribute("short_description", "param") << " ...";
     }
 
     if (g_noVtx && is64Bit()) {
-        QMessageBox::warning(NULL, QObject::trUtf8("Warning"), QObject::trUtf8("The selected session is based on a 64 bit operating system, but this computer doesn't seem to support this (VT-x/AMD-V not supported by CPU, or disabled in BIOS). You will probably get an error message while the virtualizer is initializing."));
+        QMessageBox::warning(NULL, QObject::trUtf8("Warning"),
+                QObject::trUtf8("The selected session is based on a 64 bit operating system,"
+                        " but this computer doesn't seem to support this (VT-x/AMD-V not"
+                        " supported by CPU, or disabled in BIOS). You will probably get an"
+                        " error message while the virtualizer is initializing."));
     }
 
     QString command = getAttribute("command");
@@ -322,24 +257,22 @@ bool VSession::run() const {
         return QProcess::startDetached(command);
     }
 
-    VSession session = *this;
-
-    session.addUserAndHostname();
-
-    session.mergePoolXml();
-
     // write xml to temporary file
     QTemporaryFile tmpfile(QDir::tempPath() + "/vmchooser-XXXXXX.xml");
     if (!tmpfile.open() ||
-        tmpfile.write(session.toXml().toUtf8()) == -1) {
+        tmpfile.write(this->toXml().toUtf8()) == -1) {
+        qDebug() << "Error writing xml to file" << tmpfile.fileName();
         return false;
     }
-    tmpfile.close();
+    // Docs say we should call fileName() before closing to prevent deletion
+    QString tmpFileName = tmpfile.fileName();
     tmpfile.setAutoRemove(false);
+    tmpfile.close();
 
-    _process->start(runVmScript, QStringList(tmpfile.fileName()));
-    QObject::connect(_process, SIGNAL(finished(int, QProcess::ExitStatus)), QApplication::instance(), SLOT(quit()));
-    if (_process->state() == QProcess::Starting || _process->state() == QProcess::Running)
+    QObject::connect(&_process, SIGNAL(finished(int, QProcess::ExitStatus)), QApplication::instance(), SLOT(quit()));
+    _process.start(runVmScript, QStringList(tmpFileName));
+    _process.waitForStarted(10);
+    if (_process.state() == QProcess::Starting || _process.state() == QProcess::Running)
         return true;
     else
         return false;
@@ -350,7 +283,7 @@ int VSession::type() const {
 }
 
 QList<Session*> VSession::readXmlFile(const QString& filepath) {
-    QList<Session*> retval;
+    QList<Session*> sessionList;
 
     QDomDocument doc;
     QFile file(filepath);
@@ -360,7 +293,7 @@ QList<Session*> VSession::readXmlFile(const QString& filepath) {
         if (debugMode) {
             qDebug() << "Cannot read file: " << file.fileName();
         }
-        return retval;
+        return sessionList;
     }
 
     if (!doc.setContent(&file)) {
@@ -375,7 +308,7 @@ QList<Session*> VSession::readXmlFile(const QString& filepath) {
             if (debugMode) {
                 qDebug() << "Cannot read backup file " << xml_filename << " either";
             }
-            return retval;
+            return sessionList;
         }
 
         if (!doc.setContent(&backup_file)) {
@@ -383,7 +316,7 @@ QList<Session*> VSession::readXmlFile(const QString& filepath) {
                 qDebug() << "XML file not valid: " << backup_file.fileName();
             }
             backup_file.close();
-            return retval;
+            return sessionList;
         }
 
         if (debugMode) {
@@ -404,30 +337,26 @@ QList<Session*> VSession::readXmlFile(const QString& filepath) {
         }
     }
 
-    QString dirName;
-    if (basePath.isEmpty()) {
-        dirName = QFileInfo(xml_filename).dir().absolutePath();
-    } else {
-        dirName = basePath;
-    }
     QDomElement settingsNode = doc.firstChildElement("settings");
     for (QDomElement el(settingsNode.firstChildElement("eintrag"));
             !el.isNull();
             el = el.nextSiblingElement("eintrag")) {
-        QDomDocument dummy;
-        dummy.appendChild(dummy.importNode(el, true));
         VSession* e = new VSession;
-        if (e->init(dummy.toString(), dirName) && e->isActive()) {
+        if (e->init(el) && e->isActive()) {
             e->readKeywords();
-            retval.append(e);
+            sessionList.append(e);
+        } else {
+            delete e;
         }
     }
-    return retval;
+    return sessionList;
 }
 
 bool VSession::is64Bit() const {
-    return imgtype() == VMWARE && getAttribute("os").endsWith("64");
-    // TODO: vmbox, qemu-kvm, ...
+    ImgType type = imgtype();
+    return (type == VMWARE && getAttribute("os").endsWith("-64"))
+            || (type == VBOX && getAttribute("os").endsWith("_64"));
+    // TODO: qemu-kvm, ...
 }
 
 QVariant VSession::foregroundRole() const {