From bd3850947b18e620faefa915afe0b35e22c9619d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johan=20Sj=C3=B6blom?= Date: Fri, 5 Jun 2026 16:11:42 +0200 Subject: [PATCH] Improving error handling: More descriptive error messages, not double-closing archives, not printing the same errors twice --- src/commands.cpp | 7 ------- src/helpers.cpp | 38 ++++++++++++++++++++++++++++++++++++++ src/helpers.h | 1 + src/main.cpp | 2 +- src/mpq.cpp | 36 +++++++++++++++++++++--------------- 5 files changed, 61 insertions(+), 23 deletions(-) diff --git a/src/commands.cpp b/src/commands.cpp index 9b1c744..c100a5b 100644 --- a/src/commands.cpp +++ b/src/commands.cpp @@ -44,7 +44,6 @@ int HandleAbout() { int HandleInfo(const std::string &target, const std::optional &property) { HANDLE hArchive; if (!OpenMpqArchive(target, &hArchive, MPQ_OPEN_READ_ONLY)) { - std::cerr << "[!] Failed to open MPQ archive." << std::endl; return 1; } PrintMpqInfo(hArchive, property); @@ -149,7 +148,6 @@ int HandleAdd(const std::vector &files, const std::string &target, int64_t fileDwCompression, int64_t fileDwCompressionNext) { HANDLE hArchive; if (!OpenMpqArchive(target, &hArchive, 0)) { - std::cerr << "[!] Failed to open MPQ archive." << std::endl; return 1; } @@ -213,7 +211,6 @@ int HandleRemove(const std::vector &files, const std::string &targe const std::optional &locale) { HANDLE hArchive; if (!OpenMpqArchive(target, &hArchive, 0)) { - std::cerr << "[!] Failed to open MPQ archive." << std::endl; return 1; } @@ -237,7 +234,6 @@ int HandleList(const std::string &target, const std::optional &list bool listAll, bool listDetailed, const std::vector &properties) { HANDLE hArchive; if (!OpenMpqArchive(target, &hArchive, MPQ_OPEN_READ_ONLY)) { - std::cerr << "[!] Failed to open MPQ archive." << std::endl; return 1; } ListFiles(hArchive, listfileName, listAll, listDetailed, properties); @@ -266,7 +262,6 @@ int HandleExtract(const std::string &target, const std::optional &o HANDLE hArchive; if (!OpenMpqArchive(target, &hArchive, MPQ_OPEN_READ_ONLY)) { - std::cerr << "[!] Failed to open MPQ archive." << std::endl; return 1; } @@ -294,7 +289,6 @@ int HandleRead(const std::string &file, const std::string &target, const std::optional &locale) { HANDLE hArchive; if (!OpenMpqArchive(target, &hArchive, MPQ_OPEN_READ_ONLY)) { - std::cerr << "[!] Failed to open MPQ archive." << std::endl; return 1; } @@ -319,7 +313,6 @@ int HandleRead(const std::string &file, const std::string &target, int HandleVerify(const std::string &target, bool printSignature) { HANDLE hArchive; if (!OpenMpqArchive(target, &hArchive, MPQ_OPEN_READ_ONLY)) { - std::cerr << "[!] Failed to open MPQ archive." << std::endl; return 1; } diff --git a/src/helpers.cpp b/src/helpers.cpp index 553bb83..cd62ca5 100644 --- a/src/helpers.cpp +++ b/src/helpers.cpp @@ -1,6 +1,7 @@ #include "helpers.h" #include +#include #include #include #include @@ -47,6 +48,43 @@ std::string WindowsifyFilePath(const fs::path &path) { return filePath; } +std::string StormErrorString(uint32_t err) { + switch (err) { + // clang-format off + case ERROR_SUCCESS: return "Success"; + case ERROR_FILE_NOT_FOUND: return "File not found"; + case ERROR_ACCESS_DENIED: return "Access denied (archive may be read-only or have open files)"; + case ERROR_INVALID_HANDLE: return "Invalid handle"; + case ERROR_NOT_ENOUGH_MEMORY: return "Not enough memory"; + case ERROR_NOT_SUPPORTED: return "Operation not supported"; + case ERROR_INVALID_PARAMETER: return "Invalid parameter"; + case ERROR_DISK_FULL: return "Disk full"; + case ERROR_ALREADY_EXISTS: return "Already exists"; + case ERROR_INSUFFICIENT_BUFFER: return "Insufficient buffer"; + case ERROR_BAD_FORMAT: return "Bad MPQ format"; + case ERROR_NO_MORE_FILES: return "No more files"; + case ERROR_HANDLE_EOF: return "End of file"; + case ERROR_CAN_NOT_COMPLETE: return "Cannot complete"; + case ERROR_FILE_CORRUPT: return "File is corrupt"; + case ERROR_BUFFER_OVERFLOW: return "Buffer overflow"; + case ERROR_INVALID_DATA: return "Invalid data"; + case ERROR_NO_UNICODE_TRANSLATION: return "No Unicode translation"; + case ERROR_AVI_FILE: return "Not an MPQ file (AVI file)"; + case ERROR_UNKNOWN_FILE_KEY: return "Unknown file encryption key"; + case ERROR_CHECKSUM_ERROR: return "Sector checksum mismatch"; + case ERROR_INTERNAL_FILE: return "Operation not allowed on internal file"; + case ERROR_BASE_FILE_MISSING: return "Base file missing for incremental patch"; + case ERROR_MARKED_FOR_DELETE: return "File is marked as deleted in the MPQ"; + case ERROR_FILE_INCOMPLETE: return "Required file part is missing"; + case ERROR_UNKNOWN_FILE_NAMES: return "At least one file name is unknown (listfile is incomplete)"; + case ERROR_CANT_FIND_PATCH_PREFIX: return "Cannot find patch prefix"; + case ERROR_FAKE_MPQ_HEADER: return "Fake MPQ header at this position"; + case ERROR_FILE_DELETED: return "File contains delete marker"; + default: return std::strerror(static_cast(err)); + // clang-format on + } +} + uint32_t CalculateMpqMaxFileValue(const std::string &path) { uint32_t fileCount = 0; diff --git a/src/helpers.h b/src/helpers.h index 52ae7e3..8a1fb2b 100644 --- a/src/helpers.h +++ b/src/helpers.h @@ -9,6 +9,7 @@ namespace fs = std::filesystem; std::string FileTimeToLsTime(int64_t fileTime); std::string NormalizeFilePath(const fs::path &path); std::string WindowsifyFilePath(const fs::path &path); +std::string StormErrorString(uint32_t err); uint32_t CalculateMpqMaxFileValue(const std::string &path); uint32_t NextPowerOfTwo(uint32_t n); void PrintAsBinary(const char *buffer, uint32_t size); diff --git a/src/main.cpp b/src/main.cpp index b4140f4..e0d1b99 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -19,7 +19,7 @@ int main(int argc, char **argv) { // CLI: base // These are reused in multiple subcommands - // clang-format off: preserve column-aligned flag-to-char mappings + // clang-format off std::string baseTarget; // all subcommands std::string baseFile; // extract, read std::optional baseLocale; // add, create, extract, read, remove diff --git a/src/mpq.cpp b/src/mpq.cpp index cdd7939..db50424 100644 --- a/src/mpq.cpp +++ b/src/mpq.cpp @@ -23,7 +23,9 @@ static const std::vector kSpecialMpqFiles = {"(listfile)", "(signat bool OpenMpqArchive(const std::string &filename, HANDLE *hArchive, int32_t flags) { if (!SFileOpenArchive(filename.c_str(), 0, flags, hArchive)) { - std::cerr << "[!] Failed to open: " << filename << std::endl; + const auto error = SErrGetLastError(); + std::cerr << "[!] Failed to open MPQ archive: " << filename << ": (" << error << ") " + << StormErrorString(error) << std::endl; return false; } return true; @@ -31,7 +33,9 @@ bool OpenMpqArchive(const std::string &filename, HANDLE *hArchive, int32_t flags bool CloseMpqArchive(HANDLE hArchive) { if (!SFileCloseArchive(hArchive)) { - std::cerr << "[!] Failed to close MPQ archive." << std::endl; + const auto error = SErrGetLastError(); + std::cerr << "[!] Failed to close MPQ archive: (" << error << ") " + << StormErrorString(error) << std::endl; return false; } return true; @@ -54,7 +58,9 @@ bool FileExistsInArchiveForLocale(const HANDLE hArchive, const std::string &file bool SignMpqArchive(HANDLE hArchive) { if (!SFileSignArchive(hArchive, SIGNATURE_TYPE_WEAK)) { - std::cerr << "[!] Failed to sign MPQ archive." << std::endl; + const auto error = SErrGetLastError(); + std::cerr << "[!] Failed to sign MPQ archive: (" << error << ") " << StormErrorString(error) + << std::endl; return false; } return true; @@ -70,7 +76,6 @@ int ExtractFiles(HANDLE hArchive, const std::string &output, HANDLE findHandle = SFileFindFirstFile(hArchive, "*", &findData, listfile); if (findHandle == nullptr) { std::cerr << "[!] Failed to find first file in MPQ archive." << std::endl; - SFileCloseArchive(hArchive); return 1; } @@ -133,8 +138,9 @@ int ExtractFile(HANDLE hArchive, const std::string &output, const std::string &f if (SFileExtractFile(hArchive, szFileName, outputFileName.c_str(), 0)) { std::cout << "[*] Extracted: " << fileNameString << std::endl; } else { - int32_t error = SErrGetLastError(); - std::cerr << "[!] Failed: " << "(" << error << ") " << szFileName << std::endl; + const auto error = SErrGetLastError(); + std::cerr << "[!] Failed: (" << error << ") " << StormErrorString(error) << ": " + << szFileName << std::endl; return 1; } @@ -170,9 +176,9 @@ HANDLE CreateMpqArchive(const std::string &outputArchiveName, const uint32_t fil const bool result = SFileCreateArchive2(outputArchiveName.c_str(), &createInfo, &hMpq); if (!result) { - std::cerr << "[!] Failed to create MPQ archive: " << outputArchiveName << std::endl; - int32_t error = SErrGetLastError(); - std::cout << "[!] Error: " << error << std::endl; + const auto error = SErrGetLastError(); + std::cerr << "[!] Failed to create MPQ archive: " << outputArchiveName << ": (" << error + << ") " << StormErrorString(error) << std::endl; return nullptr; } @@ -291,9 +297,9 @@ int AddFile(HANDLE hArchive, const fs::path &localFile, const std::string &archi uint32_t newMaxFiles = NextPowerOfTwo(static_cast(numberOfFiles + 1)); bool setMaxFileCount = SFileSetMaxFileCount(hArchive, newMaxFiles); if (!setMaxFileCount) { - int32_t error = SErrGetLastError(); - std::cerr << "[!] Error: " << error - << " Failed to increase new max file count to: " << newMaxFiles << std::endl; + const auto error = SErrGetLastError(); + std::cerr << "[!] Failed to increase new max file count to " << newMaxFiles << ": (" + << error << ") " << StormErrorString(error) << std::endl; return 1; } } @@ -325,8 +331,9 @@ int AddFile(HANDLE hArchive, const fs::path &localFile, const std::string &archi dwFlags, dwCompression, dwCompressionNext); if (!addedFile) { - int32_t error = SErrGetLastError(); - std::cerr << "[!] Error: " << error << " Failed to add: " << archiveFilePath << std::endl; + const auto error = SErrGetLastError(); + std::cerr << "[!] Failed to add: " << archiveFilePath << ": (" << error << ") " + << StormErrorString(error) << std::endl; return 1; } @@ -386,7 +393,6 @@ int ListFiles(HANDLE hArchive, const std::optional &listfileName, b HANDLE findHandle = SFileFindFirstFile(hArchive, "*", &findData, listfile); if (findHandle == nullptr) { std::cerr << "[!] Failed to find first file in MPQ archive." << std::endl; - SFileCloseArchive(hArchive); return -1; }