moved findJavaInstallation() and getFileExecutionContext() to spawn

fixed env::get() returning garbage after value

7 files changed, 349 insertions, 227 deletions

diff --git a/src/editexecutablesdialog.cpp b/src/editexecutablesdialog.cpp
index 8535b7a7..32b31357 100644
--- a/src/editexecutablesdialog.cpp
+++ b/src/editexecutablesdialog.cpp
@@ -24,6 +24,7 @@ along with Mod Organizer.  If not, see <http://www.gnu.org/licenses/>.
 #include "modlist.h"
 #include "forcedloaddialog.h"
 #include "organizercore.h"
+#include "spawn.h"
 
 #include <QMessageBox>
 #include <Shellapi.h>
@@ -800,7 +801,7 @@ QFileInfo EditExecutablesDialog::browseBinary(const QString& initial)
 
 void EditExecutablesDialog::setJarBinary(const QFileInfo& binary)
 {
-  auto java = OrganizerCore::findJavaInstallation(binary.absoluteFilePath());
+  auto java = spawn::findJavaInstallation(binary.absoluteFilePath());
 
   if (java.isEmpty()) {
     QMessageBox::information(
diff --git a/src/env.cpp b/src/env.cpp
index 78b5dc96..507607d1 100644
--- a/src/env.cpp
+++ b/src/env.cpp
@@ -190,19 +190,36 @@ QString setPath(const QString& s)
 
 QString get(const QString& name)
 {
-  std::wstring s(4000, L' ');
+  std::size_t bufferSize = 4000;
+  auto buffer = std::make_unique<wchar_t[]>(bufferSize);
 
   DWORD realSize = ::GetEnvironmentVariableW(
-    name.toStdWString().c_str(), s.data(), static_cast<DWORD>(s.size()));
+    name.toStdWString().c_str(),
+    buffer.get(), static_cast<DWORD>(bufferSize));
 
-  if (realSize > s.size()) {
-    s.resize(realSize);
+  if (realSize > bufferSize) {
+    bufferSize = realSize;
+    buffer = std::make_unique<wchar_t[]>(bufferSize);
 
-    ::GetEnvironmentVariableW(
-      name.toStdWString().c_str(), s.data(), static_cast<DWORD>(s.size()));
+    realSize = ::GetEnvironmentVariableW(
+      name.toStdWString().c_str(),
+      buffer.get(), static_cast<DWORD>(bufferSize));
   }
 
-  return QString::fromStdWString(s);
+  if (realSize == 0) {
+    const auto e = ::GetLastError();
+
+    // don't log if not found
+    if (e != ERROR_ENVVAR_NOT_FOUND) {
+      log::error(
+        "failed to get environment variable '{}', {}",
+        name, formatSystemMessage(e));
+    }
+
+    return {};
+  }
+
+  return QString::fromWCharArray(buffer.get(), realSize);
 }
 
 QString set(const QString& n, const QString& v)
diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp
index 2474fdc0..0181a335 100644
--- a/src/mainwindow.cpp
+++ b/src/mainwindow.cpp
@@ -5295,43 +5295,42 @@ void MainWindow::addAsExecutable()
     return;
   }
 
-  using FileExecutionTypes = OrganizerCore::FileExecutionTypes;
+  const QFileInfo target(m_ContextItem->data(0, Qt::UserRole).toString());
+  const auto fec = spawn::getFileExecutionContext(this, target);
 
-  QFileInfo targetInfo(m_ContextItem->data(0, Qt::UserRole).toString());
-  QFileInfo binaryInfo;
-  QString arguments;
-  FileExecutionTypes type;
-
-  if (!OrganizerCore::getFileExecutionContext(this, targetInfo, binaryInfo, arguments, type)) {
-    return;
-  }
-
-  switch (type)
+  switch (fec.type)
   {
-    case FileExecutionTypes::Executable: {
-        QString name = QInputDialog::getText(this, tr("Enter Name"),
-              tr("Please enter a name for the executable"), QLineEdit::Normal,
-              targetInfo.completeBaseName());
-
-        if (!name.isEmpty()) {
-          //Note: If this already exists, you'll lose custom settings
-          m_OrganizerCore.executablesList()->setExecutable(Executable()
-            .title(name)
-            .binaryInfo(binaryInfo)
-            .arguments(arguments)
-            .workingDirectory(targetInfo.absolutePath()));
+    case spawn::FileExecutionTypes::Executable:
+    {
+      const QString name = QInputDialog::getText(
+        this, tr("Enter Name"),
+        tr("Enter a name for the executable"),
+        QLineEdit::Normal,
+        target.completeBaseName());
 
-          refreshExecutablesList();
-        }
+      if (!name.isEmpty()) {
+        //Note: If this already exists, you'll lose custom settings
+        m_OrganizerCore.executablesList()->setExecutable(Executable()
+          .title(name)
+          .binaryInfo(fec.binary)
+          .arguments(fec.arguments)
+          .workingDirectory(target.absolutePath()));
 
-        break;
+        refreshExecutablesList();
       }
 
-    case FileExecutionTypes::Other:  // fall-through
-    default: {
-        QMessageBox::information(this, tr("Not an executable"), tr("This is not a recognized executable."));
-        break;
-      }
+      break;
+    }
+
+    case spawn::FileExecutionTypes::Other:  // fall-through
+    default:
+    {
+      QMessageBox::information(
+        this, tr("Not an executable"),
+        tr("This is not a recognized executable."));
+
+      break;
+    }
   }
 }
 
@@ -5458,7 +5457,8 @@ void MainWindow::openDataFile()
     return;
   }
 
-  QFileInfo targetInfo(m_ContextItem->data(0, Qt::UserRole).toString());
+  const QString path = m_ContextItem->data(0, Qt::UserRole).toString();
+  const QFileInfo targetInfo(path);
   m_OrganizerCore.runFile(this, targetInfo);
 }
 
diff --git a/src/organizercore.cpp b/src/organizercore.cpp
index 6bf2c290..78f9517c 100644
--- a/src/organizercore.cpp
+++ b/src/organizercore.cpp
@@ -976,76 +976,6 @@ QStringList OrganizerCore::modsSortedByProfilePriority() const
   return res;
 }
 
-QString OrganizerCore::findJavaInstallation(const QString& jarFile)
-{
-  if (!jarFile.isEmpty()) {
-    // try to find java automatically based on the given jar file
-    std::wstring jarFileW = jarFile.toStdWString();
-
-    WCHAR buffer[MAX_PATH];
-    if (::FindExecutableW(jarFileW.c_str(), nullptr, buffer) > (HINSTANCE)32) {
-      DWORD binaryType = 0UL;
-      if (!::GetBinaryTypeW(buffer, &binaryType)) {
-        log::debug(
-          "failed to determine binary type of \"{}\": {}",
-          QString::fromWCharArray(buffer), ::GetLastError());
-      } else if (binaryType == SCS_32BIT_BINARY || binaryType == SCS_64BIT_BINARY) {
-        return QString::fromWCharArray(buffer);
-      }
-    }
-  }
-
-  // second attempt: look to the registry
-  QSettings reg("HKEY_LOCAL_MACHINE\\Software\\JavaSoft\\Java Runtime Environment", QSettings::NativeFormat);
-  if (reg.contains("CurrentVersion")) {
-    QString currentVersion = reg.value("CurrentVersion").toString();
-    return reg.value(QString("%1/JavaHome").arg(currentVersion)).toString().append("\\bin\\javaw.exe");
-  }
-
-  // not found
-  return {};
-}
-
-bool OrganizerCore::getFileExecutionContext(
-  QWidget* parent, const QFileInfo &targetInfo,
-  QFileInfo &binaryInfo, QString &arguments, FileExecutionTypes& type)
-{
-  QString extension = targetInfo.suffix();
-  if ((extension.compare("cmd", Qt::CaseInsensitive) == 0) ||
-    (extension.compare("com", Qt::CaseInsensitive) == 0) ||
-    (extension.compare("bat", Qt::CaseInsensitive) == 0)) {
-    binaryInfo = QFileInfo("C:\\Windows\\System32\\cmd.exe");
-    arguments = QString("/C \"%1\"").arg(QDir::toNativeSeparators(targetInfo.absoluteFilePath()));
-    type = FileExecutionTypes::Executable;
-    return true;
-  } else if (extension.compare("exe", Qt::CaseInsensitive) == 0) {
-    binaryInfo = targetInfo;
-    type = FileExecutionTypes::Executable;
-    return true;
-  } else if (extension.compare("jar", Qt::CaseInsensitive) == 0) {
-    auto java = findJavaInstallation(targetInfo.absoluteFilePath());
-
-    if (java.isEmpty()) {
-      java = QFileDialog::getOpenFileName(
-        parent, QObject::tr("Select binary"),
-        QString(), QObject::tr("Binary") + " (*.exe)");
-    }
-
-    if (java.isEmpty()) {
-      return false;
-    }
-
-    binaryInfo = QFileInfo(java);
-    arguments = QString("-jar \"%1\"").arg(QDir::toNativeSeparators(targetInfo.absoluteFilePath()));
-    type = FileExecutionTypes::Executable;
-
-    return true;
-  } else {
-    type = FileExecutionTypes::Other;
-    return true;
-  }
-}
-
 bool OrganizerCore::previewFileWithAlternatives(
   QWidget* parent, QString fileName, int selectedOrigin)
 {
@@ -1177,35 +1107,23 @@ bool OrganizerCore::previewFile(
 bool OrganizerCore::runFile(
   QWidget* parent, const QFileInfo& targetInfo)
 {
-  QFileInfo binaryInfo;
-  QString arguments;
-  FileExecutionTypes type;
-
-  if (!getFileExecutionContext(parent, targetInfo, binaryInfo, arguments, type)) {
-    return false;
-  }
+  const auto fec = spawn::getFileExecutionContext(parent, targetInfo);
 
-  switch (type)
+  switch (fec.type)
   {
-    case FileExecutionTypes::Executable: {
-      runExecutableFile(
-        binaryInfo, arguments, currentProfile()->name(),
-        targetInfo.absolutePath());
-
+    case spawn::FileExecutionTypes::Executable:
+    {
+      runExecutableFile(fec.binary, fec.arguments, targetInfo.absoluteDir());
       return true;
     }
 
-    case FileExecutionTypes::Other: {
-      ::ShellExecuteW(nullptr, L"open",
-        ToWString(targetInfo.absoluteFilePath()).c_str(),
-        nullptr, nullptr, SW_SHOWNORMAL);
-
-      return true;
+    case spawn::FileExecutionTypes::Other:  // fall-through
+    default:
+    {
+      const auto r = shell::Open(targetInfo.absoluteFilePath());
+      return r.success();
     }
   }
-
-  // nop
-  return false;
 }
 
 bool OrganizerCore::runExecutableFile(
@@ -1281,6 +1199,87 @@ bool OrganizerCore::runShortcut(const MOShortcut& shortcut)
   return runExecutable(exe, false);
 }
 
+HANDLE OrganizerCore::runExecutableOrExecutableFile(
+  const QString &executable, const QStringList &args, const QString &cwd,
+  const QString &profile, const QString &forcedCustomOverwrite,
+  bool ignoreCustomOverwrite)
+{
+  QString profileName = profile;
+  if (profile == "") {
+    if (m_CurrentProfile != nullptr) {
+      profileName = m_CurrentProfile->name();
+    } else {
+      throw MyException(tr("No profile set"));
+    }
+  }
+
+  QFileInfo binary;
+  QString arguments = args.join(" ");
+  QString currentDirectory = cwd;
+  QString steamAppID;
+  QString customOverwrite;
+  QList<ExecutableForcedLoadSetting> forcedLibraries;
+
+  if (executable.contains('\\') || executable.contains('/')) {
+    // file path
+
+    binary = QFileInfo(executable);
+    if (binary.isRelative()) {
+      // relative path, should be relative to game directory
+      binary = managedGame()->gameDirectory().absoluteFilePath(executable);
+    }
+
+    if (currentDirectory == "") {
+      currentDirectory = binary.absolutePath();
+    }
+
+    try {
+      const Executable& exe = m_ExecutablesList.getByBinary(binary);
+      steamAppID = exe.steamAppID();
+      customOverwrite = m_CurrentProfile->setting("custom_overwrites", exe.title()).toString();
+      if (m_CurrentProfile->forcedLibrariesEnabled(exe.title())) {
+        forcedLibraries = m_CurrentProfile->determineForcedLibraries(exe.title());
+      }
+    } catch (const std::runtime_error &) {
+      // nop
+    }
+  } else {
+    // only a file name, search executables list
+    try {
+      const Executable &exe = m_ExecutablesList.get(executable);
+      steamAppID = exe.steamAppID();
+      customOverwrite = m_CurrentProfile->setting("custom_overwrites", exe.title()).toString();
+      if (m_CurrentProfile->forcedLibrariesEnabled(exe.title())) {
+        forcedLibraries = m_CurrentProfile->determineForcedLibraries(exe.title());
+      }
+      if (arguments == "") {
+        arguments = exe.arguments();
+      }
+      binary = exe.binaryInfo();
+      if (currentDirectory == "") {
+        currentDirectory = exe.workingDirectory();
+      }
+    } catch (const std::runtime_error &) {
+      log::warn("\"{}\" not set up as executable", executable);
+      binary = QFileInfo(executable);
+    }
+  }
+
+  if (!forcedCustomOverwrite.isEmpty())
+    customOverwrite = forcedCustomOverwrite;
+
+  if (ignoreCustomOverwrite)
+    customOverwrite.clear();
+
+  return spawnAndWait(binary,
+    arguments,
+    profileName,
+    currentDirectory,
+    steamAppID,
+    customOverwrite,
+    forcedLibraries);
+}
+
 HANDLE OrganizerCore::spawnAndWait(
   const QFileInfo &binary, const QString &arguments, const QString &profileName,
   const QDir &currentDirectory, const QString &steamAppID,
@@ -1362,87 +1361,6 @@ HANDLE OrganizerCore::spawnAndWait(
   return handle;
 }
 
-HANDLE OrganizerCore::runExecutableOrExecutableFile(
-  const QString &executable, const QStringList &args, const QString &cwd,
-  const QString &profile, const QString &forcedCustomOverwrite,
-  bool ignoreCustomOverwrite)
-{
-  QString profileName = profile;
-  if (profile == "") {
-    if (m_CurrentProfile != nullptr) {
-      profileName = m_CurrentProfile->name();
-    } else {
-      throw MyException(tr("No profile set"));
-    }
-  }
-
-  QFileInfo binary;
-  QString arguments = args.join(" ");
-  QString currentDirectory = cwd;
-  QString steamAppID;
-  QString customOverwrite;
-  QList<ExecutableForcedLoadSetting> forcedLibraries;
-
-  if (executable.contains('\\') || executable.contains('/')) {
-    // file path
-
-    binary = QFileInfo(executable);
-    if (binary.isRelative()) {
-      // relative path, should be relative to game directory
-      binary = managedGame()->gameDirectory().absoluteFilePath(executable);
-    }
-
-    if (currentDirectory == "") {
-      currentDirectory = binary.absolutePath();
-    }
-
-    try {
-      const Executable& exe = m_ExecutablesList.getByBinary(binary);
-      steamAppID = exe.steamAppID();
-      customOverwrite = m_CurrentProfile->setting("custom_overwrites", exe.title()).toString();
-      if (m_CurrentProfile->forcedLibrariesEnabled(exe.title())) {
-        forcedLibraries = m_CurrentProfile->determineForcedLibraries(exe.title());
-      }
-    } catch (const std::runtime_error &) {
-      // nop
-    }
-  } else {
-    // only a file name, search executables list
-    try {
-      const Executable &exe = m_ExecutablesList.get(executable);
-      steamAppID = exe.steamAppID();
-      customOverwrite = m_CurrentProfile->setting("custom_overwrites", exe.title()).toString();
-      if (m_CurrentProfile->forcedLibrariesEnabled(exe.title())) {
-        forcedLibraries = m_CurrentProfile->determineForcedLibraries(exe.title());
-      }
-      if (arguments == "") {
-        arguments = exe.arguments();
-      }
-      binary = exe.binaryInfo();
-      if (currentDirectory == "") {
-        currentDirectory = exe.workingDirectory();
-      }
-    } catch (const std::runtime_error &) {
-      log::warn("\"{}\" not set up as executable", executable);
-      binary = QFileInfo(executable);
-    }
-  }
-
-  if (!forcedCustomOverwrite.isEmpty())
-    customOverwrite = forcedCustomOverwrite;
-
-  if (ignoreCustomOverwrite)
-    customOverwrite.clear();
-
-  return spawnAndWait(binary,
-                           arguments,
-                           profileName,
-                           currentDirectory,
-                           steamAppID,
-                           customOverwrite,
-                           forcedLibraries);
-}
-
 bool OrganizerCore::waitForApplication(HANDLE handle, LPDWORD exitCode)
 {
   if (!Settings::instance().interface().lockGUI())
diff --git a/src/organizercore.h b/src/organizercore.h
index fa05a20f..f802c8cb 100644
--- a/src/organizercore.h
+++ b/src/organizercore.h
@@ -91,12 +91,6 @@ private:
   typedef boost::signals2::signal<void (const QString&)> SignalModInstalled;

 

 public:

-  enum class FileExecutionTypes

-  {

-    Executable = 1,

-    Other = 2

-  };

-

   static bool isNxmLink(const QString &link) { return link.startsWith("nxm://", Qt::CaseInsensitive); }

 

   OrganizerCore(Settings &settings);

@@ -152,12 +146,6 @@ public:
   void doAfterLogin(const std::function<void()> &function) { m_PostLoginTasks.append(function); }

   void loggedInAction(QWidget* parent, std::function<void ()> f);

 

-  static QString findJavaInstallation(const QString& jarFile={});

-

-  static bool getFileExecutionContext(

-    QWidget* parent,  const QFileInfo &targetInfo,

-    QFileInfo &binaryInfo, QString &arguments, FileExecutionTypes& type);

-

   bool previewFileWithAlternatives(QWidget* parent, QString filename, int selectedOrigin=-1);

   bool previewFile(QWidget* parent, const QString& originName, const QString& path);

 

diff --git a/src/spawn.cpp b/src/spawn.cpp
index 3c7d64ce..dd93bfaa 100644
--- a/src/spawn.cpp
+++ b/src/spawn.cpp
@@ -901,11 +901,11 @@ SpawnedProcess Spawner::spawn(
     return {INVALID_HANDLE_VALUE, sp};

   }

 

-  if (!spawn::checkEnvironment(parent, sp)) {

+  if (!checkEnvironment(parent, sp)) {

     return {INVALID_HANDLE_VALUE, sp};

   }

 

-  if (!spawn::checkBlacklist(parent, sp, settings)) {

+  if (!checkBlacklist(parent, sp, settings)) {

     return {INVALID_HANDLE_VALUE, sp};

   }

 

@@ -914,6 +914,185 @@ SpawnedProcess Spawner::spawn(
   return {startBinary(parent, sp), sp};

 }

 

+

+QString getExecutableForJarFile(const QString& jarFile)

+{

+  const std::wstring jarFileW = jarFile.toStdWString();

+

+  WCHAR buffer[MAX_PATH];

+

+  const auto hinst = ::FindExecutableW(jarFileW.c_str(), nullptr, buffer);

+  const auto r = static_cast<int>(reinterpret_cast<std::uintptr_t>(hinst));

+

+  // anything <= 32 signals failure

+  if (r <= 32) {

+    log::warn(

+      "failed to find executable associated with file '{}', {}",

+      jarFile, shell::formatError(r));

+

+    return {};

+  }

+

+  DWORD binaryType = 0;

+

+  if (!::GetBinaryTypeW(buffer, &binaryType)) {

+    const auto e = ::GetLastError();

+

+    log::warn(

+      "failed to determine binary type of '{}', {}",

+      QString::fromWCharArray(buffer), formatSystemMessage(e));

+

+    return {};

+  }

+

+  if (binaryType != SCS_32BIT_BINARY && binaryType != SCS_64BIT_BINARY) {

+    log::warn(

+      "unexpected binary type {} for file '{}'",

+      binaryType, QString::fromWCharArray(buffer));

+

+    return {};

+  }

+

+  return QString::fromWCharArray(buffer);

+}

+

+QString getJavaHome()

+{

+  const QString key = "HKEY_LOCAL_MACHINE\\Software\\JavaSoft\\Java Runtime Environment";

+  const QString value = "CurrentVersion";

+

+  QSettings reg(key, QSettings::NativeFormat);

+

+  if (!reg.contains(value)) {

+    log::warn("key '{}\\{}' doesn't exist", key, value);

+    return {};

+  }

+

+  const QString currentVersion = reg.value("CurrentVersion").toString();

+  const QString javaHome = QString("%1/JavaHome").arg(currentVersion);

+

+  if (!reg.contains(javaHome)) {

+    log::warn(

+      "java version '{}' was found at '{}\\{}', but '{}\\{}' doesn't exist",

+      currentVersion, key, value, key, javaHome);

+

+    return {};

+  }

+

+  const auto path = reg.value(javaHome).toString();

+  return path + "\\bin\\javaw.exe";

+}

+

+QString findJavaInstallation(const QString& jarFile)

+{

+  // try to find java automatically based on the given jar file

+  if (!jarFile.isEmpty()) {

+    const auto s = getExecutableForJarFile(jarFile);

+    if (!s.isEmpty()) {

+      return s;

+    }

+  }

+

+  // second attempt: look to the registry

+  const auto s = getJavaHome();

+  if (!s.isEmpty()) {

+    return s;

+  }

+

+  // not found

+  return {};

+}

+

+bool isBatchFile(const QFileInfo& target)

+{

+  const auto batchExtensions = {"cmd", "bat"};

+

+  const QString extension = target.suffix();

+  for (auto&& e : batchExtensions) {

+    if (extension.compare(e, Qt::CaseInsensitive) == 0) {

+      return true;

+    }

+  }

+

+  return false;

+}

+

+bool isExeFile(const QFileInfo& target)

+{

+  return (target.suffix().compare("exe", Qt::CaseInsensitive) == 0);

+}

+

+bool isJavaFile(const QFileInfo& target)

+{

+  return (target.suffix().compare("jar", Qt::CaseInsensitive) == 0);

+}

+

+QFileInfo getCmdPath()

+{

+  const auto p = env::get("COMSPEC2");

+  if (!p.isEmpty()) {

+    return p;

+  }

+

+  QString systemDirectory;

+

+  const std::size_t buffer_size = 1000;

+  wchar_t buffer[buffer_size + 1] = {};

+

+  const auto length = ::GetSystemDirectoryW(buffer, buffer_size);

+  if (length != 0) {

+    systemDirectory = QString::fromWCharArray(buffer, length);

+

+    if (!systemDirectory.endsWith("\\")) {

+      systemDirectory += "\\";

+    }

+  } else {

+    systemDirectory = "C:\\Windows\\System32\\";

+  }

+

+  return systemDirectory + "cmd.exe";

+}

+

+FileExecutionContext getFileExecutionContext(

+  QWidget* parent, const QFileInfo& target)

+{

+  if (isExeFile(target)) {

+    return {

+      target,

+      "",

+      FileExecutionTypes::Executable

+    };

+  }

+

+  if (isBatchFile(target)) {

+    return {

+      getCmdPath(),

+      QString("/C \"%1\"").arg(QDir::toNativeSeparators(target.absoluteFilePath())),

+      FileExecutionTypes::Executable

+    };

+  }

+

+  if (isJavaFile(target)) {

+    auto java = findJavaInstallation(target.absoluteFilePath());

+

+    if (java.isEmpty()) {

+      java = QFileDialog::getOpenFileName(

+        parent, QObject::tr("Select binary"),

+        QString(), QObject::tr("Binary") + " (*.exe)");

+    }

+

+    if (!java.isEmpty()) {

+      return {

+        QFileInfo(java),

+        QString("-jar \"%1\"").arg(QDir::toNativeSeparators(target.absoluteFilePath())),

+        FileExecutionTypes::Executable

+      };

+    }

+  }

+

+  return {{}, {}, FileExecutionTypes::Other};

+}

+

 } // namespace

 

 

diff --git a/src/spawn.h b/src/spawn.h
index d2853cd5..866e1795 100644
--- a/src/spawn.h
+++ b/src/spawn.h
@@ -103,6 +103,25 @@ public:
 private:

 };

 

+

+enum class FileExecutionTypes

+{

+  Executable = 1,

+  Other

+};

+

+struct FileExecutionContext

+{

+  QFileInfo binary;

+  QString arguments;

+  FileExecutionTypes type;

+};

+

+QString findJavaInstallation(const QString& jarFile);

+

+FileExecutionContext getFileExecutionContext(

+  QWidget* parent, const QFileInfo& target);

+

 } // namespace