From 218e6f7985bd1dc08ee901a45ec738a914af379e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=BCller?= Date: Sat, 4 Feb 2023 01:02:11 +0100 Subject: [PATCH 1/4] Remove `bytes_be_to_int` and `int_to_bytes_be` Use `bytes_be_to_uint` and `uint_to_bytes_be` instead. As casting between `int` and `unsigned` preserves the bit representation of the value, it's not necessary to apply additional tricks to convert between `char` arrays and `int`. --- src/base/system.cpp | 28 ---------------------------- src/base/system.h | 23 ----------------------- src/engine/client/client.cpp | 4 ++-- src/engine/client/ghost.cpp | 4 ++-- src/engine/client/ghost.h | 4 ++-- src/engine/shared/datafile.cpp | 4 ++-- src/engine/shared/demo.cpp | 16 ++++++++-------- src/engine/shared/snapshot.cpp | 6 +++--- src/game/client/components/menus.h | 4 ++-- src/test/bytes_be.cpp | 4 ++-- 10 files changed, 23 insertions(+), 74 deletions(-) diff --git a/src/base/system.cpp b/src/base/system.cpp index 9ffee9572..a98cad63b 100644 --- a/src/base/system.cpp +++ b/src/base/system.cpp @@ -3838,34 +3838,6 @@ const char *str_next_token(const char *str, const char *delim, char *buffer, int return tok + len; } -int bytes_be_to_int(const unsigned char *bytes) -{ - int Result; - unsigned char *pResult = (unsigned char *)&Result; - for(unsigned i = 0; i < sizeof(int); i++) - { -#if defined(CONF_ARCH_ENDIAN_BIG) - pResult[i] = bytes[i]; -#else - pResult[i] = bytes[sizeof(int) - i - 1]; -#endif - } - return Result; -} - -void int_to_bytes_be(unsigned char *bytes, int value) -{ - const unsigned char *pValue = (const unsigned char *)&value; - for(unsigned i = 0; i < sizeof(int); i++) - { -#if defined(CONF_ARCH_ENDIAN_BIG) - bytes[i] = pValue[i]; -#else - bytes[sizeof(int) - i - 1] = pValue[i]; -#endif - } -} - unsigned bytes_be_to_uint(const unsigned char *bytes) { return ((bytes[0] & 0xffu) << 24u) | ((bytes[1] & 0xffu) << 16u) | ((bytes[2] & 0xffu) << 8u) | (bytes[3] & 0xffu); diff --git a/src/base/system.h b/src/base/system.h index 538ed1a19..a5efde215 100644 --- a/src/base/system.h +++ b/src/base/system.h @@ -2386,29 +2386,6 @@ const char *str_next_token(const char *str, const char *delim, char *buffer, int */ int str_in_list(const char *list, const char *delim, const char *needle); -/* - Function: bytes_be_to_int - Packs 4 big endian bytes into an int - - Returns: - The packed int - - Remarks: - - Assumes the passed array is 4 bytes - - Assumes int is 4 bytes -*/ -int bytes_be_to_int(const unsigned char *bytes); - -/* - Function: int_to_bytes_be - Packs an int into 4 big endian bytes - - Remarks: - - Assumes the passed array is 4 bytes - - Assumes int is 4 bytes -*/ -void int_to_bytes_be(unsigned char *bytes, int value); - /* Function: bytes_be_to_uint Packs 4 big endian bytes into an unsigned diff --git a/src/engine/client/client.cpp b/src/engine/client/client.cpp index 22564b489..ec3eff803 100644 --- a/src/engine/client/client.cpp +++ b/src/engine/client/client.cpp @@ -4088,8 +4088,8 @@ int CClient::HandleChecksum(int Conn, CUuid Uuid, CUnpacker *pUnpacker) int FileStart = maximum(Start, (int)sizeof(m_Checksum.m_aBytes)); unsigned char aStartBytes[4]; unsigned char aEndBytes[4]; - int_to_bytes_be(aStartBytes, Start); - int_to_bytes_be(aEndBytes, End); + uint_to_bytes_be(aStartBytes, Start); + uint_to_bytes_be(aEndBytes, End); if(Start <= (int)sizeof(m_Checksum.m_aBytes)) { diff --git a/src/engine/client/ghost.cpp b/src/engine/client/ghost.cpp index b9b87ff77..6d989231f 100644 --- a/src/engine/client/ghost.cpp +++ b/src/engine/client/ghost.cpp @@ -145,11 +145,11 @@ int CGhostRecorder::Stop(int Ticks, int Time) io_seek(m_File, gs_NumTicksOffset, IOSEEK_START); unsigned char aNumTicks[4]; - int_to_bytes_be(aNumTicks, Ticks); + uint_to_bytes_be(aNumTicks, Ticks); io_write(m_File, aNumTicks, sizeof(aNumTicks)); unsigned char aTime[4]; - int_to_bytes_be(aTime, Time); + uint_to_bytes_be(aTime, Time); io_write(m_File, aTime, sizeof(aTime)); io_close(m_File); diff --git a/src/engine/client/ghost.h b/src/engine/client/ghost.h index 0de59870b..f3acb5472 100644 --- a/src/engine/client/ghost.h +++ b/src/engine/client/ghost.h @@ -23,12 +23,12 @@ struct CGhostHeader int GetTicks() const { - return bytes_be_to_int(m_aNumTicks); + return bytes_be_to_uint(m_aNumTicks); } int GetTime() const { - return bytes_be_to_int(m_aTime); + return bytes_be_to_uint(m_aTime); } CGhostInfo ToGhostInfo() const diff --git a/src/engine/shared/datafile.cpp b/src/engine/shared/datafile.cpp index 395c62c25..498c038e7 100644 --- a/src/engine/shared/datafile.cpp +++ b/src/engine/shared/datafile.cpp @@ -28,7 +28,7 @@ struct CItemEx { CItemEx Result; for(int i = 0; i < (int)sizeof(CUuid) / 4; i++) - Result.m_aUuid[i] = bytes_be_to_int(&Uuid.m_aData[i * 4]); + Result.m_aUuid[i] = bytes_be_to_uint(&Uuid.m_aData[i * 4]); return Result; } @@ -36,7 +36,7 @@ struct CItemEx { CUuid Result; for(int i = 0; i < (int)sizeof(CUuid) / 4; i++) - int_to_bytes_be(&Result.m_aData[i * 4], m_aUuid[i]); + uint_to_bytes_be(&Result.m_aData[i * 4], m_aUuid[i]); return Result; } }; diff --git a/src/engine/shared/demo.cpp b/src/engine/shared/demo.cpp index 0d7c43422..2ffb6935f 100644 --- a/src/engine/shared/demo.cpp +++ b/src/engine/shared/demo.cpp @@ -348,18 +348,18 @@ int CDemoRecorder::Stop() // add the demo length to the header io_seek(m_File, gs_LengthOffset, IOSEEK_START); unsigned char aLength[4]; - int_to_bytes_be(aLength, Length()); + uint_to_bytes_be(aLength, Length()); io_write(m_File, aLength, sizeof(aLength)); // add the timeline markers to the header io_seek(m_File, gs_NumMarkersOffset, IOSEEK_START); unsigned char aNumMarkers[4]; - int_to_bytes_be(aNumMarkers, m_NumTimelineMarkers); + uint_to_bytes_be(aNumMarkers, m_NumTimelineMarkers); io_write(m_File, aNumMarkers, sizeof(aNumMarkers)); for(int i = 0; i < m_NumTimelineMarkers; i++) { unsigned char aMarker[4]; - int_to_bytes_be(aMarker, m_aTimelineMarkers[i]); + uint_to_bytes_be(aMarker, m_aTimelineMarkers[i]); io_write(m_File, aMarker, sizeof(aMarker)); } @@ -457,7 +457,7 @@ int CDemoPlayer::ReadChunkHeader(int *pType, int *pSize, int *pTick) unsigned char aTickdata[4]; if(io_read(m_File, aTickdata, sizeof(aTickdata)) != sizeof(aTickdata)) return -1; - *pTick = bytes_be_to_int(aTickdata); + *pTick = bytes_be_to_uint(aTickdata); } } else @@ -824,11 +824,11 @@ int CDemoPlayer::Load(class IStorage *pStorage, class IConsole *pConsole, const if(m_Info.m_Header.m_Version > gs_OldVersion) { // get timeline markers - int Num = bytes_be_to_int(m_Info.m_TimelineMarkers.m_aNumTimelineMarkers); + int Num = bytes_be_to_uint(m_Info.m_TimelineMarkers.m_aNumTimelineMarkers); m_Info.m_Info.m_NumTimelineMarkers = clamp(Num, 0, MAX_TIMELINE_MARKERS); for(int i = 0; i < m_Info.m_Info.m_NumTimelineMarkers; i++) { - m_Info.m_Info.m_aTimelineMarkers[i] = bytes_be_to_int(m_Info.m_TimelineMarkers.m_aTimelineMarkers[i]); + m_Info.m_Info.m_aTimelineMarkers[i] = bytes_be_to_uint(m_Info.m_TimelineMarkers.m_aTimelineMarkers[i]); } } @@ -1117,7 +1117,7 @@ bool CDemoPlayer::GetDemoInfo(class IStorage *pStorage, const char *pFilename, i io_read(File, pTimelineMarkers, sizeof(CTimelineMarkers)); str_copy(pMapInfo->m_aName, pDemoHeader->m_aMapName); - pMapInfo->m_Crc = bytes_be_to_int(pDemoHeader->m_aMapCrc); + pMapInfo->m_Crc = bytes_be_to_uint(pDemoHeader->m_aMapCrc); SHA256_DIGEST Sha256 = SHA256_ZEROED; if(pDemoHeader->m_Version >= gs_Sha256Version) @@ -1138,7 +1138,7 @@ bool CDemoPlayer::GetDemoInfo(class IStorage *pStorage, const char *pFilename, i } pMapInfo->m_Sha256 = Sha256; - pMapInfo->m_Size = bytes_be_to_int(pDemoHeader->m_aMapSize); + pMapInfo->m_Size = bytes_be_to_uint(pDemoHeader->m_aMapSize); io_close(File); return !(mem_comp(pDemoHeader->m_aMarker, gs_aHeaderMarker, sizeof(gs_aHeaderMarker)) || pDemoHeader->m_Version < gs_OldVersion); diff --git a/src/engine/shared/snapshot.cpp b/src/engine/shared/snapshot.cpp index 8ad03f8f8..a088a1cfc 100644 --- a/src/engine/shared/snapshot.cpp +++ b/src/engine/shared/snapshot.cpp @@ -47,7 +47,7 @@ int CSnapshot::GetExternalItemType(int InternalType) const const CSnapshotItem *pTypeItem = GetItem(TypeItemIndex); CUuid Uuid; for(int i = 0; i < (int)sizeof(CUuid) / 4; i++) - int_to_bytes_be(&Uuid.m_aData[i * 4], pTypeItem->Data()[i]); + uint_to_bytes_be(&Uuid.m_aData[i * 4], pTypeItem->Data()[i]); return g_UuidManager.LookupUuid(Uuid); } @@ -71,7 +71,7 @@ const void *CSnapshot::FindItem(int Type, int ID) const CUuid TypeUuid = g_UuidManager.GetUuid(Type); int aTypeUuidItem[sizeof(CUuid) / 4]; for(int i = 0; i < (int)sizeof(CUuid) / 4; i++) - aTypeUuidItem[i] = bytes_be_to_int(&TypeUuid.m_aData[i * 4]); + aTypeUuidItem[i] = bytes_be_to_uint(&TypeUuid.m_aData[i * 4]); bool Found = false; for(int i = 0; i < m_NumItems; i++) @@ -630,7 +630,7 @@ void CSnapshotBuilder::AddExtendedItemType(int Index) if(pUuidItem) { for(int i = 0; i < (int)sizeof(CUuid) / 4; i++) - pUuidItem[i] = bytes_be_to_int(&Uuid.m_aData[i * 4]); + pUuidItem[i] = bytes_be_to_uint(&Uuid.m_aData[i * 4]); } } diff --git a/src/game/client/components/menus.h b/src/game/client/components/menus.h index 1e514f81f..3d2cbbadf 100644 --- a/src/game/client/components/menus.h +++ b/src/game/client/components/menus.h @@ -397,12 +397,12 @@ protected: int NumMarkers() const { - return clamp(bytes_be_to_int(m_TimelineMarkers.m_aNumTimelineMarkers), 0, MAX_TIMELINE_MARKERS); + return clamp(bytes_be_to_uint(m_TimelineMarkers.m_aNumTimelineMarkers), 0, MAX_TIMELINE_MARKERS); } int Length() const { - return bytes_be_to_int(m_Info.m_aLength); + return bytes_be_to_uint(m_Info.m_aLength); } unsigned Size() const diff --git a/src/test/bytes_be.cpp b/src/test/bytes_be.cpp index 0f9493971..8d0282cf8 100644 --- a/src/test/bytes_be.cpp +++ b/src/test/bytes_be.cpp @@ -12,8 +12,8 @@ TEST(BytePacking, RoundtripInt) for(auto i : INT_DATA) { unsigned char aPacked[4]; - int_to_bytes_be(aPacked, i); - EXPECT_EQ(bytes_be_to_int(aPacked), i); + uint_to_bytes_be(aPacked, i); + EXPECT_EQ(bytes_be_to_uint(aPacked), i); } } From 64081ad3e4c3cd8d10e98fc61bf01786d6e1daa1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=BCller?= Date: Sat, 4 Feb 2023 01:09:17 +0100 Subject: [PATCH 2/4] Add static assert to ensure correct size of `int` and `unsigned` We assume in various places, especially when using `bytes_be_to_uint` and `uint_to_bytes_be`, that `int` and `unsigned` are 4 bytes in size. --- src/base/system.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/base/system.cpp b/src/base/system.cpp index a98cad63b..3c10a37e8 100644 --- a/src/base/system.cpp +++ b/src/base/system.cpp @@ -3838,6 +3838,9 @@ const char *str_next_token(const char *str, const char *delim, char *buffer, int return tok + len; } +static_assert(sizeof(unsigned) == 4, "unsigned must be 4 bytes in size"); +static_assert(sizeof(unsigned) == sizeof(int), "unsigned and int must have the same size"); + unsigned bytes_be_to_uint(const unsigned char *bytes) { return ((bytes[0] & 0xffu) << 24u) | ((bytes[1] & 0xffu) << 16u) | ((bytes[2] & 0xffu) << 8u) | (bytes[3] & 0xffu); From be6862e997de29c677b3266161e7fdc3fe1f132e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=BCller?= Date: Sat, 4 Feb 2023 01:22:49 +0100 Subject: [PATCH 3/4] Use `sizeof(int32_t)` instead of `4` --- src/engine/client/client.cpp | 4 +-- src/engine/client/ghost.cpp | 8 +++--- src/engine/client/ghost.h | 6 ++--- src/engine/demo.h | 10 ++++---- src/engine/server/register.cpp | 2 +- src/engine/shared/datafile.cpp | 10 ++++---- src/engine/shared/demo.cpp | 10 ++++---- src/engine/shared/network_server.cpp | 4 +-- src/engine/shared/snapshot.cpp | 38 ++++++++++++++-------------- src/game/server/teehistorian.cpp | 4 +-- src/test/bytes_be.cpp | 8 +++--- 11 files changed, 52 insertions(+), 52 deletions(-) diff --git a/src/engine/client/client.cpp b/src/engine/client/client.cpp index ec3eff803..16bcafae5 100644 --- a/src/engine/client/client.cpp +++ b/src/engine/client/client.cpp @@ -4086,8 +4086,8 @@ int CClient::HandleChecksum(int Conn, CUuid Uuid, CUnpacker *pUnpacker) int End = Start + Length; int ChecksumBytesEnd = minimum(End, (int)sizeof(m_Checksum.m_aBytes)); int FileStart = maximum(Start, (int)sizeof(m_Checksum.m_aBytes)); - unsigned char aStartBytes[4]; - unsigned char aEndBytes[4]; + unsigned char aStartBytes[sizeof(int32_t)]; + unsigned char aEndBytes[sizeof(int32_t)]; uint_to_bytes_be(aStartBytes, Start); uint_to_bytes_be(aEndBytes, End); diff --git a/src/engine/client/ghost.cpp b/src/engine/client/ghost.cpp index 6d989231f..c8b837c25 100644 --- a/src/engine/client/ghost.cpp +++ b/src/engine/client/ghost.cpp @@ -83,7 +83,7 @@ void CGhostRecorder::WriteData(int Type, const void *pData, int Size) mem_copy(Data.m_aData, pData, Size); if(m_LastItem.m_Type == Data.m_Type) - DiffItem((int *)m_LastItem.m_aData, (int *)Data.m_aData, (int *)m_pBufferPos, Size / 4); + DiffItem((int *)m_LastItem.m_aData, (int *)Data.m_aData, (int *)m_pBufferPos, Size / sizeof(int32_t)); else { FlushChunk(); @@ -144,11 +144,11 @@ int CGhostRecorder::Stop(int Ticks, int Time) // write down num shots and time io_seek(m_File, gs_NumTicksOffset, IOSEEK_START); - unsigned char aNumTicks[4]; + unsigned char aNumTicks[sizeof(int32_t)]; uint_to_bytes_be(aNumTicks, Ticks); io_write(m_File, aNumTicks, sizeof(aNumTicks)); - unsigned char aTime[4]; + unsigned char aTime[sizeof(int32_t)]; uint_to_bytes_be(aTime, Time); io_write(m_File, aTime, sizeof(aTime)); @@ -341,7 +341,7 @@ bool CGhostLoader::ReadData(int Type, void *pData, int Size) CGhostItem Data(Type); if(m_LastItem.m_Type == Data.m_Type) - UndiffItem((int *)m_LastItem.m_aData, (int *)m_pBufferPos, (int *)Data.m_aData, Size / 4); + UndiffItem((int *)m_LastItem.m_aData, (int *)m_pBufferPos, (int *)Data.m_aData, Size / sizeof(int32_t)); else mem_copy(Data.m_aData, m_pBufferPos, Size); diff --git a/src/engine/client/ghost.h b/src/engine/client/ghost.h index f3acb5472..f555e1f2d 100644 --- a/src/engine/client/ghost.h +++ b/src/engine/client/ghost.h @@ -16,9 +16,9 @@ struct CGhostHeader unsigned char m_Version; char m_aOwner[MAX_NAME_LENGTH]; char m_aMap[64]; - unsigned char m_aZeroes[4]; // Crc before version 6 - unsigned char m_aNumTicks[4]; - unsigned char m_aTime[4]; + unsigned char m_aZeroes[sizeof(int32_t)]; // Crc before version 6 + unsigned char m_aNumTicks[sizeof(int32_t)]; + unsigned char m_aTime[sizeof(int32_t)]; SHA256_DIGEST m_MapSha256; int GetTicks() const diff --git a/src/engine/demo.h b/src/engine/demo.h index f37153272..f39d733b7 100644 --- a/src/engine/demo.h +++ b/src/engine/demo.h @@ -29,17 +29,17 @@ struct CDemoHeader unsigned char m_Version; char m_aNetversion[64]; char m_aMapName[64]; - unsigned char m_aMapSize[4]; - unsigned char m_aMapCrc[4]; + unsigned char m_aMapSize[sizeof(int32_t)]; + unsigned char m_aMapCrc[sizeof(int32_t)]; char m_aType[8]; - unsigned char m_aLength[4]; + unsigned char m_aLength[sizeof(int32_t)]; char m_aTimestamp[20]; }; struct CTimelineMarkers { - unsigned char m_aNumTimelineMarkers[4]; - unsigned char m_aTimelineMarkers[MAX_TIMELINE_MARKERS][4]; + unsigned char m_aNumTimelineMarkers[sizeof(int32_t)]; + unsigned char m_aTimelineMarkers[MAX_TIMELINE_MARKERS][sizeof(int32_t)]; }; struct CMapInfo diff --git a/src/engine/server/register.cpp b/src/engine/server/register.cpp index 7953320c8..b5f8f9362 100644 --- a/src/engine/server/register.cpp +++ b/src/engine/server/register.cpp @@ -498,7 +498,7 @@ CRegister::CRegister(CConfig *pConfig, IConsole *pConsole, IEngine *pEngine, int m_aVerifyPacketPrefix[HEADER_LEN + UUID_MAXSTRSIZE - 1] = ':'; // The DDNet code uses the `unsigned` security token in memory byte order. - unsigned char aTokenBytes[4]; + unsigned char aTokenBytes[sizeof(int32_t)]; mem_copy(aTokenBytes, &SixupSecurityToken, sizeof(aTokenBytes)); str_format(m_aConnlessTokenHex, sizeof(m_aConnlessTokenHex), "%08x", bytes_be_to_uint(aTokenBytes)); diff --git a/src/engine/shared/datafile.cpp b/src/engine/shared/datafile.cpp index 498c038e7..38869b7c7 100644 --- a/src/engine/shared/datafile.cpp +++ b/src/engine/shared/datafile.cpp @@ -22,21 +22,21 @@ enum struct CItemEx { - int m_aUuid[sizeof(CUuid) / 4]; + int m_aUuid[sizeof(CUuid) / sizeof(int32_t)]; static CItemEx FromUuid(CUuid Uuid) { CItemEx Result; - for(int i = 0; i < (int)sizeof(CUuid) / 4; i++) - Result.m_aUuid[i] = bytes_be_to_uint(&Uuid.m_aData[i * 4]); + for(size_t i = 0; i < sizeof(CUuid) / sizeof(int32_t); i++) + Result.m_aUuid[i] = bytes_be_to_uint(&Uuid.m_aData[i * sizeof(int32_t)]); return Result; } CUuid ToUuid() const { CUuid Result; - for(int i = 0; i < (int)sizeof(CUuid) / 4; i++) - uint_to_bytes_be(&Result.m_aData[i * 4], m_aUuid[i]); + for(size_t i = 0; i < sizeof(CUuid) / sizeof(int32_t); i++) + uint_to_bytes_be(&Result.m_aData[i * sizeof(int32_t)], m_aUuid[i]); return Result; } }; diff --git a/src/engine/shared/demo.cpp b/src/engine/shared/demo.cpp index 2ffb6935f..d06b56ef6 100644 --- a/src/engine/shared/demo.cpp +++ b/src/engine/shared/demo.cpp @@ -225,7 +225,7 @@ void CDemoRecorder::WriteTickMarker(int Tick, int Keyframe) { if(m_LastTickMarker == -1 || Tick - m_LastTickMarker > CHUNKMASK_TICK || Keyframe) { - unsigned char aChunk[5]; + unsigned char aChunk[sizeof(int32_t) + 1]; aChunk[0] = CHUNKTYPEFLAG_TICKMARKER; uint_to_bytes_be(aChunk + 1, Tick); @@ -347,18 +347,18 @@ int CDemoRecorder::Stop() // add the demo length to the header io_seek(m_File, gs_LengthOffset, IOSEEK_START); - unsigned char aLength[4]; + unsigned char aLength[sizeof(int32_t)]; uint_to_bytes_be(aLength, Length()); io_write(m_File, aLength, sizeof(aLength)); // add the timeline markers to the header io_seek(m_File, gs_NumMarkersOffset, IOSEEK_START); - unsigned char aNumMarkers[4]; + unsigned char aNumMarkers[sizeof(int32_t)]; uint_to_bytes_be(aNumMarkers, m_NumTimelineMarkers); io_write(m_File, aNumMarkers, sizeof(aNumMarkers)); for(int i = 0; i < m_NumTimelineMarkers; i++) { - unsigned char aMarker[4]; + unsigned char aMarker[sizeof(int32_t)]; uint_to_bytes_be(aMarker, m_aTimelineMarkers[i]); io_write(m_File, aMarker, sizeof(aMarker)); } @@ -454,7 +454,7 @@ int CDemoPlayer::ReadChunkHeader(int *pType, int *pSize, int *pTick) } else { - unsigned char aTickdata[4]; + unsigned char aTickdata[sizeof(int32_t)]; if(io_read(m_File, aTickdata, sizeof(aTickdata)) != sizeof(aTickdata)) return -1; *pTick = bytes_be_to_uint(aTickdata); diff --git a/src/engine/shared/network_server.cpp b/src/engine/shared/network_server.cpp index d6cbf2798..14c402984 100644 --- a/src/engine/shared/network_server.cpp +++ b/src/engine/shared/network_server.cpp @@ -564,8 +564,8 @@ int CNetServer::OnSixupCtrlMsg(NETADDR &Addr, CNetChunk *pChunk, int ControlMsg, else if(ControlMsg == NET_CTRLMSG_CONNECT) { SECURITY_TOKEN MyToken = GetToken(Addr); - unsigned char aToken[4]; - mem_copy(aToken, &MyToken, 4); + unsigned char aToken[sizeof(SECURITY_TOKEN)]; + mem_copy(aToken, &MyToken, sizeof(aToken)); CNetBase::SendControlMsg(m_Socket, &Addr, 0, NET_CTRLMSG_CONNECTACCEPT, aToken, sizeof(aToken), ResponseToken, true); if(Token == MyToken) diff --git a/src/engine/shared/snapshot.cpp b/src/engine/shared/snapshot.cpp index a088a1cfc..d9acce0f3 100644 --- a/src/engine/shared/snapshot.cpp +++ b/src/engine/shared/snapshot.cpp @@ -46,8 +46,8 @@ int CSnapshot::GetExternalItemType(int InternalType) const } const CSnapshotItem *pTypeItem = GetItem(TypeItemIndex); CUuid Uuid; - for(int i = 0; i < (int)sizeof(CUuid) / 4; i++) - uint_to_bytes_be(&Uuid.m_aData[i * 4], pTypeItem->Data()[i]); + for(size_t i = 0; i < sizeof(CUuid) / sizeof(int32_t); i++) + uint_to_bytes_be(&Uuid.m_aData[i * sizeof(int32_t)], pTypeItem->Data()[i]); return g_UuidManager.LookupUuid(Uuid); } @@ -69,9 +69,9 @@ const void *CSnapshot::FindItem(int Type, int ID) const if(Type >= OFFSET_UUID) { CUuid TypeUuid = g_UuidManager.GetUuid(Type); - int aTypeUuidItem[sizeof(CUuid) / 4]; - for(int i = 0; i < (int)sizeof(CUuid) / 4; i++) - aTypeUuidItem[i] = bytes_be_to_uint(&TypeUuid.m_aData[i * 4]); + int aTypeUuidItem[sizeof(CUuid) / sizeof(int32_t)]; + for(size_t i = 0; i < sizeof(CUuid) / sizeof(int32_t); i++) + aTypeUuidItem[i] = bytes_be_to_uint(&TypeUuid.m_aData[i * sizeof(int32_t)]); bool Found = false; for(int i = 0; i < m_NumItems; i++) @@ -105,7 +105,7 @@ unsigned CSnapshot::Crc() const CSnapshotItem *pItem = GetItem(i); int Size = GetItemSize(i); - for(int b = 0; b < Size / 4; b++) + for(size_t b = 0; b < Size / sizeof(int32_t); b++) Crc += pItem->Data()[b]; } return Crc; @@ -119,8 +119,8 @@ void CSnapshot::DebugDump() const CSnapshotItem *pItem = GetItem(i); int Size = GetItemSize(i); dbg_msg("snapshot", "\ttype=%d id=%d", pItem->Type(), pItem->ID()); - for(int b = 0; b < Size / 4; b++) - dbg_msg("snapshot", "\t\t%3d %12d\t%08x", b, pItem->Data()[b], pItem->Data()[b]); + for(size_t b = 0; b < Size / sizeof(int32_t); b++) + dbg_msg("snapshot", "\t\t%3d %12d\t%08x", (int)b, pItem->Data()[b], pItem->Data()[b]); } } @@ -320,13 +320,13 @@ int CSnapshotDelta::CreateDelta(CSnapshot *pFrom, CSnapshot *pTo, void *pDstData if(!IncludeSize) pItemDataDst = pData + 2; - if(DiffItem(pPastItem->Data(), pCurItem->Data(), pItemDataDst, ItemSize / 4)) + if(DiffItem(pPastItem->Data(), pCurItem->Data(), pItemDataDst, ItemSize / sizeof(int32_t))) { *pData++ = pCurItem->Type(); *pData++ = pCurItem->ID(); if(IncludeSize) - *pData++ = ItemSize / 4; - pData += ItemSize / 4; + *pData++ = ItemSize / sizeof(int32_t); + pData += ItemSize / sizeof(int32_t); pDelta->m_NumUpdateItems++; } } @@ -335,10 +335,10 @@ int CSnapshotDelta::CreateDelta(CSnapshot *pFrom, CSnapshot *pTo, void *pDstData *pData++ = pCurItem->Type(); *pData++ = pCurItem->ID(); if(IncludeSize) - *pData++ = ItemSize / 4; + *pData++ = ItemSize / sizeof(int32_t); mem_copy(pData, pCurItem->Data(), ItemSize); - pData += ItemSize / 4; + pData += ItemSize / sizeof(int32_t); pDelta->m_NumUpdateItems++; } } @@ -420,9 +420,9 @@ int CSnapshotDelta::UnpackDelta(CSnapshot *pFrom, CSnapshot *pTo, const void *pS { if(pData + 1 > pEnd) return -103; - if(*pData < 0 || *pData > INT_MAX / 4) + if(*pData < 0 || (size_t)*pData > INT_MAX / sizeof(int32_t)) return -204; - ItemSize = (*pData++) * 4; + ItemSize = (*pData++) * sizeof(int32_t); } if(ItemSize < 0 || RangeCheck(pEnd, pData, ItemSize)) @@ -442,7 +442,7 @@ int CSnapshotDelta::UnpackDelta(CSnapshot *pFrom, CSnapshot *pTo, const void *pS if(FromIndex != -1) { // we got an update so we need to apply the diff - UndiffItem(pFrom->GetItem(FromIndex)->Data(), pData, pNewData, ItemSize / 4, &m_aSnapshotDataRate[Type]); + UndiffItem(pFrom->GetItem(FromIndex)->Data(), pData, pNewData, ItemSize / sizeof(int32_t), &m_aSnapshotDataRate[Type]); } else // no previous, just copy the pData { @@ -451,7 +451,7 @@ int CSnapshotDelta::UnpackDelta(CSnapshot *pFrom, CSnapshot *pTo, const void *pS } m_aSnapshotDataUpdates[Type]++; - pData += ItemSize / 4; + pData += ItemSize / sizeof(int32_t); } // finish up @@ -629,8 +629,8 @@ void CSnapshotBuilder::AddExtendedItemType(int Index) int *pUuidItem = (int *)NewItem(0, GetTypeFromIndex(Index), sizeof(Uuid)); // NETOBJTYPE_EX if(pUuidItem) { - for(int i = 0; i < (int)sizeof(CUuid) / 4; i++) - pUuidItem[i] = bytes_be_to_uint(&Uuid.m_aData[i * 4]); + for(size_t i = 0; i < sizeof(CUuid) / sizeof(int32_t); i++) + pUuidItem[i] = bytes_be_to_uint(&Uuid.m_aData[i * sizeof(int32_t)]); } } diff --git a/src/game/server/teehistorian.cpp b/src/game/server/teehistorian.cpp index a0b4406d1..2b711466e 100644 --- a/src/game/server/teehistorian.cpp +++ b/src/game/server/teehistorian.cpp @@ -423,7 +423,7 @@ void CTeeHistorian::RecordPlayerInput(int ClientID, uint32_t UniqueClientID, con Buffer.Reset(); Buffer.AddInt(-TEEHISTORIAN_INPUT_DIFF); - CSnapshotDelta::DiffItem((int *)&pPrev->m_Input, (int *)pInput, (int *)&DiffInput, sizeof(DiffInput) / sizeof(int)); + CSnapshotDelta::DiffItem((int *)&pPrev->m_Input, (int *)pInput, (int *)&DiffInput, sizeof(DiffInput) / sizeof(int32_t)); if(m_Debug) { const int *pData = (const int *)&DiffInput; @@ -444,7 +444,7 @@ void CTeeHistorian::RecordPlayerInput(int ClientID, uint32_t UniqueClientID, con } } Buffer.AddInt(ClientID); - for(int i = 0; i < (int)(sizeof(DiffInput) / sizeof(int)); i++) + for(size_t i = 0; i < sizeof(DiffInput) / sizeof(int32_t); i++) { Buffer.AddInt(((int *)&DiffInput)[i]); } diff --git a/src/test/bytes_be.cpp b/src/test/bytes_be.cpp index 8d0282cf8..5922b6bee 100644 --- a/src/test/bytes_be.cpp +++ b/src/test/bytes_be.cpp @@ -4,14 +4,14 @@ #include -static const int INT_DATA[] = {0, 1, -1, 32, 64, 256, -512, 12345, -123456, 1234567, 12345678, 123456789, 2147483647, (-2147483647 - 1)}; -static const unsigned UINT_DATA[] = {0u, 1u, 2u, 32u, 64u, 256u, 512u, 12345u, 123456u, 1234567u, 12345678u, 123456789u, 2147483647u, 2147483648u, 4294967295u}; +static const int32_t INT_DATA[] = {0, 1, -1, 32, 64, 256, -512, 12345, -123456, 1234567, 12345678, 123456789, 2147483647, (-2147483647 - 1)}; +static const uint32_t UINT_DATA[] = {0u, 1u, 2u, 32u, 64u, 256u, 512u, 12345u, 123456u, 1234567u, 12345678u, 123456789u, 2147483647u, 2147483648u, 4294967295u}; TEST(BytePacking, RoundtripInt) { for(auto i : INT_DATA) { - unsigned char aPacked[4]; + unsigned char aPacked[sizeof(int32_t)]; uint_to_bytes_be(aPacked, i); EXPECT_EQ(bytes_be_to_uint(aPacked), i); } @@ -21,7 +21,7 @@ TEST(BytePacking, RoundtripUnsigned) { for(auto u : UINT_DATA) { - unsigned char aPacked[4]; + unsigned char aPacked[sizeof(uint32_t)]; uint_to_bytes_be(aPacked, u); EXPECT_EQ(bytes_be_to_uint(aPacked), u); } From ef2aa13c7303005f67ac3088b4b6c8c4b685782b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=BCller?= Date: Sat, 4 Feb 2023 01:22:57 +0100 Subject: [PATCH 4/4] Update documentation of `bytes_be_to_uint` and `uint_to_bytes_be` --- src/base/system.h | 42 +++++++++++++++++++++++------------------- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/src/base/system.h b/src/base/system.h index a5efde215..1a379c957 100644 --- a/src/base/system.h +++ b/src/base/system.h @@ -2386,27 +2386,31 @@ const char *str_next_token(const char *str, const char *delim, char *buffer, int */ int str_in_list(const char *list, const char *delim, const char *needle); -/* - Function: bytes_be_to_uint - Packs 4 big endian bytes into an unsigned - - Returns: - The packed unsigned - - Remarks: - - Assumes the passed array is 4 bytes - - Assumes unsigned is 4 bytes -*/ +/** + * Packs 4 big endian bytes into an unsigned. + * + * @param bytes Pointer to an array of bytes that will be packed. + * + * @return The packed unsigned. + * + * @remark Assumes the passed array is least 4 bytes in size. + * @remark Assumes unsigned is 4 bytes in size. + * + * @see uint_to_bytes_be + */ unsigned bytes_be_to_uint(const unsigned char *bytes); -/* - Function: uint_to_bytes_be - Packs an unsigned into 4 big endian bytes - - Remarks: - - Assumes the passed array is 4 bytes - - Assumes unsigned is 4 bytes -*/ +/** + * Packs an unsigned into 4 big endian bytes. + * + * @param bytes Pointer to an array where the bytes will be stored. + * @param value The values that will be packed into the array. + * + * @remark Assumes the passed array is least 4 bytes in size. + * @remark Assumes unsigned is 4 bytes in size. + * + * @see bytes_be_to_uint + */ void uint_to_bytes_be(unsigned char *bytes, unsigned value); /*