From c4387459bbe6613f2be878143403edbbf846257b Mon Sep 17 00:00:00 2001 From: Christoph Oelckers Date: Sat, 10 Mar 2018 19:22:26 +0100 Subject: [PATCH] - use FileRdr in the PNG code, in particular to sanitize the savepic handling. --- src/files.h | 5 +++ src/m_png.cpp | 41 ++++++++++------------ src/m_png.h | 12 +++---- src/menu/loadsavemenu.cpp | 52 ++++++++++++---------------- src/menu/menu.h | 1 - src/textures/pngtexture.cpp | 68 +++++++++++++++++++------------------ 6 files changed, 84 insertions(+), 95 deletions(-) diff --git a/src/files.h b/src/files.h index c8a330315..b40526c2c 100644 --- a/src/files.h +++ b/src/files.h @@ -545,6 +545,11 @@ public: Close(); } + bool isOpen() const + { + return mReader != nullptr; + } + void Close() { if (mReader != nullptr) delete mReader; diff --git a/src/m_png.cpp b/src/m_png.cpp index a7f82ed8f..63e57e35c 100644 --- a/src/m_png.cpp +++ b/src/m_png.cpp @@ -75,22 +75,17 @@ struct IHDR uint8_t Interlace; }; -PNGHandle::PNGHandle (FILE *file) : File(0), bDeleteFilePtr(true), ChunkPt(0) +PNGHandle::PNGHandle (FileRdr &file) : File(0), bDeleteFilePtr(true), ChunkPt(0) { - File = new FileReader(file); + File = std::move(file); } -PNGHandle::PNGHandle (FileReader *file, bool takereader) : File(file), bDeleteFilePtr(takereader), ChunkPt(0) {} PNGHandle::~PNGHandle () { for (unsigned int i = 0; i < TextChunks.Size(); ++i) { delete[] TextChunks[i]; } - if (bDeleteFilePtr) - { - delete File; - } } // EXTERNAL FUNCTION PROTOTYPES -------------------------------------------- @@ -309,7 +304,7 @@ unsigned int M_NextPNGChunk (PNGHandle *png, uint32_t id) { if (png->Chunks[png->ChunkPt].ID == id) { // Found the chunk - png->File->Seek (png->Chunks[png->ChunkPt++].Offset, SEEK_SET); + png->File.Seek (png->Chunks[png->ChunkPt++].Offset, FileRdr::SeekSet); return png->Chunks[png->ChunkPt - 1].Size; } } @@ -374,46 +369,43 @@ bool M_GetPNGText (PNGHandle *png, const char *keyword, char *buffer, size_t buf // //========================================================================== -PNGHandle *M_VerifyPNG (FileReader *filer, bool takereader) +PNGHandle *M_VerifyPNG (FileRdr &filer) { PNGHandle::Chunk chunk; PNGHandle *png; uint32_t data[2]; bool sawIDAT = false; - if (filer->Read(&data, 8) != 8) + if (filer.Read(&data, 8) != 8) { - if (takereader) delete filer; return NULL; } if (data[0] != MAKE_ID(137,'P','N','G') || data[1] != MAKE_ID(13,10,26,10)) { // Does not have PNG signature - if (takereader) delete filer; return NULL; } - if (filer->Read (&data, 8) != 8) + if (filer.Read (&data, 8) != 8) { - if (takereader) delete filer; return NULL; } if (data[1] != MAKE_ID('I','H','D','R')) { // IHDR must be the first chunk - if (takereader) delete filer; return NULL; } // It looks like a PNG so far, so start creating a PNGHandle for it - png = new PNGHandle (filer, takereader); + png = new PNGHandle (filer); + // filer is no longer valid after the above line! chunk.ID = data[1]; chunk.Offset = 16; chunk.Size = BigLong((unsigned int)data[0]); png->Chunks.Push (chunk); - filer->Seek (16, SEEK_SET); + png->File.Seek (16, FileRdr::SeekSet); - while (filer->Seek (chunk.Size + 4, SEEK_CUR) == 0) + while (png->File.Seek (chunk.Size + 4, FileRdr::SeekCur) == 0) { // If the file ended before an IEND was encountered, it's not a PNG. - if (filer->Read (&data, 8) != 8) + if (png->File.Read (&data, 8) != 8) { break; } @@ -432,7 +424,7 @@ PNGHandle *M_VerifyPNG (FileReader *filer, bool takereader) sawIDAT = true; } chunk.ID = data[1]; - chunk.Offset = filer->Tell(); + chunk.Offset = (uint32_t)png->File.Tell(); chunk.Size = BigLong((unsigned int)data[0]); png->Chunks.Push (chunk); @@ -441,7 +433,7 @@ PNGHandle *M_VerifyPNG (FileReader *filer, bool takereader) { char *str = new char[chunk.Size + 1]; - if (filer->Read (str, chunk.Size) != (long)chunk.Size) + if (png->File.Read (str, chunk.Size) != chunk.Size) { delete[] str; break; @@ -452,6 +444,7 @@ PNGHandle *M_VerifyPNG (FileReader *filer, bool takereader) } } + filer = std::move(png->File); // need to get the reader back if this function failed. delete png; return NULL; } @@ -477,7 +470,7 @@ void M_FreePNG (PNGHandle *png) // //========================================================================== -bool M_ReadIDAT (FileReader *file, uint8_t *buffer, int width, int height, int pitch, +bool M_ReadIDAT (FileRdr &file, uint8_t *buffer, int width, int height, int pitch, uint8_t bitdepth, uint8_t colortype, uint8_t interlace, unsigned int chunklen) { // Uninterlaced images are treated as a conceptual eighth pass by these tables. @@ -574,7 +567,7 @@ bool M_ReadIDAT (FileReader *file, uint8_t *buffer, int width, int height, int p if (stream.avail_in == 0 && chunklen > 0) { stream.next_in = chunkbuffer; - stream.avail_in = (uInt)file->Read (chunkbuffer, MIN(chunklen,sizeof(chunkbuffer))); + stream.avail_in = (uInt)file.Read (chunkbuffer, MIN(chunklen,sizeof(chunkbuffer))); chunklen -= stream.avail_in; } @@ -666,7 +659,7 @@ bool M_ReadIDAT (FileReader *file, uint8_t *buffer, int width, int height, int p { uint32_t x[3]; - if (file->Read (x, 12) != 12) + if (file.Read (x, 12) != 12) { lastIDAT = true; } diff --git a/src/m_png.h b/src/m_png.h index 7bc9b474f..814e7b281 100644 --- a/src/m_png.h +++ b/src/m_png.h @@ -36,8 +36,8 @@ #include #include "doomtype.h" #include "v_video.h" +#include "files.h" -class FileReader; class FileWriter; // PNG Writing -------------------------------------------------------------- @@ -64,7 +64,6 @@ bool M_SaveBitmap(const uint8_t *from, ESSType color_type, int width, int height // PNG Reading -------------------------------------------------------------- -class FileReader; struct PNGHandle { struct Chunk @@ -74,14 +73,13 @@ struct PNGHandle uint32_t Size; }; - FileReader *File; + FileRdr File; bool bDeleteFilePtr; TArray Chunks; TArray TextChunks; unsigned int ChunkPt; - PNGHandle(FILE *file); - PNGHandle(FileReader *file, bool takereader = false); + PNGHandle(FileRdr &file); ~PNGHandle(); }; @@ -89,7 +87,7 @@ struct PNGHandle // the signature, but also checking for the IEND chunk. CRC checking of // each chunk is not done. If it is valid, you get a PNGHandle to pass to // the following functions. -PNGHandle *M_VerifyPNG (FileReader *file, bool takereader = false); +PNGHandle *M_VerifyPNG (FileRdr &file); // Finds a chunk in a PNG file. The file pointer will be positioned at the // beginning of the chunk data, and its length will be returned. A return @@ -108,7 +106,7 @@ bool M_GetPNGText (PNGHandle *png, const char *keyword, char *buffer, size_t buf // The file must be positioned at the start of the first IDAT. It reads // image data into the provided buffer. Returns true on success. -bool M_ReadIDAT (FileReader *file, uint8_t *buffer, int width, int height, int pitch, +bool M_ReadIDAT (FileRdr &file, uint8_t *buffer, int width, int height, int pitch, uint8_t bitdepth, uint8_t colortype, uint8_t interlace, unsigned int idatlen); diff --git a/src/menu/loadsavemenu.cpp b/src/menu/loadsavemenu.cpp index c9fb7aeba..d66a3ab07 100644 --- a/src/menu/loadsavemenu.cpp +++ b/src/menu/loadsavemenu.cpp @@ -236,8 +236,8 @@ void FSavegameManager::ReadSaveStrings() } else // check for old formats. { - FileReader file; - if (file.Open(filepath)) + FileRdr file; + if (file.OpenFile(filepath)) { PNGHandle *png; char sig[16]; @@ -255,7 +255,7 @@ void FSavegameManager::ReadSaveStrings() title[OLDSAVESTRINGSIZE] = 0; - if (nullptr != (png = M_VerifyPNG(&file, false))) + if (nullptr != (png = M_VerifyPNG(file))) { char *ver = M_GetPNGText(png, "ZDoom Save Version"); if (ver != nullptr) @@ -272,7 +272,7 @@ void FSavegameManager::ReadSaveStrings() } else { - file.Seek(0, SEEK_SET); + file.Seek(0, FileRdr::SeekSet); if (file.Read(sig, 16) == 16) { @@ -496,29 +496,26 @@ unsigned FSavegameManager::ExtractSaveData(int index) FResourceLump *pic = resf->FindLump("savepic.png"); if (pic != nullptr) { - FileReader *reader = pic->NewReader(); - if (reader != nullptr) + FileRdr picreader; + + picreader.OpenMemoryArray([=](TArray &array) { - // copy to a memory buffer which gets accessed through a memory reader and PNGHandle. - // We cannot use the actual lump as backing for the texture because that requires keeping the - // savegame file open. - SavePicData.Resize(pic->LumpSize); - reader->Read(&SavePicData[0], pic->LumpSize); - reader = new MemoryReader(&SavePicData[0], SavePicData.Size()); - PNGHandle *png = M_VerifyPNG(reader); - if (png != nullptr) + auto cache = pic->CacheLump(); + array.Resize(pic->LumpSize); + memcpy(&array[0], cache, pic->LumpSize); + pic->ReleaseCache(); + return true; + }); + PNGHandle *png = M_VerifyPNG(picreader); + if (png != nullptr) + { + SavePic = PNGTexture_CreateFromFile(png, node->Filename); + delete png; + if (SavePic->GetWidth() == 1 && SavePic->GetHeight() == 1) { - SavePic = PNGTexture_CreateFromFile(png, node->Filename); - currentSavePic = reader; // must be kept so that the texture can read from it. - delete png; - if (SavePic->GetWidth() == 1 && SavePic->GetHeight() == 1) - { - delete SavePic; - SavePic = nullptr; - delete currentSavePic; - currentSavePic = nullptr; - SavePicData.Clear(); - } + delete SavePic; + SavePic = nullptr; + SavePicData.Clear(); } } } @@ -551,14 +548,9 @@ void FSavegameManager::UnloadSaveData() { V_FreeBrokenLines(SaveComment); } - if (currentSavePic != nullptr) - { - delete currentSavePic; - } SavePic = nullptr; SaveComment = nullptr; - currentSavePic = nullptr; SavePicData.Clear(); } diff --git a/src/menu/menu.h b/src/menu/menu.h index d5211421f..71595903c 100644 --- a/src/menu/menu.h +++ b/src/menu/menu.h @@ -71,7 +71,6 @@ private: FSaveGameNode NewSaveNode; int LastSaved = -1; int LastAccessed = -1; - FileReader *currentSavePic = nullptr; TArray SavePicData; FTexture *SavePic = nullptr; FBrokenLines *SaveComment = nullptr; diff --git a/src/textures/pngtexture.cpp b/src/textures/pngtexture.cpp index 7ca1a32c6..a7e2d0534 100644 --- a/src/textures/pngtexture.cpp +++ b/src/textures/pngtexture.cpp @@ -51,7 +51,7 @@ class FPNGTexture : public FTexture { public: - FPNGTexture (FileReader &lump, int lumpnum, const FString &filename, int width, int height, uint8_t bitdepth, uint8_t colortype, uint8_t interlace); + FPNGTexture (FileRdr &lump, int lumpnum, const FString &filename, int width, int height, uint8_t bitdepth, uint8_t colortype, uint8_t interlace); ~FPNGTexture (); const uint8_t *GetColumn (unsigned int column, const Span **spans_out); @@ -66,7 +66,7 @@ protected: FString SourceFile; uint8_t *Pixels; Span **Spans; - FileReader *fr; + FileRdr fr; uint8_t BitDepth; uint8_t ColorType; @@ -147,7 +147,7 @@ FTexture *PNGTexture_TryCreate(FileRdr & data, int lumpnum) } } - return new FPNGTexture (*data.Reader(), lumpnum, FString(), BigLong((int)width), BigLong((int)height), + return new FPNGTexture (data, lumpnum, FString(), BigLong((int)width), BigLong((int)height), bitdepth, colortype, interlace); } @@ -168,9 +168,9 @@ FTexture *PNGTexture_CreateFromFile(PNGHandle *png, const FString &filename) } // Check the IHDR to make sure it's a type of PNG we support. - png->File->Read(&width, 4); - png->File->Read(&height, 4); - (*png->File) >> bitdepth >> colortype >> compression >> filter >> interlace; + png->File.Read(&width, 4); + png->File.Read(&height, 4); + png->File >> bitdepth >> colortype >> compression >> filter >> interlace; if (compression != 0 || filter != 0 || interlace > 1) { @@ -185,7 +185,7 @@ FTexture *PNGTexture_CreateFromFile(PNGHandle *png, const FString &filename) return NULL; } - return new FPNGTexture (*png->File, -1, filename, BigLong((int)width), BigLong((int)height), + return new FPNGTexture (png->File, -1, filename, BigLong((int)width), BigLong((int)height), bitdepth, colortype, interlace); } @@ -195,7 +195,7 @@ FTexture *PNGTexture_CreateFromFile(PNGHandle *png, const FString &filename) // //========================================================================== -FPNGTexture::FPNGTexture (FileReader &lump, int lumpnum, const FString &filename, int width, int height, +FPNGTexture::FPNGTexture (FileRdr &lump, int lumpnum, const FString &filename, int width, int height, uint8_t depth, uint8_t colortype, uint8_t interlace) : FTexture(NULL, lumpnum), SourceFile(filename), Pixels(0), Spans(0), BitDepth(depth), ColorType(colortype), Interlace(interlace), HaveTrans(false), @@ -210,9 +210,6 @@ FPNGTexture::FPNGTexture (FileReader &lump, int lumpnum, const FString &filename uint32_t len, id; int i; - if (lumpnum == -1) fr = &lump; - else fr = nullptr; - UseType = TEX_MiscPatch; LeftOffset = 0; TopOffset = 0; @@ -225,7 +222,7 @@ FPNGTexture::FPNGTexture (FileReader &lump, int lumpnum, const FString &filename memset(trans, 255, 256); // Parse pre-IDAT chunks. I skip the CRCs. Is that bad? - lump.Seek(33, SEEK_SET); + lump.Seek(33, FileRdr::SeekSet); lump.Read(&len, 4); lump.Read(&id, 4); @@ -235,7 +232,7 @@ FPNGTexture::FPNGTexture (FileReader &lump, int lumpnum, const FString &filename switch (id) { default: - lump.Seek (len, SEEK_CUR); + lump.Seek (len, FileRdr::SeekCur); break; case MAKE_ID('g','r','A','b'): @@ -268,7 +265,7 @@ FPNGTexture::FPNGTexture (FileReader &lump, int lumpnum, const FString &filename lump.Read (p.pngpal, PaletteSize * 3); if (PaletteSize * 3 != (int)len) { - lump.Seek (len - PaletteSize * 3, SEEK_CUR); + lump.Seek (len - PaletteSize * 3, FileRdr::SeekCur); } for (i = PaletteSize - 1; i >= 0; --i) { @@ -290,12 +287,12 @@ FPNGTexture::FPNGTexture (FileReader &lump, int lumpnum, const FString &filename bMasked = true; break; } - lump.Seek(4, SEEK_CUR); // Skip CRC + lump.Seek(4, FileRdr::SeekCur); // Skip CRC lump.Read(&len, 4); id = MAKE_ID('I','E','N','D'); lump.Read(&id, 4); } - StartOfIDAT = lump.Tell() - 8; + StartOfIDAT = (uint32_t)lump.Tell() - 8; switch (colortype) { @@ -342,6 +339,8 @@ FPNGTexture::FPNGTexture (FileReader &lump, int lumpnum, const FString &filename bMasked = HaveTrans; break; } + // If this is a savegame we must keep the reader. + if (lumpnum == -1) fr = std::move(lump); } //========================================================================== @@ -457,15 +456,17 @@ const uint8_t *FPNGTexture::GetPixels () void FPNGTexture::MakeTexture () { - FileReader *lump; + FileRdr *lump; + FileRdr lfr; if (SourceLump >= 0) { - lump = new FWadLump(Wads.OpenLumpNum(SourceLump)); + lfr = Wads.OpenLumpReader(SourceLump); + lump = 𝔩 } else { - lump = fr;// new FileReader(SourceFile.GetChars()); + lump = &fr; } Pixels = new uint8_t[Width*Height]; @@ -476,13 +477,13 @@ void FPNGTexture::MakeTexture () else { uint32_t len, id; - lump->Seek (StartOfIDAT, SEEK_SET); + lump->Seek (StartOfIDAT, FileRdr::SeekSet); lump->Read(&len, 4); lump->Read(&id, 4); if (ColorType == 0 || ColorType == 3) /* Grayscale and paletted */ { - M_ReadIDAT (lump, Pixels, Width, Height, Width, BitDepth, ColorType, Interlace, BigLong((unsigned int)len)); + M_ReadIDAT (*lump, Pixels, Width, Height, Width, BitDepth, ColorType, Interlace, BigLong((unsigned int)len)); if (Width == Height) { @@ -518,7 +519,7 @@ void FPNGTexture::MakeTexture () uint8_t *in, *out; int x, y, pitch, backstep; - M_ReadIDAT (lump, tempix, Width, Height, Width*bytesPerPixel, BitDepth, ColorType, Interlace, BigLong((unsigned int)len)); + M_ReadIDAT (*lump, tempix, Width, Height, Width*bytesPerPixel, BitDepth, ColorType, Interlace, BigLong((unsigned int)len)); in = tempix; out = Pixels; @@ -602,7 +603,6 @@ void FPNGTexture::MakeTexture () delete[] tempix; } } - if (lump != fr) delete lump; } //=========================================================================== @@ -616,21 +616,24 @@ int FPNGTexture::CopyTrueColorPixels(FBitmap *bmp, int x, int y, int rotate, FCo // Parse pre-IDAT chunks. I skip the CRCs. Is that bad? PalEntry pe[256]; uint32_t len, id; - FileReader *lump; static char bpp[] = {1, 0, 3, 1, 2, 0, 4}; int pixwidth = Width * bpp[ColorType]; int transpal = false; + FileRdr *lump; + FileRdr lfr; + if (SourceLump >= 0) { - lump = new FWadLump(Wads.OpenLumpNum(SourceLump)); + lfr = Wads.OpenLumpReader(SourceLump); + lump = 𝔩 } else { - lump = fr;// new FileReader(SourceFile.GetChars()); + lump = &fr; } - lump->Seek(33, SEEK_SET); + lump->Seek(33, FileRdr::SeekSet); for(int i = 0; i < 256; i++) // default to a gray map pe[i] = PalEntry(255,i,i,i); @@ -642,7 +645,7 @@ int FPNGTexture::CopyTrueColorPixels(FBitmap *bmp, int x, int y, int rotate, FCo switch (id) { default: - lump->Seek (len, SEEK_CUR); + lump->Seek (len, FileRdr::SeekCur); break; case MAKE_ID('P','L','T','E'): @@ -664,11 +667,11 @@ int FPNGTexture::CopyTrueColorPixels(FBitmap *bmp, int x, int y, int rotate, FCo } else { - lump->Seek(len, SEEK_CUR); + lump->Seek(len, FileRdr::SeekCur); } break; } - lump->Seek(4, SEEK_CUR); // Skip CRC + lump->Seek(4, FileRdr::SeekCur); // Skip CRC lump->Read(&len, 4); id = MAKE_ID('I','E','N','D'); lump->Read(&id, 4); @@ -682,11 +685,10 @@ int FPNGTexture::CopyTrueColorPixels(FBitmap *bmp, int x, int y, int rotate, FCo uint8_t * Pixels = new uint8_t[pixwidth * Height]; - lump->Seek (StartOfIDAT, SEEK_SET); + lump->Seek (StartOfIDAT, FileRdr::SeekSet); lump->Read(&len, 4); lump->Read(&id, 4); - M_ReadIDAT (lump, Pixels, Width, Height, pixwidth, BitDepth, ColorType, Interlace, BigLong((unsigned int)len)); - if (lump != fr) delete lump; + M_ReadIDAT (*lump, Pixels, Width, Height, pixwidth, BitDepth, ColorType, Interlace, BigLong((unsigned int)len)); switch (ColorType) {