Rename variables and refactor per comments from @matejak

Signed-off-by: Alexander Scheel <[email protected]>

Author: cipherboy

include/ScanningSession.h

@@ -291,14 +291,14 @@ class ScanningSession
         /// Our own tailoring that may or may not initially be loaded from a file
         mutable struct xccdf_tailoring* mTailoring;
 
-        /// Temporary copy of opened XCCDF file
-        QTemporaryDir* mOpenDir;
-        /// Path to temporary XCCDF file
-        QString mOpenPath;
-        /// Path to original XCCDF file
-        QString mOriginalPath;
-        /// Closure of original XCCDF file
-        QSet<QString> mClosureOfFile;
+        /// Temporary copy of opened DS or XCCDF file
+        QTemporaryDir* mTempOpenDir;
+        /// Path to temporary DS or XCCDF file
+        QString mTempOpenPath;
+        /// Path to original DS or XCCDF file
+        QString mOriginalOpenPath;
+        /// Closure of original DS or XCCDF file
+        QSet<QString> mClosureOfOriginalFile;
 
         /// Temporary file provides auto deletion and a valid temp file path
         QTemporaryFile mTailoringFile;
@@ -319,12 +319,14 @@ class ScanningSession
         QString mUserTailoringCID;
 
         /// Gets the dependency closure of the specified file.
-        void getDependencyClosureOfFile(const QString& filePath, QSet<QString>& targetSet) const;
+        void updateDependencyClosureOfFile(const QString& filePath, QSet<QString>& targetSet) const;
 
         /// Clones openFile(path) to a Temporary File
         void cloneToTemporaryFile(const QString& path);
         /// Closes mOpenFile if it is open
         void cleanTmpDir();
+        /// Copies all files from the path into the temporary location
+        void copyTempFiles(QString path, QString baseDirectory, const QFileInfo pathInfo);
 };
 
 #endif

src/ScanningSession.cpp

@@ -47,7 +47,9 @@ ScanningSession::ScanningSession():
     mSession(0),
     mTailoring(0),
 
-    mOpenDir(NULL),
+    mTempOpenDir(NULL),
+    mTempOpenPath(""),
+    mOriginalOpenPath(""),
 
     mSkipValid(false),
     mSessionDirty(false),
@@ -80,17 +82,63 @@ struct xccdf_session* ScanningSession::getXCCDFSession() const
 void ScanningSession::cleanTmpDir()
 {
     /*
-     * For use in cloneToTemporaryFile(...); closes mOpenDir
+     * For use in cloneToTemporaryFile(...); closes mTempOpenDir
      * if it is open and removes backing source. Ignores errors.
      *
      * A new Temporary Directory will be created afterwards,
      * so the worst case is leaving temporary directory (+files),
      * but this is cleaned on reboot or manually by the user.
      */
-    if (mOpenDir != NULL) {
-        mOpenDir->remove();
-        delete mOpenDir;
-        mOpenDir = NULL;
+    if (mTempOpenDir != NULL) {
+        mTempOpenDir->remove();
+        delete mTempOpenDir;
+        mTempOpenDir = NULL;
+    }
+}
+
+void ScanningSession::copyTempFiles(QString path, QString baseDirectory,
+                                    const QFileInfo pathInfo)
+{
+    // File is valid; extract the new location and copy dependencies into
+    // their correct places.
+    QString tmpPath = mTempOpenDir->path() + QDir::separator();
+
+    // set mTempOpenPath to the new temporary location and copy it.
+    mTempOpenPath = tmpPath + pathInfo.fileName();
+    QFile::copy(path, mTempOpenPath);
+
+    QDir tmpDir(tmpPath);
+    for (const QString& cur : mClosureOfOriginalFile)
+    {
+        const QFileInfo curInfo(cur);
+        const QDir curDir = curInfo.absoluteDir();
+
+        if (curDir.absolutePath() != baseDirectory)
+        {
+            // Create subdirectories and copy into the new directory
+            //
+            // Since curPath is an absolute path, which has an eventual
+            // parent of baseDirectory, the left n = baseDirectory.length()+1
+            // characters are baseDirectory plus a trailing slash; the
+            // remaining characters to the right of this (curPath.length()-n)
+            // is the recursive directory structure of cur (e.g., all the
+            // subdirs it resides in). We can then plant this on tmpPath
+            // and use mkpath to recreate that directory structure. We ignore
+            // errors from mkpath because we might have previously created
+            // this path in a previous loop iteration.
+            const QString curPath = curDir.absolutePath();
+            const int pathLen = curPath.length() - baseDirectory.length() - 1;
+            const QString subDirs = curPath.right(pathLen);
+            const QString newPath = tmpPath + subDirs + QDir::separator() + curInfo.fileName();
+
+            tmpDir.mkpath(tmpPath + subDirs);
+            QFile::copy(cur, newPath);
+        }
+        else
+        {
+            // Simply copy the file since it resides in baseDirectory
+            QFile::copy(cur, tmpPath + curInfo.fileName());
+        }
     }
 }
 
@@ -104,7 +152,7 @@ void ScanningSession::cloneToTemporaryFile(const QString& path)
      * 2) We need to create a new temporary file:
      *      Then we can copy data into it from path.
      *
-     * By not closing and removing mOpenDir in closeFile()
+     * By not closing and removing mTempOpenDir in closeFile()
      * until openFile() is called, we prevent unintentional reloading
      * of the real path. Further, calling fileOpen(...) with the same
      * path results in reloading the file.
@@ -113,12 +161,12 @@ void ScanningSession::cloneToTemporaryFile(const QString& path)
     // Clean the temporary directory if it is open already, then create
     // a new one.
     cleanTmpDir();
-    mOpenDir = new QTemporaryDir();
+    mTempOpenDir = new QTemporaryDir();
 
     // Recalling is unlikely to succeed, so throw a fatal exception
-    if (!mOpenDir->isValid())
+    if (!mTempOpenDir->isValid())
     {
-        throw std::runtime_error(mOpenDir->errorString().toUtf8().constData());
+        throw std::runtime_error(mTempOpenDir->errorString().toUtf8().constData());
     }
 
     // XCCDF files can have relative includes; get a complete closure set
@@ -128,14 +176,14 @@ void ScanningSession::cloneToTemporaryFile(const QString& path)
     //
     // Files which violate this constraint are rare; thus it should be safe
     // to refuse to open them.
-    mClosureOfFile.clear();
-    getDependencyClosureOfFile(path, mClosureOfFile);
+    mClosureOfOriginalFile.clear();
+    updateDependencyClosureOfFile(path, mClosureOfOriginalFile);
 
     const QFileInfo pathInfo(path);
     QString baseDirectory = pathInfo.absolutePath();
-    for (const QString& cur : mClosureOfFile)
+    for (const QString& cur : mClosureOfOriginalFile)
     {
-        // Since getDependencyClosureOfFile(...) returns cleaned absolute
+        // Since updateDependencyClosureOfFile(...) returns cleaned, absolute
         // paths, we can check if the path starts with the baseDirectory
         if (!cur.startsWith(baseDirectory))
         {
@@ -145,63 +193,24 @@ void ScanningSession::cloneToTemporaryFile(const QString& path)
         }
     }
 
-    // File is valid; extract the new location and copy dependencies into
-    // their correct places.
-    QString tmpPath = mOpenDir->path() + QDir::separator();
-
-    // set mOpenPath to the new temporary location and copy it.
-    mOpenPath = tmpPath + pathInfo.fileName();
-    QFile::copy(path, mOpenPath);
-
-    QDir tmpDir(tmpPath);
-    for (const QString& cur : mClosureOfFile)
-    {
-        const QFileInfo curInfo(cur);
-        const QDir curDir = curInfo.absoluteDir();
-
-        if (curDir.absolutePath() != baseDirectory)
-        {
-            // Create subdirectories and copy into the new directory
-            //
-            // Since curPath is an absolute path, which has an eventual
-            // parent of baseDirectory, the left n = baseDirectory.length()+1
-            // characters are baseDirectory plus a trailing slash; the
-            // remaining characters to the right of this (curPath.length()-n)
-            // is the recursive directory structure of cur (e.g., all the
-            // subdirs it resides in). We can then plant this on tmpPath
-            // and use mkpath to recreate that directory structure. We ignore
-            // errors from mkpath because we might have previously created
-            // this path in a previous loop iteration.
-            const QString curPath = curDir.absolutePath();
-            const int pathLen = curPath.length() - baseDirectory.length() - 1;
-            const QString subDirs = curPath.right(pathLen);
-            const QString newPath = tmpPath + subDirs + QDir::separator() + curInfo.fileName();
-
-            tmpDir.mkpath(tmpPath + subDirs);
-            QFile::copy(cur, newPath);
-        }
-        else
-        {
-            // Simply copy the file since it resides in baseDirectory
-            QFile::copy(cur, tmpPath + curInfo.fileName());
-        }
-    }
+    // Copy all files into the new temporary directory since the structure is valid.
+    copyTempFiles(path, baseDirectory, pathInfo);
 
-    mOriginalPath = path;
+    mOriginalOpenPath = path;
 }
 
 void ScanningSession::openFile(const QString& path, bool reload)
 {
     if (mSession)
         closeFile();
 
-    if (reload || mOriginalPath != path)
+    if (reload || mOriginalOpenPath != path)
     {
         cloneToTemporaryFile(path);
     }
 
-    const QString tmpPath = mOpenPath;
-    const QFileInfo pathInfo(mOpenPath);
+    const QString tmpPath = mTempOpenPath;
+    const QFileInfo pathInfo(mTempOpenPath);
 
     // We have to make sure that we *ALWAYS* open the session by absolute
     // path. oscap local won't be run from the same directory from where
@@ -250,15 +259,15 @@ QString ScanningSession::getOriginalFilePath() const
     if (!fileOpened())
         return QString("");
 
-    return mOriginalPath;
+    return mOriginalOpenPath;
 }
 
 QSet<QString> ScanningSession::getOriginalClosure() const
 {
-    return mClosureOfFile;
+    return mClosureOfOriginalFile;
 }
 
-void ScanningSession::getDependencyClosureOfFile(const QString& filePath, QSet<QString>& targetSet) const
+void ScanningSession::updateDependencyClosureOfFile(const QString& filePath, QSet<QString>& targetSet) const
 {
     QFileInfo fileInfo(filePath);
     targetSet.insert(fileInfo.absoluteFilePath()); // insert current file
@@ -299,7 +308,7 @@ void ScanningSession::getDependencyClosureOfFile(const QString& filePath, QSet<Q
                 const QAbstractXmlNodeModel* model = itemIdx.model();
                 const QString relativeFileName = model->stringValue(itemIdx);
                 const QString absUncleanPath = parentDir.absoluteFilePath(relativeFileName);
-                getDependencyClosureOfFile(QDir::cleanPath(absUncleanPath), targetSet);
+                updateDependencyClosureOfFile(QDir::cleanPath(absUncleanPath), targetSet);
                 item = result.next();
             }
 
@@ -323,7 +332,7 @@ void ScanningSession::getDependencyClosureOfFile(const QString& filePath, QSet<Q
 QSet<QString> ScanningSession::getOpenedFilesClosure() const
 {
     QSet<QString> ret;
-    ScanningSession::getDependencyClosureOfFile(getOpenedFilePath(), ret);
+    ScanningSession::updateDependencyClosureOfFile(getOpenedFilePath(), ret);
     return ret;
 }