moved helper stuff to spawn so it can reuse error handling

removed unused helper::init()
removed logging when deleting a credential that doesn't exist, happens all the time

7 files changed, 175 insertions, 166 deletions

diff --git a/src/envsecurity.cpp b/src/envsecurity.cpp
index 6e3fadbe..3b4cdcaa 100644
--- a/src/envsecurity.cpp
+++ b/src/envsecurity.cpp
@@ -275,7 +275,7 @@ std::vector<SecurityProduct> getSecurityProductsFromWMI()
     }
 
     if (prop.vt != VT_UI4 && prop.vt != VT_I4) {
-      log::error("productState is a {}, is not a VT_UI4", prop.vt);
+      log::error("productState is a {}, not a VT_UI4", prop.vt);
       return;
     }
 
diff --git a/src/helper.cpp b/src/helper.cpp
index 59a2d3d1..24446cb8 100644
--- a/src/helper.cpp
+++ b/src/helper.cpp
@@ -17,109 +17,4 @@ You should have received a copy of the GNU General Public License
 along with Mod Organizer.  If not, see <http://www.gnu.org/licenses/>.

 */

 

-#include "helper.h"

-#include "utility.h"

-#include <report.h>

-#include <LMCons.h>

-

-#define WIN32_LEAN_AND_MEAN

-#include <Windows.h>

-

-#include <QDir>

-#include <QApplication>

-

-using MOBase::reportError;

-

-

-namespace Helper {

-

-

-static bool helperExec(LPCWSTR moDirectory, LPCWSTR commandLine, BOOL async)

-{

-  wchar_t fileName[MAX_PATH];

-  _snwprintf(fileName, MAX_PATH, L"%ls\\helper.exe", moDirectory);

-

-  SHELLEXECUTEINFOW execInfo = {0};

-

-  execInfo.cbSize = sizeof(SHELLEXECUTEINFOW);

-  execInfo.fMask = SEE_MASK_NOCLOSEPROCESS;

-  execInfo.hwnd = nullptr;

-  execInfo.lpVerb = L"runas";

-  execInfo.lpFile = fileName;

-  execInfo.lpParameters = commandLine;

-  execInfo.lpDirectory = moDirectory;

-  execInfo.nShow = SW_SHOW;

-

-  ::ShellExecuteExW(&execInfo);

-

-  if (execInfo.hProcess == 0) {

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

-    return false;

-  }

-

-  if (async) {

-    return true;

-  }

-

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

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

-    return false;

-  }

-

-  DWORD exitCode;

-  GetExitCodeProcess(execInfo.hProcess, &exitCode);

-  return exitCode == NOERROR;

-}

-

-

-bool init(const std::wstring &moPath, const std::wstring &dataPath)

-{

-  DWORD userNameLen = UNLEN + 1;

-  wchar_t userName[UNLEN + 1];

-

-  if (!GetUserName(userName, &userNameLen)) {

-    reportError(QObject::tr("failed to determine account name"));

-    return false;

-  }

-  wchar_t *commandLine = new wchar_t[32768];

-

-  _snwprintf(commandLine, 32768, L"init \"%ls\" \"%ls\"",

-             dataPath.c_str(), userName);

-

-  bool res = helperExec(moPath.c_str(), commandLine, FALSE);

-  delete [] commandLine;

-

-  return res;

-}

-

-

-bool backdateBSAs(const std::wstring &moPath, const std::wstring &dataPath)

-{

-  wchar_t *commandLine = new wchar_t[32768];

-  _snwprintf(commandLine, 32768, L"backdateBSA \"%ls\"",

-             dataPath.c_str());

-

-  bool res = helperExec(moPath.c_str(), commandLine, FALSE);

-  delete [] commandLine;

-

-  return res;

-}

-

-

-bool adminLaunch(const std::wstring &moPath, const std::wstring &moFile, const std::wstring &workingDir)

-{

-  wchar_t *commandLine = new wchar_t[32768];

-  _snwprintf(commandLine, 32768, L"adminLaunch %d \"%ls\" \"%ls\"",

-             ::GetCurrentProcessId(),

-             moFile.c_str(),

-             workingDir.c_str()

-             );

-

-  bool res = helperExec(moPath.c_str(), commandLine, TRUE);

-  delete [] commandLine;

-

-  return res;

-}

-

-

-} // namespace

+// moved to spawn.cpp

diff --git a/src/helper.h b/src/helper.h
index f6667a84..335b8647 100644
--- a/src/helper.h
+++ b/src/helper.h
@@ -20,45 +20,7 @@ along with Mod Organizer.  If not, see <http://www.gnu.org/licenses/>.
 #ifndef HELPER_H

 #define HELPER_H

 

-

-#include <string>

-

-

-/**

- * @brief Convenience functions to work with the external helper program.

- *

- * The mo_helper program is used to make changes on the system that require administrative

- * rights, so that ModOrganizer itself can run without special privileges

- **/

-namespace Helper {

-

-/**

- * @brief initialise the specified directory for use with mod organizer.

- *

- * This will create all required sub-directories and give the user running ModOrganizer

- * write-access

- *

- * @param moPath absolute path to the ModOrganizer base directory

- * @return true on success

- **/

-bool init(const std::wstring &moPath, const std::wstring &dataPath);

-

-/**

- * @brief sets the last modified time for all .bsa-files in the target directory well into the past

- * @param moPath absolute path to the modOrganizer base directory

- * @param dataPath the path taht contains the .bsa-files, usually the data directory of the game

- **/

-bool backdateBSAs(const std::wstring &moPath, const std::wstring &dataPath);

-

-/**

- * @brief waits for the current process to exit and restarts it as an administrator

- * @param moPath absolute path to the modOrganizer base directory

- * @param moFile file name of modOrganizer

- * @param workingDir current working directory

- **/

-bool adminLaunch(const std::wstring &moPath, const std::wstring &moFile, const std::wstring &workingDir);

-

-}

-

+// all helper code moved to spawn.h and spawn.cpp

+#include "spawn.h"

 

 #endif // HELPER_H

diff --git a/src/settingsdialogworkarounds.cpp b/src/settingsdialogworkarounds.cpp
index 4d811e40..f89b021c 100644
--- a/src/settingsdialogworkarounds.cpp
+++ b/src/settingsdialogworkarounds.cpp
@@ -83,7 +83,7 @@ void WorkaroundsSettingsTab::on_bsaDateBtn_clicked()
   const auto* game = qApp->property("managed_game").value<MOBase::IPluginGame*>();
   QDir dir = game->dataDirectory();
 
-  Helper::backdateBSAs(qApp->applicationDirPath().toStdWString(),
+  helper::backdateBSAs(qApp->applicationDirPath().toStdWString(),
     dir.absolutePath().toStdWString());
 }
 
diff --git a/src/settingsutilities.cpp b/src/settingsutilities.cpp
index 6c99a602..db7c1818 100644
--- a/src/settingsutilities.cpp
+++ b/src/settingsutilities.cpp
@@ -224,16 +224,18 @@ bool deleteWindowsCredential(const QString& key)
 
   if (!CredDeleteW(credName.toStdWString().c_str(), CRED_TYPE_GENERIC, 0)) {
     const auto e = GetLastError();
+
+    // not an error if the key already doesn't exist, and don't log it because
+    // it happens all the time when the settings dialog is closed since it
+    // doesn't check first
     if (e == ERROR_NOT_FOUND) {
-      // not an error if the key already doesn't exist
-      log::debug("can't delete windows credential {}, doesn't exist", credName);
       return true;
-    } else {
-      log::error(
-        "failed to delete windows credential {}, {}",
-        credName, formatSystemMessage(e));
-      return false;
     }
+
+    log::error(
+      "failed to delete windows credential {}, {}",
+      credName, formatSystemMessage(e));
+    return false;
   }
 
   log::debug("deleted windows credential {}", credName);
@@ -269,7 +271,7 @@ bool addWindowsCredential(const QString& key, const QString& data)
     return false;
   }
 
-  log::debug("added windows credential {}", credName);
+  log::debug("set windows credential {}", credName);
 
   return true;
 }
diff --git a/src/spawn.cpp b/src/spawn.cpp
index 94737871..6c524681 100644
--- a/src/spawn.cpp
+++ b/src/spawn.cpp
@@ -64,7 +64,7 @@ std::wstring makeRightsDetails(const env::FileSecurity& fs)
   return s;

 }

 

-std::wstring makeDetails(const SpawnParameters& sp, DWORD code)

+QString makeDetails(const SpawnParameters& sp, DWORD code, const QString& more={})

 {

   std::wstring owner, rights;

 

@@ -109,7 +109,7 @@ std::wstring makeDetails(const SpawnParameters& sp, DWORD code)
   }

 

   std::wstring f =

-    L"Error {code} {codename}: {error}\n"

+    L"Error {code} {codename}{more}: {error}\n"

     L" . binary: '{bin}'\n"

     L" . owner: {owner}\n"

     L" . rights: {rights}\n"

@@ -122,9 +122,12 @@ std::wstring makeDetails(const SpawnParameters& sp, DWORD code)
     f += L"\n . usvfs x86:{x86_dll} x64:{x64_dll} proxy_x86:{x86_proxy} proxy_x64:{x64_proxy}";

   }

 

-  return fmt::format(f,

+  const std::wstring wmore = (more.isEmpty() ? L"" : (", " + more).toStdWString());

+

+  const auto s = fmt::format(f,

     fmt::arg(L"code", code),

     fmt::arg(L"codename", errorCodeName(code)),

+    fmt::arg(L"more", wmore),

     fmt::arg(L"bin", QDir::toNativeSeparators(sp.binary.absoluteFilePath()).toStdWString()),

     fmt::arg(L"owner", owner),

     fmt::arg(L"rights", rights),

@@ -140,6 +143,8 @@ std::wstring makeDetails(const SpawnParameters& sp, DWORD code)
     fmt::arg(L"x86_proxy", usvfs_x86_proxy),

     fmt::arg(L"x64_proxy", usvfs_x64_proxy),

     fmt::arg(L"elevated", elevated));

+

+  return QString::fromStdWString(s);

 }

 

 QString makeContent(const SpawnParameters& sp, DWORD code)

@@ -198,7 +203,7 @@ QMessageBox::StandardButton startSteamFailed(
   return MOBase::TaskDialog(parent, QObject::tr("Cannot start Steam"))

     .main(QObject::tr("Cannot start Steam"))

     .content(makeContent(sp, e))

-    .details(QString::fromStdWString(details))

+    .details(details)

     .button({

     QObject::tr("Continue without starting Steam"),

     QObject::tr("The program may fail to launch."),

@@ -211,7 +216,7 @@ QMessageBox::StandardButton startSteamFailed(
 

 void spawnFailed(const SpawnParameters& sp, DWORD code)

 {

-  const auto details = QString::fromStdWString(makeDetails(sp, code));

+  const auto details = makeDetails(sp, code);

   log::error("{}", details);

 

   const auto title = QObject::tr("Cannot launch program");

@@ -231,10 +236,38 @@ void spawnFailed(const SpawnParameters& sp, DWORD code)
     .exec();

 }

 

+void helperFailed(

+  DWORD code, const QString& why, const std::wstring& binary,

+  const std::wstring& cwd, const std::wstring& args)

+{

+  SpawnParameters sp;

+  sp.binary = QString::fromStdWString(binary);

+  sp.currentDirectory.setPath(QString::fromStdWString(cwd));

+  sp.arguments = QString::fromStdWString(args);

+

+  const auto details = makeDetails(sp, code, "in " + why);

+  log::error("{}", details);

+

+  const auto title = QObject::tr("Cannot launch helper");

+

+  const auto mainText = QObject::tr("Cannot start %1")

+    .arg(sp.binary.fileName());

+

+  QWidget *window = qApp->activeWindow();

+  if ((window != nullptr) && (!window->isVisible())) {

+    window = nullptr;

+  }

+

+  MOBase::TaskDialog(window, title)

+    .main(mainText)

+    .content(makeContent(sp, code))

+    .details(details)

+    .exec();

+}

+

 bool confirmRestartAsAdmin(const SpawnParameters& sp)

 {

-  const auto details = QString::fromStdWString(

-    makeDetails(sp, ERROR_ELEVATION_REQUIRED));

+  const auto details = makeDetails(sp, ERROR_ELEVATION_REQUIRED);

 

   log::error("{}", details);

 

@@ -370,18 +403,18 @@ bool restartAsAdmin()
     cwd[0] = L'\0';

   }

 

-  if (!Helper::adminLaunch(

+  if (!helper::adminLaunch(

     qApp->applicationDirPath().toStdWString(),

     qApp->applicationFilePath().toStdWString(),

     std::wstring(cwd)))

   {

-    // todo

     log::error("admin launch failed");

     return false;

   }

 

   log::debug("exiting MO");

   qApp->exit(0);

+

   return true;

 }

 

@@ -728,4 +761,96 @@ HANDLE startBinary(QWidget* parent, const SpawnParameters& sp)
   }

 }

 

-} // namespace
\ No newline at end of file
+} // namespace

+

+

+

+namespace helper

+{

+

+bool helperExec(

+  const std::wstring& moDirectory, const std::wstring& commandLine, BOOL async)

+{

+  const std::wstring fileName = moDirectory + L"\\helper.exe";

+

+  env::HandlePtr process;

+

+  {

+    SHELLEXECUTEINFOW execInfo = {};

+

+    ULONG flags = SEE_MASK_FLAG_NO_UI ;

+    if (!async)

+      flags |= SEE_MASK_NOCLOSEPROCESS;

+

+    execInfo.cbSize = sizeof(SHELLEXECUTEINFOW);

+    execInfo.fMask = flags;

+    execInfo.hwnd = 0;

+    execInfo.lpVerb = L"runas";

+    execInfo.lpFile = fileName.c_str();

+    execInfo.lpParameters = commandLine.c_str();

+    execInfo.lpDirectory = moDirectory.c_str();

+    execInfo.nShow = SW_SHOW;

+

+    if (!::ShellExecuteExW(&execInfo) && execInfo.hProcess == 0) {

+      const auto e = GetLastError();

+

+      spawn::dialogs::helperFailed(

+        e, "ShellExecuteExW()", fileName, moDirectory, commandLine);

+

+      return false;

+    }

+

+    if (async) {

+      return true;

+    }

+

+    process.reset(execInfo.hProcess);

+  }

+

+  const auto r = ::WaitForSingleObject(process.get(), INFINITE);

+

+  if (r != WAIT_OBJECT_0) {

+    // for WAIT_ABANDONED, the documentation doesn't mention that GetLastError()

+    // returns something meaningful, but code ERROR_ABANDONED_WAIT_0 exists, so

+    // use that instead

+    const auto code = (r == WAIT_ABANDONED ?

+      ERROR_ABANDONED_WAIT_0 : GetLastError());

+

+    spawn::dialogs::helperFailed(

+      code, "WaitForSingleObject()", fileName, moDirectory, commandLine);

+

+    return false;

+  }

+

+  DWORD exitCode = 0;

+  if (!GetExitCodeProcess(process.get(), &exitCode)) {

+    const auto e = GetLastError();

+

+    spawn::dialogs::helperFailed(

+      e, "GetExitCodeProcess()", fileName, moDirectory, commandLine);

+

+    return false;

+  }

+

+  return (exitCode == 0);

+}

+

+bool backdateBSAs(const std::wstring &moPath, const std::wstring &dataPath)

+{

+  const std::wstring commandLine = fmt::format(

+    L"backdateBSA \"{}\"", dataPath);

+

+  return helperExec(moPath, commandLine, FALSE);

+}

+

+

+bool adminLaunch(const std::wstring &moPath, const std::wstring &moFile, const std::wstring &workingDir)

+{

+  const std::wstring commandLine = fmt::format(

+    L"adminLaunch {} \"{}\" \"{}\"",

+    ::GetCurrentProcessId(), moFile, workingDir);

+

+  return helperExec(moPath, commandLine, true);

+}

+

+} // namespace

diff --git a/src/spawn.h b/src/spawn.h
index 9398b6cc..da626329 100644
--- a/src/spawn.h
+++ b/src/spawn.h
@@ -71,5 +71,30 @@ HANDLE startBinary(QWidget* parent, const SpawnParameters& sp);
 

 } // namespace

 

+

+// convenience functions to work with the external helper program, which is used

+// to make changes on the system that require administrative rights, so that

+// ModOrganizer itself can run without special privileges

+//

+namespace helper

+{

+

+/**

+* @brief sets the last modified time for all .bsa-files in the target directory well into the past

+* @param moPath absolute path to the modOrganizer base directory

+* @param dataPath the path taht contains the .bsa-files, usually the data directory of the game

+**/

+bool backdateBSAs(const std::wstring &moPath, const std::wstring &dataPath);

+

+/**

+* @brief waits for the current process to exit and restarts it as an administrator

+* @param moPath absolute path to the modOrganizer base directory

+* @param moFile file name of modOrganizer

+* @param workingDir current working directory

+**/

+bool adminLaunch(const std::wstring &moPath, const std::wstring &moFile, const std::wstring &workingDir);

+

+} // namespace

+

 #endif // SPAWN_H