From 6b464a9330270d8df86d35271c11e8aba7abdc6f Mon Sep 17 00:00:00 2001 From: elsid Date: Sun, 10 Apr 2022 13:35:00 +0200 Subject: [PATCH 1/2] Check ESMReader value size in compile time --- apps/essimporter/converter.cpp | 2 +- apps/essimporter/importcellref.cpp | 4 ++-- apps/essimporter/importplayer.cpp | 2 +- components/esm3/cellid.cpp | 2 +- components/esm3/cellref.cpp | 4 ++-- components/esm3/effectlist.cpp | 2 +- components/esm3/esmreader.hpp | 25 +++++++++++-------------- components/esm3/loadalch.cpp | 2 +- components/esm3/loadarmo.cpp | 2 +- components/esm3/loadbody.cpp | 2 +- components/esm3/loadbook.cpp | 2 +- components/esm3/loadcell.cpp | 2 +- components/esm3/loadclas.cpp | 2 +- components/esm3/loadclot.cpp | 2 +- components/esm3/loadcont.cpp | 4 ++-- components/esm3/loadcrea.cpp | 2 +- components/esm3/loadench.cpp | 2 +- components/esm3/loadfact.cpp | 2 +- components/esm3/loadinfo.cpp | 2 +- components/esm3/loadingr.cpp | 2 +- components/esm3/loadligh.cpp | 2 +- components/esm3/loadlock.cpp | 2 +- components/esm3/loadmgef.cpp | 2 +- components/esm3/loadmisc.cpp | 2 +- components/esm3/loadpgrd.cpp | 2 +- components/esm3/loadprob.cpp | 2 +- components/esm3/loadrace.cpp | 2 +- components/esm3/loadrepa.cpp | 2 +- components/esm3/loadskil.cpp | 2 +- components/esm3/loadsndg.cpp | 2 +- components/esm3/loadsoun.cpp | 2 +- components/esm3/loadspel.cpp | 4 ++-- components/esm3/loadweap.cpp | 2 +- components/esm3/player.cpp | 4 ++-- components/esm3/savedgame.cpp | 2 +- 35 files changed, 50 insertions(+), 53 deletions(-) diff --git a/apps/essimporter/converter.cpp b/apps/essimporter/converter.cpp index ad28a9295f..0d5b391112 100644 --- a/apps/essimporter/converter.cpp +++ b/apps/essimporter/converter.cpp @@ -278,7 +278,7 @@ namespace ESSImport while (esm.isNextSub("MPCD")) { float notepos[3]; - esm.getHT(notepos, 3*sizeof(float)); + esm.getHTSized<3 * sizeof(float)>(notepos); // Markers seem to be arranged in a 32*32 grid, notepos has grid-indices. // This seems to be the reason markers can't be placed everywhere in interior cells, diff --git a/apps/essimporter/importcellref.cpp b/apps/essimporter/importcellref.cpp index e6c62cc812..22af1af5fa 100644 --- a/apps/essimporter/importcellref.cpp +++ b/apps/essimporter/importcellref.cpp @@ -35,8 +35,8 @@ namespace ESSImport // DATA should occur for all references, except levelled creature spawners // I've seen DATA *twice* on a creature record, and with the exact same content too! weird // alarmvoi0000.ess - esm.getHNOT(mPos, "DATA", 24); - esm.getHNOT(mPos, "DATA", 24); + esm.getHNOTSized<24>(mPos, "DATA"); + esm.getHNOTSized<24>(mPos, "DATA"); mDeleted = 0; if (esm.isNextSub("DELE")) diff --git a/apps/essimporter/importplayer.cpp b/apps/essimporter/importplayer.cpp index 52e4a9b7d0..f2ec57ab3f 100644 --- a/apps/essimporter/importplayer.cpp +++ b/apps/essimporter/importplayer.cpp @@ -13,7 +13,7 @@ namespace ESSImport mActorData.load(esm); - esm.getHNOT(mPos, "DATA", 24); + esm.getHNOTSized<24>(mPos, "DATA"); } void PCDT::load(ESM::ESMReader &esm) diff --git a/components/esm3/cellid.cpp b/components/esm3/cellid.cpp index ad91d30e04..87ad0cc21c 100644 --- a/components/esm3/cellid.cpp +++ b/components/esm3/cellid.cpp @@ -11,7 +11,7 @@ void ESM::CellId::load (ESMReader &esm) if (esm.isNextSub ("CIDX")) { - esm.getHT (mIndex, 8); + esm.getHTSized<8>(mIndex); mPaged = true; } else diff --git a/components/esm3/cellref.cpp b/components/esm3/cellref.cpp index 2efa3b845d..9b46bf182a 100644 --- a/components/esm3/cellref.cpp +++ b/components/esm3/cellref.cpp @@ -114,7 +114,7 @@ namespace ESM break; case ESM::fourCC("DATA"): if constexpr (load) - esm.getHT(cellRef.mPos, 24); + esm.getHTSized<24>(cellRef.mPos); else esm.skip(24); break; @@ -150,7 +150,7 @@ namespace ESM void ESM::RefNum::load(ESMReader& esm, bool wide, ESM::NAME tag) { if (wide) - esm.getHNT(*this, tag, 8); + esm.getHNTSized<8>(*this, tag); else esm.getHNT(mIndex, tag); } diff --git a/components/esm3/effectlist.cpp b/components/esm3/effectlist.cpp index f6d5a6e071..1cfb25618d 100644 --- a/components/esm3/effectlist.cpp +++ b/components/esm3/effectlist.cpp @@ -16,7 +16,7 @@ void EffectList::load(ESMReader &esm) void EffectList::add(ESMReader &esm) { ENAMstruct s; - esm.getHT(s, 24); + esm.getHTSized<24>(s); mList.push_back(s); } diff --git a/components/esm3/esmreader.hpp b/components/esm3/esmreader.hpp index 464925d936..e931cde42a 100644 --- a/components/esm3/esmreader.hpp +++ b/components/esm3/esmreader.hpp @@ -2,7 +2,6 @@ #define OPENMW_ESM_READER_H #include -#include #include #include @@ -114,20 +113,18 @@ public: // Version with extra size checking, to make sure the compiler // doesn't mess up our struct padding. - template - void getHNT(X &x, NAME name, int size) + template + void getHNTSized(X &x, NAME name) { - assert(sizeof(X) == size); - getSubNameIs(name); - getHT(x); + static_assert(sizeof(X) == size); + getHNT(x, name); } - template - void getHNOT(X &x, NAME name, int size) + template + void getHNOTSized(X &x, NAME name) { - assert(sizeof(X) == size); - if(isNextSub(name)) - getHT(x); + static_assert(sizeof(X) == size); + getHNOT(x, name); } // Get data of a given type/size, including subrecord header @@ -151,10 +148,10 @@ public: // Version with extra size checking, to make sure the compiler // doesn't mess up our struct padding. - template - void getHT(X &x, int size) + template + void getHTSized(X &x) { - assert(sizeof(X) == size); + static_assert(sizeof(X) == size); getHT(x); } diff --git a/components/esm3/loadalch.cpp b/components/esm3/loadalch.cpp index 1aacfc8976..7173090e4b 100644 --- a/components/esm3/loadalch.cpp +++ b/components/esm3/loadalch.cpp @@ -39,7 +39,7 @@ namespace ESM mName = esm.getHString(); break; case ESM::fourCC("ALDT"): - esm.getHT(mData, 12); + esm.getHTSized<12>(mData); hasData = true; break; case ESM::fourCC("ENAM"): diff --git a/components/esm3/loadarmo.cpp b/components/esm3/loadarmo.cpp index b476e930a8..7176f82f1e 100644 --- a/components/esm3/loadarmo.cpp +++ b/components/esm3/loadarmo.cpp @@ -63,7 +63,7 @@ namespace ESM mName = esm.getHString(); break; case ESM::fourCC("AODT"): - esm.getHT(mData, 24); + esm.getHTSized<24>(mData); hasData = true; break; case ESM::fourCC("SCRI"): diff --git a/components/esm3/loadbody.cpp b/components/esm3/loadbody.cpp index 148e80d94c..55aa5b7bf5 100644 --- a/components/esm3/loadbody.cpp +++ b/components/esm3/loadbody.cpp @@ -31,7 +31,7 @@ namespace ESM mRace = esm.getHString(); break; case ESM::fourCC("BYDT"): - esm.getHT(mData, 4); + esm.getHTSized<4>(mData); hasData = true; break; case ESM::SREC_DELE: diff --git a/components/esm3/loadbook.cpp b/components/esm3/loadbook.cpp index 72618f3ce8..7e5f75e055 100644 --- a/components/esm3/loadbook.cpp +++ b/components/esm3/loadbook.cpp @@ -31,7 +31,7 @@ namespace ESM mName = esm.getHString(); break; case ESM::fourCC("BKDT"): - esm.getHT(mData, 20); + esm.getHTSized<20>(mData); hasData = true; break; case ESM::fourCC("SCRI"): diff --git a/components/esm3/loadcell.cpp b/components/esm3/loadcell.cpp index 1a4fe1db8f..b36f066501 100644 --- a/components/esm3/loadcell.cpp +++ b/components/esm3/loadcell.cpp @@ -76,7 +76,7 @@ namespace ESM mName = esm.getHString(); break; case ESM::fourCC("DATA"): - esm.getHT(mData, 12); + esm.getHTSized<12>(mData); hasData = true; break; case ESM::SREC_DELE: diff --git a/components/esm3/loadclas.cpp b/components/esm3/loadclas.cpp index ecd43796de..718ba6c3ac 100644 --- a/components/esm3/loadclas.cpp +++ b/components/esm3/loadclas.cpp @@ -58,7 +58,7 @@ namespace ESM mName = esm.getHString(); break; case ESM::fourCC("CLDT"): - esm.getHT(mData, 60); + esm.getHTSized<60>(mData); if (mData.mIsPlayable > 1) esm.fail("Unknown bool value"); hasData = true; diff --git a/components/esm3/loadclot.cpp b/components/esm3/loadclot.cpp index 1bd5db9655..5b6fa4210b 100644 --- a/components/esm3/loadclot.cpp +++ b/components/esm3/loadclot.cpp @@ -33,7 +33,7 @@ namespace ESM mName = esm.getHString(); break; case ESM::fourCC("CTDT"): - esm.getHT(mData, 12); + esm.getHTSized<12>(mData); hasData = true; break; case ESM::fourCC("SCRI"): diff --git a/components/esm3/loadcont.cpp b/components/esm3/loadcont.cpp index 5cbdd80428..6477bb9301 100644 --- a/components/esm3/loadcont.cpp +++ b/components/esm3/loadcont.cpp @@ -55,11 +55,11 @@ namespace ESM mName = esm.getHString(); break; case ESM::fourCC("CNDT"): - esm.getHT(mWeight, 4); + esm.getHTSized<4>(mWeight); hasWeight = true; break; case ESM::fourCC("FLAG"): - esm.getHT(mFlags, 4); + esm.getHTSized<4>(mFlags); if (mFlags & 0xf4) esm.fail("Unknown flags"); if (!(mFlags & 0x8)) diff --git a/components/esm3/loadcrea.cpp b/components/esm3/loadcrea.cpp index f1203ed39e..f82c488c63 100644 --- a/components/esm3/loadcrea.cpp +++ b/components/esm3/loadcrea.cpp @@ -50,7 +50,7 @@ namespace ESM { mScript = esm.getHString(); break; case ESM::fourCC("NPDT"): - esm.getHT(mData, 96); + esm.getHTSized<96>(mData); hasNpdt = true; break; case ESM::fourCC("FLAG"): diff --git a/components/esm3/loadench.cpp b/components/esm3/loadench.cpp index 2e138444f3..4280b3a8af 100644 --- a/components/esm3/loadench.cpp +++ b/components/esm3/loadench.cpp @@ -26,7 +26,7 @@ namespace ESM hasName = true; break; case ESM::fourCC("ENDT"): - esm.getHT(mData, 16); + esm.getHTSized<16>(mData); hasData = true; break; case ESM::fourCC("ENAM"): diff --git a/components/esm3/loadfact.cpp b/components/esm3/loadfact.cpp index 4d4697d93b..9fc5410164 100644 --- a/components/esm3/loadfact.cpp +++ b/components/esm3/loadfact.cpp @@ -56,7 +56,7 @@ namespace ESM mRanks[rankCounter++] = esm.getHString(); break; case ESM::fourCC("FADT"): - esm.getHT(mData, 240); + esm.getHTSized<240>(mData); if (mData.mIsHidden > 1) esm.fail("Unknown flag!"); hasData = true; diff --git a/components/esm3/loadinfo.cpp b/components/esm3/loadinfo.cpp index 0b5030bb82..fb32f80188 100644 --- a/components/esm3/loadinfo.cpp +++ b/components/esm3/loadinfo.cpp @@ -26,7 +26,7 @@ namespace ESM switch (esm.retSubName().toInt()) { case ESM::fourCC("DATA"): - esm.getHT(mData, 12); + esm.getHTSized<12>(mData); break; case ESM::fourCC("ONAM"): mActor = esm.getHString(); diff --git a/components/esm3/loadingr.cpp b/components/esm3/loadingr.cpp index 173e2c5636..d7317a8dc4 100644 --- a/components/esm3/loadingr.cpp +++ b/components/esm3/loadingr.cpp @@ -31,7 +31,7 @@ namespace ESM mName = esm.getHString(); break; case ESM::fourCC("IRDT"): - esm.getHT(mData, 56); + esm.getHTSized<56>(mData); hasData = true; break; case ESM::fourCC("SCRI"): diff --git a/components/esm3/loadligh.cpp b/components/esm3/loadligh.cpp index 94c109e677..7b3c6e8787 100644 --- a/components/esm3/loadligh.cpp +++ b/components/esm3/loadligh.cpp @@ -34,7 +34,7 @@ namespace ESM mIcon = esm.getHString(); break; case ESM::fourCC("LHDT"): - esm.getHT(mData, 24); + esm.getHTSized<24>(mData); hasData = true; break; case ESM::fourCC("SCRI"): diff --git a/components/esm3/loadlock.cpp b/components/esm3/loadlock.cpp index 50d27d4e1a..0bdcbba635 100644 --- a/components/esm3/loadlock.cpp +++ b/components/esm3/loadlock.cpp @@ -31,7 +31,7 @@ namespace ESM mName = esm.getHString(); break; case ESM::fourCC("LKDT"): - esm.getHT(mData, 16); + esm.getHTSized<16>(mData); hasData = true; break; case ESM::fourCC("SCRI"): diff --git a/components/esm3/loadmgef.cpp b/components/esm3/loadmgef.cpp index f15d51def6..b528e8bf9a 100644 --- a/components/esm3/loadmgef.cpp +++ b/components/esm3/loadmgef.cpp @@ -195,7 +195,7 @@ void MagicEffect::load(ESMReader &esm, bool &isDeleted) mId = indexToId (mIndex); - esm.getHNT(mData, "MEDT", 36); + esm.getHNTSized<36>(mData, "MEDT"); if (esm.getFormat() == 0) { // don't allow mods to change fixed flags in the legacy format diff --git a/components/esm3/loadmisc.cpp b/components/esm3/loadmisc.cpp index eee46ac8b0..bbf1ac0635 100644 --- a/components/esm3/loadmisc.cpp +++ b/components/esm3/loadmisc.cpp @@ -31,7 +31,7 @@ namespace ESM mName = esm.getHString(); break; case ESM::fourCC("MCDT"): - esm.getHT(mData, 12); + esm.getHTSized<12>(mData); hasData = true; break; case ESM::fourCC("SCRI"): diff --git a/components/esm3/loadpgrd.cpp b/components/esm3/loadpgrd.cpp index 9d0f3cf66c..ed2ab2c5d3 100644 --- a/components/esm3/loadpgrd.cpp +++ b/components/esm3/loadpgrd.cpp @@ -52,7 +52,7 @@ namespace ESM mCell = esm.getHString(); break; case ESM::fourCC("DATA"): - esm.getHT(mData, 12); + esm.getHTSized<12>(mData); hasData = true; break; case ESM::fourCC("PGRP"): diff --git a/components/esm3/loadprob.cpp b/components/esm3/loadprob.cpp index 10738c0863..f084732775 100644 --- a/components/esm3/loadprob.cpp +++ b/components/esm3/loadprob.cpp @@ -31,7 +31,7 @@ namespace ESM mName = esm.getHString(); break; case ESM::fourCC("PBDT"): - esm.getHT(mData, 16); + esm.getHTSized<16>(mData); hasData = true; break; case ESM::fourCC("SCRI"): diff --git a/components/esm3/loadrace.cpp b/components/esm3/loadrace.cpp index ba0046bc9b..9520b792ed 100644 --- a/components/esm3/loadrace.cpp +++ b/components/esm3/loadrace.cpp @@ -40,7 +40,7 @@ namespace ESM mName = esm.getHString(); break; case ESM::fourCC("RADT"): - esm.getHT(mData, 140); + esm.getHTSized<140>(mData); hasData = true; break; case ESM::fourCC("DESC"): diff --git a/components/esm3/loadrepa.cpp b/components/esm3/loadrepa.cpp index aaf518a1cd..16dd296a25 100644 --- a/components/esm3/loadrepa.cpp +++ b/components/esm3/loadrepa.cpp @@ -31,7 +31,7 @@ namespace ESM mName = esm.getHString(); break; case ESM::fourCC("RIDT"): - esm.getHT(mData, 16); + esm.getHTSized<16>(mData); hasData = true; break; case ESM::fourCC("SCRI"): diff --git a/components/esm3/loadskil.cpp b/components/esm3/loadskil.cpp index 902410fe5a..f7bb0c49d6 100644 --- a/components/esm3/loadskil.cpp +++ b/components/esm3/loadskil.cpp @@ -144,7 +144,7 @@ namespace ESM hasIndex = true; break; case ESM::fourCC("SKDT"): - esm.getHT(mData, 24); + esm.getHTSized<24>(mData); hasData = true; break; case ESM::fourCC("DESC"): diff --git a/components/esm3/loadsndg.cpp b/components/esm3/loadsndg.cpp index 2f5dea1eb4..5519d835b4 100644 --- a/components/esm3/loadsndg.cpp +++ b/components/esm3/loadsndg.cpp @@ -25,7 +25,7 @@ namespace ESM hasName = true; break; case ESM::fourCC("DATA"): - esm.getHT(mType, 4); + esm.getHTSized<4>(mType); hasData = true; break; case ESM::fourCC("CNAM"): diff --git a/components/esm3/loadsoun.cpp b/components/esm3/loadsoun.cpp index 453d4eebfc..0ddda2d6b2 100644 --- a/components/esm3/loadsoun.cpp +++ b/components/esm3/loadsoun.cpp @@ -28,7 +28,7 @@ namespace ESM mSound = esm.getHString(); break; case ESM::fourCC("DATA"): - esm.getHT(mData, 3); + esm.getHTSized<3>(mData); hasData = true; break; case ESM::SREC_DELE: diff --git a/components/esm3/loadspel.cpp b/components/esm3/loadspel.cpp index aeed60a1cb..f2891c2b6c 100644 --- a/components/esm3/loadspel.cpp +++ b/components/esm3/loadspel.cpp @@ -30,12 +30,12 @@ namespace ESM mName = esm.getHString(); break; case ESM::fourCC("SPDT"): - esm.getHT(mData, 12); + esm.getHTSized<12>(mData); hasData = true; break; case ESM::fourCC("ENAM"): ENAMstruct s; - esm.getHT(s, 24); + esm.getHTSized<24>(s); mEffects.mList.push_back(s); break; case ESM::SREC_DELE: diff --git a/components/esm3/loadweap.cpp b/components/esm3/loadweap.cpp index e75ecf7516..452036cb95 100644 --- a/components/esm3/loadweap.cpp +++ b/components/esm3/loadweap.cpp @@ -31,7 +31,7 @@ namespace ESM mName = esm.getHString(); break; case ESM::fourCC("WPDT"): - esm.getHT(mData, 32); + esm.getHTSized<32>(mData); hasData = true; break; case ESM::fourCC("SCRI"): diff --git a/components/esm3/player.cpp b/components/esm3/player.cpp index 028a042809..1839a45749 100644 --- a/components/esm3/player.cpp +++ b/components/esm3/player.cpp @@ -10,12 +10,12 @@ void ESM::Player::load (ESMReader &esm) mCellId.load (esm); - esm.getHNT (mLastKnownExteriorPosition, "LKEP", 12); + esm.getHNTSized<12>(mLastKnownExteriorPosition, "LKEP"); if (esm.isNextSub ("MARK")) { mHasMark = true; - esm.getHT (mMarkedPosition, 24); + esm.getHTSized<24>(mMarkedPosition); mMarkedCell.load (esm); } else diff --git a/components/esm3/savedgame.cpp b/components/esm3/savedgame.cpp index 667dbdfbce..94ea075759 100644 --- a/components/esm3/savedgame.cpp +++ b/components/esm3/savedgame.cpp @@ -15,7 +15,7 @@ void ESM::SavedGame::load (ESMReader &esm) mPlayerClassName = esm.getHNOString("PLCN"); mPlayerCell = esm.getHNString("PLCE"); - esm.getHNT (mInGameTime, "TSTM", 16); + esm.getHNTSized<16>(mInGameTime, "TSTM"); esm.getHNT (mTimePlayed, "TIME"); mDescription = esm.getHNString ("DESC"); From c3a924de2384922b154ad6d460aac4a5ca466ac9 Mon Sep 17 00:00:00 2001 From: elsid Date: Sun, 10 Apr 2022 13:35:54 +0200 Subject: [PATCH 2/2] Fix skip DATA in cell ref loading --- components/esm3/cellref.cpp | 2 +- components/esm3/esmreader.hpp | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/components/esm3/cellref.cpp b/components/esm3/cellref.cpp index 9b46bf182a..d5ed0b4003 100644 --- a/components/esm3/cellref.cpp +++ b/components/esm3/cellref.cpp @@ -116,7 +116,7 @@ namespace ESM if constexpr (load) esm.getHTSized<24>(cellRef.mPos); else - esm.skip(24); + esm.skipHTSized<24, decltype(cellRef.mPos)>(); break; case ESM::fourCC("NAM0"): { diff --git a/components/esm3/esmreader.hpp b/components/esm3/esmreader.hpp index e931cde42a..ffa7a94d7b 100644 --- a/components/esm3/esmreader.hpp +++ b/components/esm3/esmreader.hpp @@ -155,6 +155,13 @@ public: getHT(x); } + template + void skipHTSized() + { + static_assert(sizeof(T) == size); + skipHT(); + } + // Read a string by the given name if it is the next record. std::string getHNOString(NAME name);