review feedback

fabric-installer-native-bootstrap.vcxproj

@@ -261,12 +261,14 @@
     </Manifest>
   </ItemDefinitionGroup>
   <ItemGroup>
+    <ClCompile Include="src\Architecture.cpp" />
     <ClCompile Include="src\Bootstrap.cpp" />
     <ClCompile Include="src\Logger.cpp" />
     <ClCompile Include="src\main.cpp" />
     <ClCompile Include="src\SystemHelper.cpp" />
   </ItemGroup>
   <ItemGroup>
+    <ClInclude Include="src\Architecture.h" />
     <ClInclude Include="src\Bootstrap.h" />
     <ClInclude Include="src\ISystemHelper.h" />
     <ClInclude Include="resource.h" />

src/Bootstrap.cpp

@@ -39,8 +39,8 @@ void Bootstrap::launch() {
 }
 
 bool Bootstrap::launchMinecraftLauncher() {
-	const HostArchitecture::Value hostArch = systemHelper.getHostArchitecture();
-	logger.log(L"Host archiecture: " + HostArchitecture::AsString(hostArch));
+	const Architecture::Value hostArch = systemHelper.getHostArchitecture();
+	logger.log(L"Host archiecture: " + Architecture::AsString(hostArch));
 
 	const auto javaPaths = getMinecraftJavaPaths(hostArch);
 
@@ -140,31 +140,31 @@ bool Bootstrap::attemptLaunch(const std::wstring& path, bool checkExists) {
 }
 
 // Return all of the possible java paths, starting with the newest on the host platform, down to the oldest on the none host platforms.
-const std::vector<std::wstring> Bootstrap::getMinecraftJavaPaths(const HostArchitecture::Value& hostArch) {
+const std::vector<std::wstring> Bootstrap::getMinecraftJavaPaths(const Architecture::Value& hostArch) {
 	std::vector<std::wstring> paths;
 
-	for (const HostArchitecture::Value& arch : HostArchitecture::VALUES) {
-		if (arch == HostArchitecture::UNKNOWN) {
+	for (const Architecture::Value& arch : Architecture::VALUES) {
+		if (arch == Architecture::UNKNOWN) {
 			continue;
 		}
 
 		if (arch < hostArch) {
 			// Skip arches that the host does not support.
 			// E.g: On x64 there is no need to go looking for arm64 JDKs as its never going to run.
-			logger.log(L"Arch not supported: " + HostArchitecture::AsString(arch));
+			logger.log(L"Arch not supported: " + Architecture::AsString(arch));
 			continue;
 		}
 
 		std::wstring javaName;
 
 		switch (arch) {
-		case HostArchitecture::X64:
+		case Architecture::X64:
 			javaName = L"windows-x64";
 			break;
-		case HostArchitecture::ARM64:
+		case Architecture::ARM64:
 			javaName = L"windows-arm64";
 			break;
-		case HostArchitecture::X86:
+		case Architecture::X86:
 			javaName = L"windows-x86";
 			break;
 		default:

src/Bootstrap.h

@@ -20,7 +20,7 @@ class Bootstrap {
 
 	void showErrorMessage();
 
-	const std::vector<std::wstring> getMinecraftJavaPaths(const HostArchitecture::Value& hostArch);
+	const std::vector<std::wstring> getMinecraftJavaPaths(const Architecture::Value& hostArch);
 
 private:
 	ISystemHelper& systemHelper;

src/ISystemHelper.h

@@ -6,44 +6,18 @@
 #include <string>
 #include <vector>
 
-namespace HostArchitecture {
-	// Ordered to ensure that we only check for JVMs that will run on the host arch.
-	// On an x64 host only check for x64 and x86 as arm will never run.
-	// On x86 there is no need to try any other arch as it wont run.
-	enum Value {
-		UNKNOWN,
-		ARM64,
-		X64,
-		X86,
-	};
-
-	constexpr Value VALUES[] = { UNKNOWN, ARM64, X64, X86 };
-
-	inline std::wstring AsString(const Value& arch) {
-		switch (arch) {
-		case X64:
-			return L"x64";
-		case ARM64:
-			return L"arm64";
-		case X86:
-			return L"x86";
-		case UNKNOWN:
-		default:
-			return L"unknown";
-		}
-	}
-}
+#include "Architecture.h"
 
 class ISystemHelper {
 public:
-	virtual std::optional<std::wstring> getRegValue(HKEY hive, const std::wstring& path, const std::wstring& key) = 0;
-	virtual std::optional<std::wstring> getEnvVar(const std::wstring& key) = 0;
-	virtual void showErrorMessage(const std::wstring& title, const std::wstring& message, const std::wstring& url) = 0;
-	virtual DWORD createProcess(std::vector<std::wstring> args) = 0;
-	virtual bool fileExists(const std::wstring& path) = 0;
-	virtual bool dirExists(const std::wstring& path) = 0;
-	virtual std::wstring getBootstrapFilename() = 0;
-	virtual std::wstring getTempDir() = 0;
-	virtual HostArchitecture::Value getHostArchitecture() = 0;
-	virtual long long getEpochTime() = 0;
+	virtual std::optional<std::wstring> getRegValue(HKEY hive, const std::wstring& path, const std::wstring& key) const = 0;
+	virtual std::optional<std::wstring> getEnvVar(const std::wstring& key) const = 0;
+	virtual void showErrorMessage(const std::wstring& title, const std::wstring& message, const std::wstring& url) const = 0;
+	virtual DWORD createProcess(std::vector<std::wstring> args) const = 0;
+	virtual bool fileExists(const std::wstring& path) const = 0;
+	virtual bool dirExists(const std::wstring& path) const = 0;
+	virtual std::wstring getBootstrapFilename() const = 0;
+	virtual std::wstring getTempDir() const = 0;
+	virtual Architecture::Value getHostArchitecture() const = 0;
+	virtual int64_t getEpochTime() const = 0;
 };

src/SystemHelper.cpp

@@ -4,7 +4,7 @@
 #include <sstream>
 #include <chrono>
 
-std::optional<std::wstring> SystemHelper::getRegValue(HKEY hive, const std::wstring& path, const std::wstring& key) {
+std::optional<std::wstring> SystemHelper::getRegValue(HKEY hive, const std::wstring& path, const std::wstring& key) const {
 	DWORD dataSize{};
 	LONG retCode = ::RegGetValueW(
 		hive,
@@ -43,8 +43,7 @@ std::optional<std::wstring> SystemHelper::getRegValue(HKEY hive, const std::wstr
 	return value;
 }
 
-std::optional<std::wstring> SystemHelper::getEnvVar(const std::wstring& key)
-{
+std::optional<std::wstring> SystemHelper::getEnvVar(const std::wstring& key) const {
 	// Read the size of the env var
 	DWORD size = ::GetEnvironmentVariableW(key.c_str(), nullptr, 0);
 	if (!size || ::GetLastError() == ERROR_ENVVAR_NOT_FOUND) {
@@ -62,7 +61,7 @@ std::optional<std::wstring> SystemHelper::getEnvVar(const std::wstring& key)
 	return value;
 }
 
-void SystemHelper::showErrorMessage(const std::wstring& title, const std::wstring& message, const std::wstring& url) {
+void SystemHelper::showErrorMessage(const std::wstring& title, const std::wstring& message, const std::wstring& url) const {
 	const int result = ::MessageBoxW(
 		nullptr,
 		message.c_str(),
@@ -75,8 +74,7 @@ void SystemHelper::showErrorMessage(const std::wstring& title, const std::wstrin
 	}
 }
 
-DWORD SystemHelper::createProcess(std::vector<std::wstring> args)
-{
+DWORD SystemHelper::createProcess(std::vector<std::wstring> args) const {
 	STARTUPINFOW info;
 	PROCESS_INFORMATION processInfo;
 
@@ -122,32 +120,31 @@ DWORD SystemHelper::createProcess(std::vector<std::wstring> args)
 	}
 }
 
-bool SystemHelper::fileExists(const std::wstring& path) {
+bool SystemHelper::fileExists(const std::wstring& path) const {
 	const auto attributes = ::GetFileAttributesW(path.c_str());
 	return (attributes != INVALID_FILE_ATTRIBUTES && !(attributes & FILE_ATTRIBUTE_DIRECTORY));
 }
 
-bool SystemHelper::dirExists(const std::wstring& path) {
+bool SystemHelper::dirExists(const std::wstring& path) const {
 	const auto attributes = ::GetFileAttributesW(path.c_str());
 	return (attributes != INVALID_FILE_ATTRIBUTES && (attributes & FILE_ATTRIBUTE_DIRECTORY));
 }
 
-std::wstring SystemHelper::getBootstrapFilename() {
+std::wstring SystemHelper::getBootstrapFilename() const {
 	wchar_t moduleFileName[MAX_PATH] = { 0 };
 	::GetModuleFileNameW(nullptr, moduleFileName, MAX_PATH);
 	return moduleFileName;
 }
 
-std::wstring SystemHelper::getTempDir()
-{
+std::wstring SystemHelper::getTempDir() const {
 	std::wstring tempDir;
 	DWORD size = ::GetTempPath(0, nullptr);
 
 	if (!size) {
 		throw std::runtime_error("Failed to get temp path");
 	}
 
-	tempDir.resize(size + 1);
+	tempDir.resize((size_t) size + 1);
 	size = ::GetTempPath(size + 1, tempDir.data());
 
 	if (!size || size >= tempDir.size()) {
@@ -159,31 +156,31 @@ std::wstring SystemHelper::getTempDir()
 }
 
 // A windows 7 compatible version of getHostArchitecture, must either be x64 or x86
-HostArchitecture::Value getLegacyHostArchitecture() {
+Architecture::Value getLegacyHostArchitecture() {
 #if defined(_M_X64)
 	// x64 bin will only run on x64 Windows 7 & 8
-	return HostArchitecture::X64;
+	return Architecture::X64;
 #else
 	BOOL isWow64 = FALSE;
 
 	if (!::IsWow64Process(::GetCurrentProcess(), &isWow64)) {
-		return HostArchitecture::Value::UNKNOWN;
+		return Architecture::Value::UNKNOWN;
 	}
 
 	if (isWow64) {
-		return HostArchitecture::Value::X64;
+		return Architecture::Value::X64;
 	}
 
-	return HostArchitecture::Value::X86;
+	return Architecture::Value::X86;
 # endif
 }
 
 // https://devblogs.microsoft.com/oldnewthing/20220209-00/?p=106239
 // Slightly fun as we are almost always ran though emulation
-HostArchitecture::Value SystemHelper::getHostArchitecture() {
+Architecture::Value SystemHelper::getHostArchitecture() const {
 #if defined(_M_ARM64)
 	// ARM64 bin will only run on ARM64.
-	return HostArchitecture::ARM64;
+	return Architecture::ARM64;
 #else
 	const auto kernel32Handle = ::GetModuleHandle(TEXT("kernel32.dll"));
 
@@ -206,24 +203,23 @@ HostArchitecture::Value SystemHelper::getHostArchitecture() {
 
 	if (result == 0) {
 		// Error/Unknown
-		return HostArchitecture::Value::UNKNOWN;
+		return Architecture::Value::UNKNOWN;
 	}
 
 	switch (native_machine) {
 	case IMAGE_FILE_MACHINE_ARM64:
-		return HostArchitecture::Value::ARM64;
+		return Architecture::Value::ARM64;
 	case IMAGE_FILE_MACHINE_AMD64:
-		return HostArchitecture::Value::X64;
+		return Architecture::Value::X64;
 	case IMAGE_FILE_MACHINE_I386:
-		return HostArchitecture::Value::X86;
+		return Architecture::Value::X86;
 	default:
-		return HostArchitecture::Value::UNKNOWN;
+		return Architecture::Value::UNKNOWN;
 	}
 # endif
 }
 
-long long SystemHelper::getEpochTime()
-{
+int64_t SystemHelper::getEpochTime() const {
 	const auto now = std::chrono::system_clock::now();
 	return std::chrono::duration_cast<std::chrono::seconds>(now.time_since_epoch()).count();
 }

src/SystemHelper.h

@@ -4,14 +4,14 @@
 
 class SystemHelper : public ISystemHelper {
 public:
-	std::optional<std::wstring> getRegValue(HKEY hive, const std::wstring& path, const std::wstring& key) override;
-	std::optional<std::wstring> getEnvVar(const std::wstring& key) override;
-	void showErrorMessage(const std::wstring& title, const std::wstring& message, const std::wstring& url) override;
-	DWORD createProcess(std::vector<std::wstring> args) override;
-	bool fileExists(const std::wstring& path) override;
-	bool dirExists(const std::wstring& path) override;
-	std::wstring getBootstrapFilename() override;
-	std::wstring getTempDir() override;
-	HostArchitecture::Value getHostArchitecture() override;
-	long long getEpochTime() override;
+	std::optional<std::wstring> getRegValue(HKEY hive, const std::wstring& path, const std::wstring& key) const override;
+	std::optional<std::wstring> getEnvVar(const std::wstring& key) const override;
+	void showErrorMessage(const std::wstring& title, const std::wstring& message, const std::wstring& url) const override;
+	DWORD createProcess(std::vector<std::wstring> args) const override;
+	bool fileExists(const std::wstring& path) const override;
+	bool dirExists(const std::wstring& path) const override;
+	std::wstring getBootstrapFilename() const override;
+	std::wstring getTempDir() const override;
+	Architecture::Value getHostArchitecture() const override;
+	int64_t getEpochTime() const override;
 };