From 0abc66dbff9875354d66138e8ce20d0f5ba4aa62 Mon Sep 17 00:00:00 2001 From: Christoph Oelckers Date: Tue, 20 Aug 2019 22:34:35 +0200 Subject: [PATCH] - uncoupled the decompressors from ZDoom's internal error handling. This code made it hard to repurpose this code for other tools, so now the error handler must be passed as a callback to OpenDecompressor. --- src/gamedata/resourcefiles/file_wad.cpp | 2 +- src/gamedata/resourcefiles/file_zip.cpp | 2 +- src/i_net.cpp | 1 + src/m_misc.cpp | 29 -------- src/m_misc.h | 4 -- src/maploader/maploader.cpp | 16 +++-- src/utility/cmdlib.cpp | 30 +++++++++ src/utility/cmdlib.h | 1 + src/utility/files.h | 8 ++- src/utility/files_decompress.cpp | 90 +++++++++++++++++-------- 10 files changed, 115 insertions(+), 68 deletions(-) diff --git a/src/gamedata/resourcefiles/file_wad.cpp b/src/gamedata/resourcefiles/file_wad.cpp index d3aab7d5e..f6db35f14 100644 --- a/src/gamedata/resourcefiles/file_wad.cpp +++ b/src/gamedata/resourcefiles/file_wad.cpp @@ -83,7 +83,7 @@ public: if(Compressed) { FileReader lzss; - if (lzss.OpenDecompressor(Owner->Reader, LumpSize, METHOD_LZSS, false)) + if (lzss.OpenDecompressor(Owner->Reader, LumpSize, METHOD_LZSS, false, [](const char* err) { I_Error("%s", err); })) { lzss.Read(Cache, LumpSize); } diff --git a/src/gamedata/resourcefiles/file_zip.cpp b/src/gamedata/resourcefiles/file_zip.cpp index ac7ae7a08..8ee031ae0 100644 --- a/src/gamedata/resourcefiles/file_zip.cpp +++ b/src/gamedata/resourcefiles/file_zip.cpp @@ -68,7 +68,7 @@ static bool UncompressZipLump(char *Cache, FileReader &Reader, int Method, int L case METHOD_LZMA: { FileReader frz; - if (frz.OpenDecompressor(Reader, LumpSize, Method, false)) + if (frz.OpenDecompressor(Reader, LumpSize, Method, false, [](const char* err) { I_Error("%s", err); })) { frz.Read(Cache, LumpSize); } diff --git a/src/i_net.cpp b/src/i_net.cpp index d2b17efb1..717e204cf 100644 --- a/src/i_net.cpp +++ b/src/i_net.cpp @@ -63,6 +63,7 @@ #include "m_misc.h" #include "doomerrors.h" #include "atterm.h" +#include "cmdlib.h" #include "i_net.h" diff --git a/src/m_misc.cpp b/src/m_misc.cpp index cb980bf73..ea6974153 100644 --- a/src/m_misc.cpp +++ b/src/m_misc.cpp @@ -656,32 +656,3 @@ UNSAFE_CCMD (screenshot) G_ScreenShot (argv[1]); } -// -// M_ZlibError -// -FString M_ZLibError(int zerr) -{ - if (zerr >= 0) - { - return "OK"; - } - else if (zerr < -6) - { - FString out; - out.Format("%d", zerr); - return out; - } - else - { - static const char *errs[6] = - { - "Errno", - "Stream Error", - "Data Error", - "Memory Error", - "Buffer Error", - "Version Error" - }; - return errs[-zerr - 1]; - } -} diff --git a/src/m_misc.h b/src/m_misc.h index a0f1e6747..8a0b9ef7b 100644 --- a/src/m_misc.h +++ b/src/m_misc.h @@ -44,10 +44,6 @@ void M_LoadDefaults (); bool M_SaveDefaults (const char *filename); void M_SaveCustomKeys (FConfigFile *config, char *section, char *subsection, size_t sublen); - - -FString M_ZLibError(int zerrnum); - // Get special directory paths (defined in m_specialpaths.cpp) #ifdef __unix__ diff --git a/src/maploader/maploader.cpp b/src/maploader/maploader.cpp index 65fd69da4..368d38824 100644 --- a/src/maploader/maploader.cpp +++ b/src/maploader/maploader.cpp @@ -728,13 +728,21 @@ bool MapLoader::LoadExtendedNodes (FileReader &dalump, uint32_t id) if (compressed) { FileReader zip; - if (zip.OpenDecompressor(dalump, -1, METHOD_ZLIB, false)) + try { - LoadZNodes(zip, type); + if (zip.OpenDecompressor(dalump, -1, METHOD_ZLIB, false, [](const char* err) { I_Error("%s", err); })) + { + LoadZNodes(zip, type); + } + else + { + Printf("Error loading nodes: Corrupt data.\n"); + return false; + } } - else + catch (const CRecoverableError& err) { - Printf("Error loading nodes: Corrupt data.\n"); + Printf("Error loading nodes: %s.\n", err.what()); return false; } } diff --git a/src/utility/cmdlib.cpp b/src/utility/cmdlib.cpp index efc590032..9901a4060 100644 --- a/src/utility/cmdlib.cpp +++ b/src/utility/cmdlib.cpp @@ -995,3 +995,33 @@ bool IsAbsPath(const char *name) #endif /* _WIN32 */ return 0; } + +// +// M_ZlibError +// +FString M_ZLibError(int zerr) +{ + if (zerr >= 0) + { + return "OK"; + } + else if (zerr < -6) + { + FString out; + out.Format("%d", zerr); + return out; + } + else + { + static const char* errs[6] = + { + "Errno", + "Stream Error", + "Data Error", + "Memory Error", + "Buffer Error", + "Version Error" + }; + return errs[-zerr - 1]; + } +} diff --git a/src/utility/cmdlib.h b/src/utility/cmdlib.h index 850e7fe69..7f42370e4 100644 --- a/src/utility/cmdlib.h +++ b/src/utility/cmdlib.h @@ -70,6 +70,7 @@ struct FFileList bool ScanDirectory(TArray &list, const char *dirpath); bool IsAbsPath(const char*); +FString M_ZLibError(int zerrnum); #endif diff --git a/src/utility/files.h b/src/utility/files.h index ea24cff20..3ab278466 100644 --- a/src/utility/files.h +++ b/src/utility/files.h @@ -70,12 +70,18 @@ public: class DecompressorBase : public FileReaderInterface { + std::function ErrorCallback = nullptr; public: // These do not work but need to be defined to satisfy the FileReaderInterface. // They will just error out when called. long Tell() const override; long Seek(long offset, int origin) override; char *Gets(char *strbuf, int len) override; + void DecompressionError(const char* error, ...) const; + void SetErrorCallback(const std::function& cb) + { + ErrorCallback = cb; + } }; class MemoryReader : public FileReaderInterface @@ -167,7 +173,7 @@ public: bool OpenMemory(const void *mem, Size length); // read directly from the buffer bool OpenMemoryArray(const void *mem, Size length); // read from a copy of the buffer. bool OpenMemoryArray(std::function&)> getter); // read contents to a buffer and return a reader to it - bool OpenDecompressor(FileReader &parent, Size length, int method, bool seekable); // creates a decompressor stream. 'seekable' uses a buffered version so that the Seek and Tell methods can be used. + bool OpenDecompressor(FileReader &parent, Size length, int method, bool seekable, const std::function& cb); // creates a decompressor stream. 'seekable' uses a buffered version so that the Seek and Tell methods can be used. Size Tell() const { diff --git a/src/utility/files_decompress.cpp b/src/utility/files_decompress.cpp index 4ff30f791..03f0c107c 100644 --- a/src/utility/files_decompress.cpp +++ b/src/utility/files_decompress.cpp @@ -39,24 +39,46 @@ #include #include "files.h" -#include "doomerrors.h" #include "templates.h" -#include "m_misc.h" +#include "cmdlib.h" +//========================================================================== +// +// I_Error +// +// Throw an error that will send us to the console if we are far enough +// along in the startup process. +// +//========================================================================== + +void DecompressorBase::DecompressionError(const char *error, ...) const +{ + const int MAX_ERRORTEXT = 300; + va_list argptr; + char errortext[MAX_ERRORTEXT]; + + va_start(argptr, error); + myvsnprintf(errortext, MAX_ERRORTEXT, error, argptr); + va_end(argptr); + + if (ErrorCallback != nullptr) ErrorCallback(errortext); + else std::terminate(); +} + long DecompressorBase::Tell () const { - I_Error("Cannot get position of decompressor stream"); + DecompressionError("Cannot get position of decompressor stream"); return 0; } long DecompressorBase::Seek (long offset, int origin) { - I_Error("Cannot seek in decompressor stream"); + DecompressionError("Cannot seek in decompressor stream"); return 0; } char *DecompressorBase::Gets(char *strbuf, int len) { - I_Error("Cannot use Gets on decompressor stream"); + DecompressionError("Cannot use Gets on decompressor stream"); return nullptr; } @@ -79,11 +101,12 @@ class DecompressorZ : public DecompressorBase uint8_t InBuff[BUFF_SIZE]; public: - DecompressorZ (FileReader &file, bool zip) + DecompressorZ (FileReader &file, bool zip, const std::function& cb) : File(file), SawEOF(false) { int err; + SetErrorCallback(cb); FillBuffer (); Stream.zalloc = Z_NULL; @@ -94,7 +117,7 @@ public: if (err != Z_OK) { - I_Error ("DecompressorZ: inflateInit failed: %s\n", M_ZLibError(err).GetChars()); + DecompressionError ("DecompressorZ: inflateInit failed: %s\n", M_ZLibError(err).GetChars()); } } @@ -121,12 +144,12 @@ public: if (err != Z_OK && err != Z_STREAM_END) { - I_Error ("Corrupt zlib stream"); + DecompressionError ("Corrupt zlib stream"); } if (Stream.avail_out != 0) { - I_Error ("Ran out of data in zlib stream"); + DecompressionError ("Ran out of data in zlib stream"); } return len - Stream.avail_out; @@ -155,6 +178,10 @@ public: // //========================================================================== +class DecompressorBZ2; +static DecompressorBZ2 * stupidGlobal; // Why does that dumb global error callback not pass the decompressor state? + // Thanks to that brain-dead interface we have to use a global variable to get the error to the proper handler. + class DecompressorBZ2 : public DecompressorBase { enum { BUFF_SIZE = 4096 }; @@ -165,11 +192,13 @@ class DecompressorBZ2 : public DecompressorBase uint8_t InBuff[BUFF_SIZE]; public: - DecompressorBZ2 (FileReader &file) + DecompressorBZ2 (FileReader &file, const std::function& cb) : File(file), SawEOF(false) { int err; + SetErrorCallback(cb); + stupidGlobal = this; FillBuffer (); Stream.bzalloc = NULL; @@ -180,12 +209,13 @@ public: if (err != BZ_OK) { - I_Error ("DecompressorBZ2: bzDecompressInit failed: %d\n", err); + DecompressionError ("DecompressorBZ2: bzDecompressInit failed: %d\n", err); } } ~DecompressorBZ2 () { + stupidGlobal = this; BZ2_bzDecompressEnd (&Stream); } @@ -193,6 +223,7 @@ public: { int err; + stupidGlobal = this; Stream.next_out = (char *)buffer; Stream.avail_out = len; @@ -207,12 +238,12 @@ public: if (err != BZ_OK && err != BZ_STREAM_END) { - I_Error ("Corrupt bzip2 stream"); + DecompressionError ("Corrupt bzip2 stream"); } if (Stream.avail_out != 0) { - I_Error ("Ran out of data in bzip2 stream"); + DecompressionError ("Ran out of data in bzip2 stream"); } return len - Stream.avail_out; @@ -242,7 +273,8 @@ public: extern "C" void bz_internal_error (int errcode) { - I_FatalError("libbzip2: internal error number %d\n", errcode); + if (stupidGlobal) stupidGlobal->DecompressionError("libbzip2: internal error number %d\n", errcode); + else std::terminate(); } //========================================================================== @@ -273,11 +305,12 @@ class DecompressorLZMA : public DecompressorBase public: - DecompressorLZMA (FileReader &file, size_t uncompressed_size) + DecompressorLZMA (FileReader &file, size_t uncompressed_size, const std::function& cb) : File(file), SawEOF(false) { uint8_t header[4 + LZMA_PROPS_SIZE]; int err; + SetErrorCallback(cb); Size = uncompressed_size; OutProcessed = 0; @@ -285,11 +318,11 @@ public: // Read zip LZMA properties header if (File.Read(header, sizeof(header)) < (long)sizeof(header)) { - I_Error("DecompressorLZMA: File too short\n"); + DecompressionError("DecompressorLZMA: File too short\n"); } if (header[2] + header[3] * 256 != LZMA_PROPS_SIZE) { - I_Error("DecompressorLZMA: LZMA props size is %d (expected %d)\n", + DecompressionError("DecompressorLZMA: LZMA props size is %d (expected %d)\n", header[2] + header[3] * 256, LZMA_PROPS_SIZE); } @@ -300,7 +333,7 @@ public: if (err != SZ_OK) { - I_Error("DecompressorLZMA: LzmaDec_Allocate failed: %d\n", err); + DecompressionError("DecompressorLZMA: LzmaDec_Allocate failed: %d\n", err); } LzmaDec_Init(&Stream); @@ -330,13 +363,13 @@ public: len = (long)(len - out_processed); if (err != SZ_OK) { - I_Error ("Corrupt LZMA stream"); + DecompressionError ("Corrupt LZMA stream"); } if (in_processed == 0 && out_processed == 0) { if (status != LZMA_STATUS_FINISHED_WITH_MARK) { - I_Error ("Corrupt LZMA stream"); + DecompressionError ("Corrupt LZMA stream"); } } if (InSize == 0 && !SawEOF) @@ -347,12 +380,12 @@ public: if (err != Z_OK && err != Z_STREAM_END) { - I_Error ("Corrupt LZMA stream"); + DecompressionError ("Corrupt LZMA stream"); } if (len != 0) { - I_Error ("Ran out of data in LZMA stream"); + DecompressionError ("Ran out of data in LZMA stream"); } return (long)(next_out - (Byte *)buffer); @@ -500,8 +533,9 @@ class DecompressorLZSS : public DecompressorBase } public: - DecompressorLZSS(FileReader &file) : File(file), SawEOF(false) + DecompressorLZSS(FileReader &file, const std::function& cb) : File(file), SawEOF(false) { + SetErrorCallback(cb); Stream.State = STREAM_EMPTY; Stream.WindowData = Stream.InternalBuffer = Stream.Window+WINDOW_SIZE; Stream.InternalOut = 0; @@ -562,26 +596,26 @@ public: }; -bool FileReader::OpenDecompressor(FileReader &parent, Size length, int method, bool seekable) +bool FileReader::OpenDecompressor(FileReader &parent, Size length, int method, bool seekable, const std::function& cb) { DecompressorBase *dec = nullptr; switch (method) { case METHOD_DEFLATE: case METHOD_ZLIB: - dec = new DecompressorZ(parent, method == METHOD_DEFLATE); + dec = new DecompressorZ(parent, method == METHOD_DEFLATE, cb); break; case METHOD_BZIP2: - dec = new DecompressorBZ2(parent); + dec = new DecompressorBZ2(parent, cb); break; case METHOD_LZMA: - dec = new DecompressorLZMA(parent, length); + dec = new DecompressorLZMA(parent, length, cb); break; case METHOD_LZSS: - dec = new DecompressorLZSS(parent); + dec = new DecompressorLZSS(parent, cb); break; // todo: METHOD_IMPLODE, METHOD_SHRINK