tons of code cleanup and minor fixes to harden the code (mostly suggestions from static code analysis)

15 files changed, 42 insertions, 62 deletions

diff --git a/src/ModOrganizer.pro b/src/ModOrganizer.pro
index a8f7f53e..c0e5d9f5 100644
--- a/src/ModOrganizer.pro
+++ b/src/ModOrganizer.pro
@@ -8,7 +8,6 @@ SUBDIRS = bsatk \
           archive \

           helper \

           plugins \

-          proxydll \

           nxmhandler \

           BossDummy \

           pythonRunner \

diff --git a/src/aboutdialog.cpp b/src/aboutdialog.cpp
index cd29600b..7b311263 100644
--- a/src/aboutdialog.cpp
+++ b/src/aboutdialog.cpp
@@ -37,7 +37,7 @@ AboutDialog::AboutDialog(const QString &version, QWidget *parent)
   m_LicenseFiles[LICENSE_ZLIB] = "zlib.txt";

   m_LicenseFiles[LICENSE_APACHE2] = "apache-license-2.0.txt";

 

-  addLicense("Qt 5.3", LICENSE_LGPL3);

+  addLicense("Qt 5.4", LICENSE_LGPL3);

   addLicense("Qt Json", LICENSE_GPL3);

   addLicense("Boost Library", LICENSE_BOOST);

   addLicense("7-zip", LICENSE_LGPL3);

diff --git a/src/helper.cpp b/src/helper.cpp
index 4c22dbda..1f072bd9 100644
--- a/src/helper.cpp
+++ b/src/helper.cpp
@@ -49,7 +49,7 @@ static bool helperExec(LPCWSTR moDirectory, LPCWSTR commandLine)
 

   ::ShellExecuteExW(&execInfo);

 

-  if (::WaitForSingleObject(execInfo.hProcess, INFINITE) != WAIT_OBJECT_0) {

+  if ((execInfo.hProcess == 0) || (::WaitForSingleObject(execInfo.hProcess, INFINITE) != WAIT_OBJECT_0)) {

     reportError(QObject::tr("helper failed"));

     return false;

   }

diff --git a/src/installationmanager.cpp b/src/installationmanager.cpp
index 6101cf39..7d431f02 100644
--- a/src/installationmanager.cpp
+++ b/src/installationmanager.cpp
@@ -70,7 +70,8 @@ static T resolveFunction(QLibrary &lib, const char *name)
 

 

 InstallationManager::InstallationManager()

-  : m_InstallationProgress(nullptr)

+  : m_ParentWidget(nullptr)

+  , m_InstallationProgress(nullptr)

   , m_SupportedExtensions({ "zip", "rar", "7z", "fomod", "001" })

 {

   QLibrary archiveLib("dlls\\archive.dll");

@@ -441,7 +442,6 @@ QString InstallationManager::generateBackupName(const QString &directoryName) co
 bool InstallationManager::testOverwrite(GuessedValue<QString> &modName, bool *merge) const

 {

   QString targetDirectory = QDir::fromNativeSeparators(m_ModsDirectory + "\\" + modName);

-

   while (QDir(targetDirectory).exists()) {

     QueryOverwriteDialog overwriteDialog(m_ParentWidget);

     if (overwriteDialog.exec()) {

@@ -644,7 +644,7 @@ bool InstallationManager::install(const QString &fileName, GuessedValue<QString>
   int categoryID = 0;

   QString repository = "Nexus";

 

-  QString metaName = fileName.mid(0).append(".meta");

+  QString metaName = fileName + ".meta";

   if (QFile(metaName).exists()) {

     QSettings metaFile(metaName, QSettings::IniFormat);

     modID = metaFile.value("modID", 0).toInt();

@@ -697,8 +697,8 @@ bool InstallationManager::install(const QString &fileName, GuessedValue<QString>
       });

 

   foreach (IPluginInstaller *installer, m_Installers) {

-    // don't use inactive installers

-    if (!installer->isActive()) {

+    // don't use inactive installers (installer can't be null here but vc static code analysis thinks it could)

+    if ((installer == nullptr) || !installer->isActive()) {

       continue;

     }

 

diff --git a/src/main.cpp b/src/main.cpp
index bc89375e..a61e3150 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -69,6 +69,7 @@ along with Mod Organizer.  If not, see <http://www.gnu.org/licenses/>.
 #include <QSplashScreen>

 #include <QDirIterator>

 #include <QDesktopServices>

+#include <QLibraryInfo>

 #include <ShellAPI.h>

 #include <eh.h>

 #include <windows_error.h>

@@ -146,7 +147,7 @@ bool bootstrap()
 

 bool isNxmLink(const QString &link)

 {

-  return link.left(6).toLower() == "nxm://";

+  return link.startsWith("nxm://", Qt::CaseInsensitive);

 }

 

 static LONG WINAPI MyUnhandledExceptionFilter(struct _EXCEPTION_POINTERS *exceptionPtrs)

@@ -166,7 +167,6 @@ static LONG WINAPI MyUnhandledExceptionFilter(struct _EXCEPTION_POINTERS *except
   if (dbgDLL) {

     FuncMiniDumpWriteDump funcDump = (FuncMiniDumpWriteDump)::GetProcAddress(dbgDLL, "MiniDumpWriteDump");

     if (funcDump) {

-

       if (QMessageBox::question(nullptr, QObject::tr("Woops"),

                             QObject::tr("ModOrganizer has crashed! Should a diagnostic file be created? If you send me this file "

                                "(%1) to sherb@gmx.net, the bug is a lot more likely to be fixed. "

diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp
index 9e8d0c82..af654e2f 100644
--- a/src/mainwindow.cpp
+++ b/src/mainwindow.cpp
@@ -138,8 +138,6 @@ using namespace MOBase;
 using namespace MOShared;

 

 

-

-

 MainWindow::MainWindow(const QString &exeName

                        , QSettings &initSettings

                        , OrganizerCore &organizerCore

@@ -2172,7 +2170,6 @@ void MainWindow::removeMod_clicked()
       QString mods;

       QStringList modNames;

       foreach (QModelIndex idx, selection->selectedRows()) {

-//        QString name = ModInfo::getByIndex(m_ModListGroupProxy->mapToSource(idx).row())->name();

         QString name = idx.data().toString();

         if (!ModInfo::getByIndex(idx.data(Qt::UserRole + 1).toInt())->isRegular()) {

           continue;

@@ -3363,7 +3360,9 @@ void MainWindow::installTranslator(const QString &name)
   QTranslator *translator = new QTranslator(this);

   QString fileName = name + "_" + m_CurrentLanguage;

   if (!translator->load(fileName, qApp->applicationDirPath() + "/translations")) {

-    if ((m_CurrentLanguage != "en-US") && (m_CurrentLanguage != "en_US")) {

+    if ((m_CurrentLanguage != "en-US")

+        && (m_CurrentLanguage != "en_US")

+        && (m_CurrentLanguage != "en-GB")) {

       qDebug("localization file %s not found", qPrintable(fileName));

     } // we don't actually expect localization files for english

   }

diff --git a/src/plugincontainer.cpp b/src/plugincontainer.cpp
index bbcd3b6a..1b06d1a6 100644
--- a/src/plugincontainer.cpp
+++ b/src/plugincontainer.cpp
@@ -138,7 +138,8 @@ bool PluginContainer::registerPlugin(QObject *plugin, const QString &fileName)
     IPluginProxy *proxy = qobject_cast<IPluginProxy*>(plugin);

     if (verifyPlugin(proxy)) {

       bf::at_key<IPluginProxy>(m_Plugins).push_back(proxy);

-      QStringList pluginNames = proxy->pluginList(QCoreApplication::applicationDirPath() + "/" + ToQString(AppConfig::pluginPath()));

+      QStringList pluginNames = proxy->pluginList(

+            QCoreApplication::applicationDirPath() + "/" + ToQString(AppConfig::pluginPath()));

       foreach (const QString &pluginName, pluginNames) {

         try {

           QObject *proxiedPlugin = proxy->instantiate(pluginName);

@@ -146,11 +147,13 @@ bool PluginContainer::registerPlugin(QObject *plugin, const QString &fileName)
             if (registerPlugin(proxiedPlugin, pluginName)) {

               qDebug("loaded plugin \"%s\"", qPrintable(pluginName));

             } else {

-              qWarning("plugin \"%s\" failed to load", qPrintable(pluginName));

+              qWarning("plugin \"%s\" failed to load. If this plugin is for an older version of MO "

+                       "you have to update it or delete it if no update exists.",

+                       qPrintable(pluginName));

             }

           }

         } catch (const std::exception &e) {

-          reportError(QObject::tr("failed to init plugin %1: %2").arg(pluginName).arg(e.what()));

+          reportError(QObject::tr("failed to initialize plugin %1: %2").arg(pluginName).arg(e.what()));

         }

       }

       return true;

@@ -199,7 +202,7 @@ void PluginContainer::unloadPlugins()
   while (!m_PluginLoaders.empty()) {

     QPluginLoader *loader = m_PluginLoaders.back();

     m_PluginLoaders.pop_back();

-    if (!loader->unload()) {

+    if ((loader != nullptr) && !loader->unload()) {

       qDebug("failed to unload %s: %s", qPrintable(loader->fileName()), qPrintable(loader->errorString()));

     }

     delete loader;

diff --git a/src/qt.conf b/src/qt.conf
new file mode 100644
index 00000000..f834a22a
--- /dev/null
+++ b/src/qt.conf
@@ -0,0 +1,3 @@
+[Paths]

+Prefix=.

+Plugins=dlls

diff --git a/src/shared/directoryentry.cpp b/src/shared/directoryentry.cpp
index 5cf75752..9a864245 100644
--- a/src/shared/directoryentry.cpp
+++ b/src/shared/directoryentry.cpp
@@ -235,7 +235,8 @@ void FileEntry::addOrigin(int origin, FILETIME fileTime, const std::wstring &arc
     m_Origin = origin;

     m_FileTime = fileTime;

     m_Archive = archive;

-  } else if (m_Parent->getOriginByID(origin).getPriority() > m_Parent->getOriginByID(m_Origin).getPriority()) {

+  } else if ((m_Parent != nullptr)

+             && (m_Parent->getOriginByID(origin).getPriority() > m_Parent->getOriginByID(m_Origin).getPriority())) {

     if (std::find(m_Alternatives.begin(), m_Alternatives.end(), m_Origin) == m_Alternatives.end()) {

       m_Alternatives.push_back(m_Origin);

     }

@@ -253,7 +254,8 @@ void FileEntry::addOrigin(int origin, FILETIME fileTime, const std::wstring &arc
         // already an origin

         return;

       }

-      if (m_Parent->getOriginByID(*iter).getPriority() < m_Parent->getOriginByID(origin).getPriority()) {

+      if ((m_Parent != nullptr)

+          && (m_Parent->getOriginByID(*iter).getPriority() < m_Parent->getOriginByID(origin).getPriority())) {

         m_Alternatives.insert(iter, origin);

         found = true;

         break;

@@ -508,7 +510,7 @@ void DirectoryEntry::addFiles(FilesOrigin &origin, wchar_t *buffer, int bufferOf
 {

   WIN32_FIND_DATAW findData;

 

-  _snwprintf(buffer + bufferOffset, MAXPATH_UNICODE - bufferOffset, L"\\*");

+  _snwprintf_s(buffer + bufferOffset, MAXPATH_UNICODE - bufferOffset, _TRUNCATE, L"\\*");

 

   HANDLE searchHandle = nullptr;

 

diff --git a/src/shared/error_report.cpp b/src/shared/error_report.cpp
index c9277c1b..6d091630 100644
--- a/src/shared/error_report.cpp
+++ b/src/shared/error_report.cpp
@@ -46,8 +46,7 @@ void reportError(LPCWSTR format, ...)
   va_list argList;

   va_start(argList, format);

 

-

-  _vsnwprintf(buffer, 1024, format, argList);

+  _vsnwprintf_s(buffer, 1024, format, argList);

   va_end(argList);

 

   MessageBoxW(nullptr, buffer, L"Error", MB_OK | MB_ICONERROR);

diff --git a/src/shared/fallout3info.h b/src/shared/fallout3info.h
index 1f3a381d..6e6a7b85 100644
--- a/src/shared/fallout3info.h
+++ b/src/shared/fallout3info.h
@@ -43,38 +43,7 @@ public:
 

   virtual std::wstring getGameName() const { return L"Fallout 3"; }

   virtual std::wstring getGameShortName() const { return L"Fallout3"; }

-/*

-  virtual std::wstring getInvalidationBSA()

-  {

-    return L"Fallout - Invalidation.bsa";

-  }

-

-  virtual bool isInvalidationBSA(const std::wstring &bsaName)

-  {

-    static LPCWSTR invalidation[] = { L"Fallout - AI!.bsa", L"Fallout - Invalidation.bsa", nullptr };

-

-    for (int i = 0; invalidation[i] != nullptr; ++i) {

-      if (wcscmp(bsaName.c_str(), invalidation[i]) == 0) {

-        return true;

-      }

-    }

-    return false;

-  }

-

-  virtual std::vector<std::wstring> getVanillaBSAs()

-  {

-    return boost::assign::list_of (L"Fallout - Textures.bsa")

-        (L"Fallout - Meshes.bsa")

-        (L"Fallout - Voices.bsa")

-        (L"Fallout - Sound.bsa")

-        (L"Fallout - MenuVoices.bsa")

-        (L"Fallout - Misc.bsa");

-  }

 

-  virtual std::vector<std::wstring> getPrimaryPlugins()

-  {

-    return boost::assign::list_of(L"fallout3.esm");

-  }*/

   virtual std::vector<std::wstring> getDLCPlugins();

   virtual std::vector<std::wstring> getSavegameAttachmentExtensions();

 

diff --git a/src/shared/leaktrace.cpp b/src/shared/leaktrace.cpp
index 0d99b82e..73786c6b 100644
--- a/src/shared/leaktrace.cpp
+++ b/src/shared/leaktrace.cpp
@@ -34,7 +34,7 @@ static struct __TraceData {
              iter->second.size(), iter->first.toString().c_str());

       printf("Addresses: ");

       for (int i = 0; i < (std::min<int>)(5, iter->second.size()); ++i) {

-        printf("%p, ", iter->second[i]);

+        printf("%p, ", reinterpret_cast<void*>(iter->second[i]));

       }

       printf("\n");

     }

diff --git a/src/shared/stackdata.cpp b/src/shared/stackdata.cpp
index 2f8bfc4e..43acd90a 100644
--- a/src/shared/stackdata.cpp
+++ b/src/shared/stackdata.cpp
@@ -135,7 +135,6 @@ void StackData::initTrace() {
   }

 }

 

-#pragma warning( enable : 4748 )

 

 bool MOShared::operator==(const StackData &LHS, const StackData &RHS) {

   if (LHS.m_Count != RHS.m_Count) {

diff --git a/src/shared/util.cpp b/src/shared/util.cpp
index ceed3a34..2ce88eca 100644
--- a/src/shared/util.cpp
+++ b/src/shared/util.cpp
@@ -55,7 +55,10 @@ std::string ToString(const std::wstring &source, bool utf8)
 {

   std::string result;

   if (source.length() > 0) {

-    UINT codepage = utf8 ? CP_UTF8 : GetACP();

+    UINT codepage = CP_UTF8;

+    if (!utf8) {

+      codepage = AreFileApisANSI() ? GetACP() : GetOEMCP();

+    }

     int sizeRequired = ::WideCharToMultiByte(codepage, 0, &source[0], -1, nullptr, 0, nullptr, nullptr);

     if (sizeRequired == 0) {

       throw windows_error("failed to convert string to multibyte");

@@ -73,7 +76,10 @@ std::wstring ToWString(const std::string &source, bool utf8)
 {

   std::wstring result;

   if (source.length() > 0) {

-    UINT codepage = utf8 ? CP_UTF8 : GetACP();

+    UINT codepage = CP_UTF8;

+    if (!utf8) {

+      codepage = AreFileApisANSI() ? GetACP() : GetOEMCP();

+    }

     int sizeRequired = ::MultiByteToWideChar(codepage, 0, source.c_str(), source.length(), nullptr, 0);

     if (sizeRequired == 0) {

       throw windows_error("failed to convert string to wide character");

@@ -113,7 +119,7 @@ std::wstring ToLower(const std::wstring &text)
 

 VS_FIXEDFILEINFO GetFileVersion(const std::wstring &fileName)

 {

-  DWORD handle;

+  DWORD handle = 0UL;

   DWORD size = ::GetFileVersionInfoSizeW(fileName.c_str(), &handle);

   if (size == 0) {

     throw windows_error("failed to determine file version info size");

@@ -121,6 +127,7 @@ VS_FIXEDFILEINFO GetFileVersion(const std::wstring &fileName)
 

   void *buffer = new char[size];

   try {

+    handle = 0UL;

     if (!::GetFileVersionInfoW(fileName.c_str(), handle, size, buffer)) {

       throw windows_error("failed to determine file version info");

     }

diff --git a/src/spawn.cpp b/src/spawn.cpp
index c031b25e..b1127df3 100644
--- a/src/spawn.cpp
+++ b/src/spawn.cpp
@@ -61,7 +61,7 @@ static bool spawn(LPCWSTR binary, LPCWSTR arguments, LPCWSTR currentDirectory, b
     _snwprintf(commandLine, length, L"\"%ls\" %ls", binary, arguments);

   } else {

     commandLine = new wchar_t[length];

-    _snwprintf(commandLine, length, L"\"%ls\"", binary);

+    _snwprintf_s(commandLine, length, _TRUNCATE, L"\"%ls\"", binary);

   }

 

   QString moPath = QCoreApplication::applicationDirPath();

@@ -75,8 +75,8 @@ static bool spawn(LPCWSTR binary, LPCWSTR arguments, LPCWSTR currentDirectory, b
 

   {

     boost::scoped_array<TCHAR> newPath(new TCHAR[offset + moPath.length() + 2]);

-    _tcsncpy(newPath.get(), oldPath.get(), offset - 1);

-    newPath.get()[offset - 1] = L'\0';

+    _tcsncpy(newPath.get(), oldPath.get(), offset);

+    newPath.get()[offset] = '\0';

     _tcsncat(newPath.get(), TEXT(";"), 1);

     _tcsncat(newPath.get(), ToWString(QDir::toNativeSeparators(moPath)).c_str(), moPath.length());