Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Make hsStream typed readers and writers type-safe #871

Merged
merged 2 commits into from
Mar 14, 2021
Merged

Make hsStream typed readers and writers type-safe #871

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2749e1a
Fix several instances of using size_t and long in stream I/O.
zrax Mar 12, 2021
b1d38ba
Make all hsStream reads and writes size-explicit.
zrax Mar 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 6 additions & 8 deletions Sources/Plasma/Apps/plPythonPack/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ void WritePythonFile(const plFileName &fileName, const plFileName &path, hsStrea

ST::printf(out, "\n");

s->WriteLE32(size);
s->WriteLE32((int32_t)size);
s->Write(size, pycode);
delete[] pycode;
}
Expand Down Expand Up @@ -295,12 +295,10 @@ void PackDirectory(const plFileName& dir, const plFileName& rootPath, const plFi
if (!s.Open(pakName, "wb"))
return;

s.WriteLE32(fileNames.size());

int i;
for (i = 0; i < fileNames.size(); i++)
s.WriteLE32((uint32_t)fileNames.size());
for (const plFileName& fn : fileNames)
{
s.WriteSafeString(fileNames[i].AsString());
s.WriteSafeString(fn.AsString());
s.WriteLE32(0);
}

Expand All @@ -309,7 +307,7 @@ void PackDirectory(const plFileName& dir, const plFileName& rootPath, const plFi
std::vector<uint32_t> filePositions;
filePositions.resize(fileNames.size());

for (i = 0; i < fileNames.size(); i++)
for (size_t i = 0; i < fileNames.size(); i++)
{
// strip '.py' from the file name
plFileName properFileName = fileNames[i].StripFileExt();
Expand All @@ -320,7 +318,7 @@ void PackDirectory(const plFileName& dir, const plFileName& rootPath, const plFi
}

s.SetPosition(sizeof(uint32_t));
for (i = 0; i < fileNames.size(); i++)
for (size_t i = 0; i < fileNames.size(); i++)
{
s.WriteSafeString(fileNames[i].AsString());
s.WriteLE32(filePositions[i]);
Expand Down
2 changes: 1 addition & 1 deletion Sources/Plasma/CoreLib/hsBitVector.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ void hsBitVector::Read(hsStream* s)
{
Reset();

s->ReadLE(&fNumBitVectors);
s->ReadLE32(&fNumBitVectors);
if( fNumBitVectors )
{
delete [] fBitVectors;
Expand Down
16 changes: 8 additions & 8 deletions Sources/Plasma/CoreLib/hsStream.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ uint32_t hsStream::WriteSafeStringLong(const ST::string &string)
uint32_t i;
for (i = 0; i < len; i++)
{
WriteByte(~buffp[i]);
WriteByte(static_cast<uint8_t>(~buffp[i]));
}
return i;
}
Expand All @@ -137,9 +137,9 @@ uint32_t hsStream::WriteSafeWStringLong(const ST::string &string)
const char16_t *buffp = wbuff.data();
for (uint32_t i=0; i<len; i++)
{
WriteLE16(~buffp[i]);
WriteLE16(static_cast<uint16_t>(~buffp[i]));
}
WriteLE16(static_cast<char16_t>(0));
WriteLE16(static_cast<uint16_t>(0));
}
return 0;
}
Expand Down Expand Up @@ -173,7 +173,7 @@ ST::string hsStream::ReadSafeWStringLong()
retVal.allocate(numChars);
for (int i=0; i<numChars; i++)
retVal[i] = ReadLE16();
ReadLE16(); // we wrote the null out, read it back in
(void)ReadLE16(); // we wrote the null out, read it back in

if (retVal[0] & 0x80)
{
Expand All @@ -198,7 +198,7 @@ uint32_t hsStream::WriteSafeString(const ST::string &string)
const char *buffp = string.c_str();
for (i = 0; i < len; i++)
{
WriteByte(~buffp[i]);
WriteByte(static_cast<uint8_t>(~buffp[i]));
}
return i;
}
Expand All @@ -219,7 +219,7 @@ uint32_t hsStream::WriteSafeWString(const ST::string &string)
const char16_t *buffp = wbuff.data();
for (uint32_t i=0; i<len; i++)
{
WriteLE16(~buffp[i]);
WriteLE16(static_cast<uint16_t>(~buffp[i]));
}
WriteLE16(static_cast<uint16_t>(0));
}
Expand All @@ -235,7 +235,7 @@ ST::string hsStream::ReadSafeString()
// Backward compat hack - remove in a week or so (from 6/30/03)
bool oldFormat = !(numChars & 0xf000);
if (oldFormat)
ReadLE16();
(void)ReadLE16();
#endif

numChars &= ~0xf000;
Expand Down Expand Up @@ -269,7 +269,7 @@ ST::string hsStream::ReadSafeWString()
retVal.allocate(numChars);
for (int i=0; i<numChars; i++)
retVal[i] = ReadLE16();
ReadLE16(); // we wrote the null out, read it back in
(void)ReadLE16(); // we wrote the null out, read it back in

if (retVal[0] & 0x80)
{
Expand Down
37 changes: 24 additions & 13 deletions Sources/Plasma/CoreLib/hsStream.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,19 +126,30 @@ enum {
void WriteLEDouble(double value);
void WriteLEDouble(size_t count, const double values[]);

template <typename T>
inline void ReadLE(T* value)
{
Read(sizeof(T), value);
*value = hsToLE(*value);
}

template <typename T>
inline void WriteLE(T value)
{
value = hsToLE(value);
Write(sizeof(T), &value);
}
// Type-safe placement readers
template <typename T> inline void ReadByte(T*) = delete;
void ReadByte(uint8_t* v) { *v = ReadByte(); }
void ReadByte(int8_t* v) { *v = (int8_t)ReadByte(); }
template <typename T> inline void ReadLE16(T*) = delete;
void ReadLE16(uint16_t* v) { *v = ReadLE16(); }
void ReadLE16(int16_t* v) { *v = (int16_t)ReadLE16(); }
template <typename T> inline void ReadLE32(T*) = delete;
void ReadLE32(uint32_t* v) { *v = ReadLE32(); }
void ReadLE32(int32_t* v) { *v = (int32_t)ReadLE32(); }
template <typename T> inline void ReadLEFloat(T*) = delete;
void ReadLEFloat(float* v) { *v = ReadLEFloat(); }
template <typename T> inline void ReadLEDouble(T*) = delete;
void ReadLEDouble(double* v) { *v = ReadLEDouble(); }
Comment on lines +130 to +142
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if these should be marked [[nodiscard]]? I'm assuming the reason for the (void) when reading and throwing away with (void)s->ReadLE32(); is to silence warnings about return values?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I actually did have [[nodiscard]] at one point (hence the (void) casts), but I later changed my mind... I could add them back in if preferred.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://www.youtube.com/watch?v=CxK_nA2iVXw


// Type-safety checks for writers
template <typename T> void WriteByte(T) = delete;
void WriteByte(int8_t v) { WriteByte((uint8_t)v); }
template <typename T> void WriteLE16(T) = delete;
void WriteLE16(int16_t v) { WriteLE16((uint16_t)v); }
template <typename T> void WriteLE32(T) = delete;
void WriteLE32(int32_t v) { WriteLE32((uint32_t)v); }
template <typename T> void WriteLEFloat(T) = delete;
template <typename T> void WriteLEDouble(T) = delete;
};

class hsStreamable {
Expand Down
16 changes: 8 additions & 8 deletions Sources/Plasma/CoreLib/pcSmallRect.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,16 +50,16 @@ You can contact Cyan Worlds, Inc. by email legal@cyan.com

void pcSmallRect::Read( hsStream *s )
{
s->ReadLE( &fX );
s->ReadLE( &fY );
s->ReadLE( &fWidth );
s->ReadLE( &fHeight );
s->ReadLE16(&fX);
s->ReadLE16(&fY);
s->ReadLE16(&fWidth);
s->ReadLE16(&fHeight);
}

void pcSmallRect::Write( hsStream *s )
{
s->WriteLE( fX );
s->WriteLE( fY );
s->WriteLE( fWidth );
s->WriteLE( fHeight );
s->WriteLE16(fX);
s->WriteLE16(fY);
s->WriteLE16(fWidth);
s->WriteLE16(fHeight);
}
8 changes: 4 additions & 4 deletions Sources/Plasma/CoreLib/plGeneric.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,11 +110,11 @@ int plGeneric::Write(hsStream* stream)
break;

case kInt:
stream->WriteLE(fIntVal);
stream->WriteLE32(fIntVal);
break;

case kFloat:
stream->WriteLE(fFloatVal);
stream->WriteLEDouble(fFloatVal);
break;

case kString:
Expand All @@ -138,11 +138,11 @@ int plGeneric::Read(hsStream* stream)
break;

case kInt:
stream->ReadLE(&fIntVal);
stream->ReadLE32(&fIntVal);
break;

case kFloat:
stream->ReadLE(&fFloatVal);
stream->ReadLEDouble(&fFloatVal);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

twitch

break;

case kString:
Expand Down
2 changes: 1 addition & 1 deletion Sources/Plasma/CoreLib/plLoadMask.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ void plLoadMask::Write(hsStream* s) const
{
// write packed into 1 byte
uint8_t qc = (fQuality[0]<<4) | (fQuality[1] & 0xf);
s->WriteLE(qc);
s->WriteByte(qc);
}

uint32_t plLoadMask::ValidateReps(int num, const int quals[], const int caps[])
Expand Down
4 changes: 2 additions & 2 deletions Sources/Plasma/FeatureLib/pfConditional/plFacingConditionalObject.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,14 +68,14 @@ bool plFacingConditionalObject::MsgReceive(plMessage* msg)
void plFacingConditionalObject::Write(hsStream* stream, hsResMgr* mgr)
{
plConditionalObject::Write(stream, mgr);
stream->WriteLE(fTolerance);
stream->WriteLEFloat(fTolerance);
stream->WriteBool(fDirectional);
}

void plFacingConditionalObject::Read(hsStream* stream, hsResMgr* mgr)
{
plConditionalObject::Read(stream, mgr);
stream->ReadLE(&fTolerance);
stream->ReadLEFloat(&fTolerance);
fDirectional = stream->ReadBool();
}

Expand Down
5 changes: 3 additions & 2 deletions Sources/Plasma/FeatureLib/pfGameGUIMgr/pfGUIControlHandlers.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,9 @@ void pfGUIConsoleCmdProc::IWrite( hsStream *s )
{
if (fCommand != nullptr)
{
s->WriteLE32( strlen( fCommand ) );
s->Write( strlen( fCommand ), fCommand );
size_t cmdLen = strlen(fCommand);
s->WriteLE32((uint32_t)cmdLen);
s->Write(cmdLen, fCommand);
}
else
s->WriteLE32( 0 );
Expand Down
12 changes: 6 additions & 6 deletions Sources/Plasma/FeatureLib/pfGameGUIMgr/pfGUIControlMod.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,8 @@ void pfGUIColorScheme::Read( hsStream *s )
fTransparent = s->ReadBOOL();

fFontFace = s->ReadSafeString();
s->ReadLE( &fFontSize );
s->ReadLE( &fFontFlags );
s->ReadByte(&fFontSize);
s->ReadByte(&fFontFlags);
}

void pfGUIColorScheme::Write( hsStream *s )
Expand All @@ -132,8 +132,8 @@ void pfGUIColorScheme::Write( hsStream *s )
s->WriteBOOL( fTransparent );

s->WriteSafeString( fFontFace );
s->WriteLE( fFontSize );
s->WriteLE( fFontFlags );
s->WriteByte(fFontSize);
s->WriteByte(fFontFlags);
}

//// Destructor //////////////////////////////////////////////////
Expand Down Expand Up @@ -777,7 +777,7 @@ void pfGUIControlMod::Refresh()
void pfGUIControlMod::Read( hsStream *s, hsResMgr *mgr )
{
plSingleModifier::Read(s, mgr);
s->ReadLE( &fTagID );
s->ReadLE32(&fTagID);
fVisible = s->ReadBool();

// Read the handler in
Expand Down Expand Up @@ -825,7 +825,7 @@ void pfGUIControlMod::Write( hsStream *s, hsResMgr *mgr )
ClearFlag( kHasProxy );

plSingleModifier::Write( s, mgr );
s->WriteLE( fTagID );
s->WriteLE32(fTagID);
s->WriteBool( fVisible );

// Write the handler out (if it's not a writeable, damn you)
Expand Down
8 changes: 4 additions & 4 deletions Sources/Plasma/FeatureLib/pfGameGUIMgr/pfGUIDialogMod.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,13 +299,13 @@ void pfGUIDialogMod::Read( hsStream *s, hsResMgr *mgr )
hsgResMgr::ResMgr()->AddViaNotify( GetKey(), refMsg, plRefFlags::kPassiveRef );
}

s->ReadLE( &fTagID );
s->ReadLE32(&fTagID);

fProcReceiver = mgr->ReadKey( s );
if (fProcReceiver != nullptr)
SetHandler( new pfGUIDialogNotifyProc( fProcReceiver ) );

s->ReadLE( &fVersion );
s->ReadLE32(&fVersion);

fColorScheme->Read( s );

Expand All @@ -323,11 +323,11 @@ void pfGUIDialogMod::Write( hsStream *s, hsResMgr *mgr )
for (pfGUIControlMod* ctrl : fControls)
mgr->WriteKey(s, ctrl->GetKey());

s->WriteLE( fTagID );
s->WriteLE32(fTagID);

mgr->WriteKey( s, fProcReceiver );

s->WriteLE( fVersion );
s->WriteLE32(fVersion);

fColorScheme->Write( s );

Expand Down
36 changes: 17 additions & 19 deletions Sources/Plasma/FeatureLib/pfGameGUIMgr/pfGUIPopUpMenu.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -877,16 +877,15 @@ void pfGUISkin::Read( hsStream *s, hsResMgr *mgr )
{
hsKeyedObject::Read( s, mgr );

s->ReadLE( &fItemMargin );
s->ReadLE( &fBorderMargin );
s->ReadLE16(&fItemMargin);
s->ReadLE16(&fBorderMargin);

uint32_t i, count;
s->ReadLE( &count );
uint32_t count = s->ReadLE32();

for( i = 0; i < count; i++ )
for (uint32_t i = 0; i < count; i++)
fElements[ i ].Read( s );

for( ; i < kNumElements; i++ )
for (uint32_t i = count; i < kNumElements; i++)
fElements[ i ].Empty();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At some point, this needs to be cleaned up. Not in this PR, though.


mgr->ReadKeyNotifyMe( s, new plGenRefMsg( GetKey(), plRefMsg::kOnCreate, -1, kRefMipmap ), plRefFlags::kActiveRef );
Expand All @@ -896,13 +895,12 @@ void pfGUISkin::Write( hsStream *s, hsResMgr *mgr )
{
hsKeyedObject::Write( s, mgr );

s->WriteLE( fItemMargin );
s->WriteLE( fBorderMargin );
s->WriteLE16(fItemMargin);
s->WriteLE16(fBorderMargin);

uint32_t i = kNumElements;
s->WriteLE( i );
s->WriteLE32((uint32_t)kNumElements);

for( i = 0; i < kNumElements; i++ )
for (uint32_t i = 0; i < kNumElements; i++)
fElements[ i ].Write( s );

mgr->WriteKey( s, fTexture );
Expand All @@ -926,16 +924,16 @@ bool pfGUISkin::MsgReceive( plMessage *msg )

void pfGUISkin::pfSRect::Read( hsStream *s )
{
s->ReadLE( &fX );
s->ReadLE( &fY );
s->ReadLE( &fWidth );
s->ReadLE( &fHeight );
s->ReadLE16(&fX);
s->ReadLE16(&fY);
s->ReadLE16(&fWidth);
s->ReadLE16(&fHeight);
}

void pfGUISkin::pfSRect::Write( hsStream *s )
{
s->WriteLE( fX );
s->WriteLE( fY );
s->WriteLE( fWidth );
s->WriteLE( fHeight );
s->WriteLE16(fX);
s->WriteLE16(fY);
s->WriteLE16(fWidth);
s->WriteLE16(fHeight);
}
Loading