- 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

@@ -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;

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;

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
+long DecompressorBase::Seek (long offset, int origin)
-	{
+{
 	I_Error("Cannot seek in decompressor stream");
 	return 0;
-	}
+}
-	virtual char *Gets(char *strbuf, int len) override
+char *DecompressorBase::Gets(char *strbuf, int len)
-	{
+{
 	I_Error("Cannot use Gets on decompressor stream");
 	return nullptr;
-	}
+}
-};
 
 //==========================================================================
 //

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);
 }

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

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.GetLength();
-
-	FileSize = (uint32_t)fin.Tell();
 	uMaxBack = MIN<uint32_t>(0xffff, FileSize);
 
 	uBackRead = 4;

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);