From 8bbfd39f423433f8049c7b16b77f69f435a05d0c Mon Sep 17 00:00:00 2001 From: Christoph Oelckers Date: Sun, 11 Mar 2018 13:26:30 +0100 Subject: [PATCH] - fixed some remaining issues: * initial positioning in a subsection of a file failed. This mainly affected music playback. * made the FileRdr constructor which takes a FileReaderInterface private so that everything that needs it must be explicitly declared as friend. * removed a few redundant construction initializers for FileRdrs. * loading compressed nodes needs to check the validity of its reader. * use GetLength to detemine the size of a Zip file instead of doing another seek to the end. * removed duplicate Length variables. --- src/files.cpp | 8 +++----- src/files.h | 28 ++++++++++++++++++++-------- src/files_decompress.cpp | 33 ++++++++++++++------------------- src/m_png.cpp | 2 +- src/p_setup.cpp | 4 ++-- src/resourcefiles/file_zip.cpp | 4 +--- src/sound/oalsound.cpp | 2 +- 7 files changed, 42 insertions(+), 39 deletions(-) diff --git a/src/files.cpp b/src/files.cpp index b445c2789..f0939c923 100644 --- a/src/files.cpp +++ b/src/files.cpp @@ -51,7 +51,6 @@ class StdFileReader : public FileReaderInterface { FILE *File = nullptr; - long Length = 0; long StartPos = 0; long FilePos = 0; @@ -76,7 +75,7 @@ public: StartPos = startpos; Length = CalcFileLen(); if (len >= 0 && len < Length) Length = len; - if (startpos > 0) Seek(startpos, SEEK_SET); + if (startpos > 0) Seek(0, SEEK_SET); return true; } @@ -99,7 +98,7 @@ public: { offset += StartPos + Length; } - if (offset < StartPos || offset >= StartPos + Length) return -1; // out of scope + if (offset < StartPos || offset > StartPos + Length) return -1; // out of scope if (0 == fseek(File, offset, SEEK_SET)) { @@ -161,7 +160,6 @@ private: class FileReaderRedirect : public FileReaderInterface { FileRdr *mReader = nullptr; - long Length = 0; long StartPos = 0; long FilePos = 0; @@ -196,7 +194,7 @@ public: offset += (long)mReader->Tell(); break; } - if (offset < StartPos || offset >= StartPos + Length) return -1; // out of scope + if (offset < StartPos || offset > StartPos + Length) return -1; // out of scope if (mReader->Seek(offset, FileRdr::SeekSet) == 0) { FilePos = offset; diff --git a/src/files.h b/src/files.h index c567baac8..f4cf5c10f 100644 --- a/src/files.h +++ b/src/files.h @@ -43,7 +43,7 @@ #include "doomtype.h" #include "m_swap.h" -// Zip compression methods, extended by some internal types to be passed to FileReader::OpenDecompressor +// Zip compression methods, extended by some internal types to be passed to OpenDecompressor enum { METHOD_STORED = 0, @@ -70,6 +70,16 @@ public: long GetLength () const { return Length; } }; +class DecompressorBase : public FileReaderInterface +{ +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; +}; + class MemoryReader : public FileReaderInterface { protected: @@ -95,14 +105,22 @@ public: }; +struct FResourceLump; -class FileRdr // this is just a temporary name, until the old FileReader hierarchy can be made private. +class FileRdr { + friend struct FResourceLump; // needs access to the private constructor. + FileReaderInterface *mReader = nullptr; FileRdr(const FileRdr &r) = delete; FileRdr &operator=(const FileRdr &r) = delete; + explicit FileRdr(FileReaderInterface *r) + { + mReader = r; + } + public: enum ESeek { @@ -115,12 +133,6 @@ public: FileRdr() {} - // These two functions are only needed as long as the FileReader has not been fully replaced throughout the code. - explicit FileRdr(FileReaderInterface *r) - { - mReader = r; - } - FileRdr(FileRdr &&r) { mReader = r.mReader; diff --git a/src/files_decompress.cpp b/src/files_decompress.cpp index 4b295fe38..0e8c905ba 100644 --- a/src/files_decompress.cpp +++ b/src/files_decompress.cpp @@ -42,26 +42,21 @@ #include "m_misc.h" -class DecompressorBase : public FileReaderInterface +long DecompressorBase::Tell () const { -public: - // These do not work but need to be defined to satisfy the FileReader interface - long Tell () const override - { - I_Error("Cannot get position of decompressor stream"); - return 0; - } - virtual long Seek (long offset, int origin) override - { - I_Error("Cannot seek in decompressor stream"); - return 0; - } - virtual char *Gets(char *strbuf, int len) override - { - I_Error("Cannot use Gets on decompressor stream"); - return nullptr; - } -}; + I_Error("Cannot get position of decompressor stream"); + return 0; +} +long DecompressorBase::Seek (long offset, int origin) +{ + I_Error("Cannot seek in decompressor stream"); + return 0; +} +char *DecompressorBase::Gets(char *strbuf, int len) +{ + I_Error("Cannot use Gets on decompressor stream"); + return nullptr; +} //========================================================================== // diff --git a/src/m_png.cpp b/src/m_png.cpp index 63e57e35c..ee73e0270 100644 --- a/src/m_png.cpp +++ b/src/m_png.cpp @@ -75,7 +75,7 @@ struct IHDR uint8_t Interlace; }; -PNGHandle::PNGHandle (FileRdr &file) : File(0), bDeleteFilePtr(true), ChunkPt(0) +PNGHandle::PNGHandle (FileRdr &file) : bDeleteFilePtr(true), ChunkPt(0) { File = std::move(file); } diff --git a/src/p_setup.cpp b/src/p_setup.cpp index 8438e4074..f02c8f121 100644 --- a/src/p_setup.cpp +++ b/src/p_setup.cpp @@ -3853,7 +3853,7 @@ void P_SetupLevel (const char *lumpname, int position) if (!ForceNodeBuild) { // Check for compressed nodes first, then uncompressed nodes - FileRdr *fr; + FileRdr *fr = nullptr; uint32_t id = MAKE_ID('X','x','X','x'), idcheck = 0, idcheck2 = 0, idcheck3 = 0, idcheck4 = 0, idcheck5 = 0, idcheck6 = 0; if (map->Size(ML_ZNODES) != 0) @@ -3874,7 +3874,7 @@ void P_SetupLevel (const char *lumpname, int position) idcheck6 = MAKE_ID('X','G','L','3'); } - fr->Read (&id, 4); + if (fr != nullptr && fr->isOpen()) fr->Read (&id, 4); if (id != 0 && (id == idcheck || id == idcheck2 || id == idcheck3 || id == idcheck4 || id == idcheck5 || id == idcheck6)) { try diff --git a/src/resourcefiles/file_zip.cpp b/src/resourcefiles/file_zip.cpp index a2015ac58..684a3424f 100644 --- a/src/resourcefiles/file_zip.cpp +++ b/src/resourcefiles/file_zip.cpp @@ -124,9 +124,7 @@ static uint32_t Zip_FindCentralDir(FileRdr &fin) uint32_t uMaxBack; // maximum size of global comment uint32_t uPosFound=0; - fin.Seek(0, FileRdr::SeekEnd); - - FileSize = (uint32_t)fin.Tell(); + FileSize = (uint32_t)fin.GetLength(); uMaxBack = MIN(0xffff, FileSize); uBackRead = 4; diff --git a/src/sound/oalsound.cpp b/src/sound/oalsound.cpp index ac2d346e9..a08077844 100644 --- a/src/sound/oalsound.cpp +++ b/src/sound/oalsound.cpp @@ -290,7 +290,7 @@ class OpenALSoundStream : public SoundStream public: OpenALSoundStream(OpenALSoundRenderer *renderer) - : Renderer(renderer), Source(0), Playing(false), Looping(false), Volume(1.0f), Reader(NULL), Decoder(NULL) + : Renderer(renderer), Source(0), Playing(false), Looping(false), Volume(1.0f), Decoder(NULL) { memset(Buffers, 0, sizeof(Buffers)); Renderer->AddStream(this);