edg-l/ddnet
1
0
Fork 0
mirror of https://github.com/ddnet/ddnet.git synced 2024-09-20 01:24:18 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge #6588 #6589

Browse source 
6588: Add default argument to `CDataFileReader/IMap::GetItem` r=def- a=Robyt3

Add default value `nullptr` for `int *pType` and `int *pID` output parameters of `GetItem` functions.

Use `nullptr` instead of `0`.

## Checklist

- [ ] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test (especially base/) or added coverage to integration test
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)


6589: Fix heap-use-after-free when quitting/restarting r=def- a=Robyt3

Items and particles were also rendered in the client states quitting and restarting, which was causing accesses to the already freed map data.

Closes #6387 until further notice.

## Checklist

- [X] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test (especially base/) or added coverage to integration test
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)


Co-authored-by: Robert Müller <robytemueller@gmail.com>
This commit is contained in:
bors[bot] 2023-05-14 18:58:08 +00:00 committed by GitHub
parent 03d17a98ed d08626cf69 f3e8513477
commit cca6d867cb
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
18 changed files with 39 additions and 39 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
src/engine/map.h
View file

@ -19,7 +19,7 @@ public:
virtual int GetDataSize(int Index) = 0;
virtual void *GetDataSwapped(int Index) = 0;
virtual void UnloadData(int Index) = 0;
virtual void *GetItem(int Index, int *pType, int *pID) = 0;
virtual void *GetItem(int Index, int *pType = nullptr, int *pID = nullptr) = 0;
virtual int GetItemSize(int Index) = 0;
virtual void GetType(int Type, int *pStart, int *pNum) = 0;
virtual void *FindItem(int Type, int ID) = 0;

8
src/engine/shared/datafile.cpp
View file

@ -354,7 +354,7 @@ int CDataFileReader::GetExternalItemType(int InternalType)
{
return InternalType;
}
const CItemEx *pItemEx = (const CItemEx *)GetItem(TypeIndex, 0, 0);
const CItemEx *pItemEx = (const CItemEx *)GetItem(TypeIndex);
// Propagate UUID_UNKNOWN, it doesn't hurt.
return g_UuidManager.LookupUuid(pItemEx->ToUuid());
}
@ -375,7 +375,7 @@ int CDataFileReader::GetInternalItemType(int ExternalType)
continue;
}
int ID;
if(Uuid == ((const CItemEx *)GetItem(i, 0, &ID))->ToUuid())
if(Uuid == ((const CItemEx *)GetItem(i, nullptr, &ID))->ToUuid())
{
return ID;
}
@ -439,7 +439,7 @@ int CDataFileReader::FindItemIndex(int Type, int ID)
for(int i = 0; i < Num; i++)
{
int ItemID;
GetItem(Start + i, 0, &ItemID);
GetItem(Start + i, nullptr, &ItemID);
if(ID == ItemID)
{
return Start + i;
@ -455,7 +455,7 @@ void *CDataFileReader::FindItem(int Type, int ID)
{
return 0;
}
return GetItem(Index, 0, 0);
return GetItem(Index);
}
int CDataFileReader::NumItems() const

2
src/engine/shared/datafile.h
View file

@ -39,7 +39,7 @@ public:
void *GetDataSwapped(int Index); // makes sure that the data is 32bit LE ints when saved
int GetDataSize(int Index);
void UnloadData(int Index);
void *GetItem(int Index, int *pType, int *pID);
void *GetItem(int Index, int *pType = nullptr, int *pID = nullptr);
int GetItemSize(int Index) const;
void GetType(int Type, int *pStart, int *pNum);
int FindItemIndex(int Type, int ID);

2
src/game/client/components/items.cpp
View file

@ -358,7 +358,7 @@ void CItems::RenderLaser(const CLaserData *pCurrent, bool IsPredicted)
void CItems::OnRender()
{
if(Client()->State() < IClient::STATE_ONLINE)
if(Client()->State() != IClient::STATE_ONLINE && Client()->State() != IClient::STATE_DEMOPLAYBACK)
return;
bool IsSuper = m_pClient->IsLocalCharSuper();

2
src/game/client/components/mapimages.cpp
View file

@ -105,7 +105,7 @@ void CMapImages::OnMapLoadImpl(class CLayers *pLayers, IMap *pMap)
for(int i = 0; i < m_Count; i++)
{
int LoadFlag = (((m_aTextureUsedByTileOrQuadLayerFlag[i] & 1) != 0) ? TextureLoadFlag : 0) | (((m_aTextureUsedByTileOrQuadLayerFlag[i] & 2) != 0) ? 0 : (Graphics()->IsTileBufferingEnabled() ? IGraphics::TEXLOAD_NO_2D_TEXTURE : 0));
CMapItemImage *pImg = (CMapItemImage *)pMap->GetItem(Start + i, 0, 0);
CMapItemImage *pImg = (CMapItemImage *)pMap->GetItem(Start + i);
if(pImg->m_External)
{
char aPath[IO_MAX_PATH_LENGTH];

4
src/game/client/components/maplayers.cpp
View file

@ -66,7 +66,7 @@ void CMapLayers::EnvelopeEval(int TimeOffsetMillis, int Env, ColorRGBA &Channels
int Start, Num;
pThis->m_pLayers->Map()->GetType(MAPITEMTYPE_ENVPOINTS, &Start, &Num);
if(Num)
pPoints = (CEnvPoint *)pThis->m_pLayers->Map()->GetItem(Start, 0, 0);
pPoints = (CEnvPoint *)pThis->m_pLayers->Map()->GetItem(Start);
}
int Start, Num;
@ -75,7 +75,7 @@ void CMapLayers::EnvelopeEval(int TimeOffsetMillis, int Env, ColorRGBA &Channels
if(Env >= Num)
return;
CMapItemEnvelope *pItem = (CMapItemEnvelope *)pThis->m_pLayers->Map()->GetItem(Start + Env, 0, 0);
CMapItemEnvelope *pItem = (CMapItemEnvelope *)pThis->m_pLayers->Map()->GetItem(Start + Env);
const auto TickToNanoSeconds = std::chrono::nanoseconds(1s) / (int64_t)pThis->Client()->GameTickSpeed();

2
src/game/client/components/mapsounds.cpp
View file

@ -32,7 +32,7 @@ void CMapSounds::OnMapLoad()
{
m_aSounds[i] = 0;
CMapItemSound *pSound = (CMapItemSound *)pMap->GetItem(Start + i, 0, 0);
CMapItemSound *pSound = (CMapItemSound *)pMap->GetItem(Start + i);
if(pSound->m_External)
{
char aBuf[IO_MAX_PATH_LENGTH];

2
src/game/client/components/particles.cpp
View file

@ -145,7 +145,7 @@ void CParticles::Update(float TimePassed)
void CParticles::OnRender()
{
if(Client()->State() < IClient::STATE_ONLINE)
if(Client()->State() != IClient::STATE_ONLINE && Client()->State() != IClient::STATE_DEMOPLAYBACK)
return;
set_new_tick();

2
src/game/client/gameclient.cpp
View file

@ -3222,7 +3222,7 @@ void CGameClient::LoadMapSettings()
for(int i = Start; i < Start + Num; i++)
{
int ItemID;
CMapItemInfoSettings *pItem = (CMapItemInfoSettings *)pMap->GetItem(i, 0, &ItemID);
CMapItemInfoSettings *pItem = (CMapItemInfoSettings *)pMap->GetItem(i, nullptr, &ItemID);
int ItemSize = pMap->GetItemSize(i);
if(!pItem || ItemID != 0)
continue;

16
src/game/editor/io.cpp
View file

@ -479,7 +479,7 @@ bool CEditorMap::Load(class IStorage *pStorage, const char *pFileName, int Stora
DataFile.GetType(MAPITEMTYPE_IMAGE, &Start, &Num);
for(int i = 0; i < Num; i++)
{
CMapItemImage *pItem = (CMapItemImage *)DataFile.GetItem(Start + i, nullptr, nullptr);
CMapItemImage *pItem = (CMapItemImage *)DataFile.GetItem(Start + i);
char *pName = (char *)DataFile.GetData(pItem->m_ImageName);
// copy base info
@ -542,7 +542,7 @@ bool CEditorMap::Load(class IStorage *pStorage, const char *pFileName, int Stora
DataFile.GetType(MAPITEMTYPE_SOUND, &Start, &Num);
for(int i = 0; i < Num; i++)
{
CMapItemSound *pItem = (CMapItemSound *)DataFile.GetItem(Start + i, nullptr, nullptr);
CMapItemSound *pItem = (CMapItemSound *)DataFile.GetItem(Start + i);
char *pName = (char *)DataFile.GetData(pItem->m_SoundName);
// copy base info
@ -594,10 +594,10 @@ bool CEditorMap::Load(class IStorage *pStorage, const char *pFileName, int Stora
DataFile.GetType(MAPITEMTYPE_GROUP_EX, &StartEx, &NumEx);
for(int g = 0; g < Num; g++)
{
CMapItemGroup *pGItem = (CMapItemGroup *)DataFile.GetItem(Start + g, nullptr, nullptr);
CMapItemGroup *pGItem = (CMapItemGroup *)DataFile.GetItem(Start + g);
CMapItemGroupEx *pGItemEx = nullptr;
if(NumEx)
pGItemEx = (CMapItemGroupEx *)DataFile.GetItem(StartEx + g, nullptr, nullptr);
pGItemEx = (CMapItemGroupEx *)DataFile.GetItem(StartEx + g);
if(pGItem->m_Version < 1 || pGItem->m_Version > CMapItemGroup::CURRENT_VERSION)
continue;
@ -627,7 +627,7 @@ bool CEditorMap::Load(class IStorage *pStorage, const char *pFileName, int Stora
for(int l = 0; l < pGItem->m_NumLayers; l++)
{
CLayer *pLayer = nullptr;
CMapItemLayer *pLayerItem = (CMapItemLayer *)DataFile.GetItem(LayersStart + pGItem->m_StartLayer + l, nullptr, nullptr);
CMapItemLayer *pLayerItem = (CMapItemLayer *)DataFile.GetItem(LayersStart + pGItem->m_StartLayer + l);
if(!pLayerItem)
continue;
@ -924,14 +924,14 @@ bool CEditorMap::Load(class IStorage *pStorage, const char *pFileName, int Stora
int Start, Num;
DataFile.GetType(MAPITEMTYPE_ENVPOINTS, &Start, &Num);
if(Num)
pPoints = (CEnvPoint *)DataFile.GetItem(Start, nullptr, nullptr);
pPoints = (CEnvPoint *)DataFile.GetItem(Start);
}
int Start, Num;
DataFile.GetType(MAPITEMTYPE_ENVELOPE, &Start, &Num);
for(int e = 0; e < Num; e++)
{
CMapItemEnvelope *pItem = (CMapItemEnvelope *)DataFile.GetItem(Start + e, nullptr, nullptr);
CMapItemEnvelope *pItem = (CMapItemEnvelope *)DataFile.GetItem(Start + e);
CEnvelope *pEnv = new CEnvelope(pItem->m_Channels);
pEnv->m_vPoints.resize(pItem->m_NumPoints);
mem_copy(pEnv->m_vPoints.data(), &pPoints[pItem->m_StartPoint], sizeof(CEnvPoint) * pItem->m_NumPoints);
@ -948,7 +948,7 @@ bool CEditorMap::Load(class IStorage *pStorage, const char *pFileName, int Stora
DataFile.GetType(MAPITEMTYPE_AUTOMAPPER_CONFIG, &Start, &Num);
for(int i = 0; i < Num; i++)
{
CMapItemAutoMapperConfig *pItem = (CMapItemAutoMapperConfig *)DataFile.GetItem(Start + i, nullptr, nullptr);
CMapItemAutoMapperConfig *pItem = (CMapItemAutoMapperConfig *)DataFile.GetItem(Start + i);
if(pItem->m_Version == CMapItemAutoMapperConfig::CURRENT_VERSION)
{
if(pItem->m_GroupId >= 0 && (size_t)pItem->m_GroupId < m_vpGroups.size() &&

6
src/game/layers.cpp
View file

@ -220,7 +220,7 @@ void CLayers::InitTilemapSkip()
CMapItemGroup *CLayers::GetGroup(int Index) const
{
return static_cast<CMapItemGroup *>(m_pMap->GetItem(m_GroupsStart + Index, 0, 0));
return static_cast<CMapItemGroup *>(m_pMap->GetItem(m_GroupsStart + Index));
}
CMapItemGroupEx *CLayers::GetGroupEx(int Index) const
@ -229,10 +229,10 @@ CMapItemGroupEx *CLayers::GetGroupEx(int Index) const
if(m_GroupsExNum != m_GroupsNum)
return nullptr;
return static_cast<CMapItemGroupEx *>(m_pMap->GetItem(m_GroupsExStart + Index, 0, 0));
return static_cast<CMapItemGroupEx *>(m_pMap->GetItem(m_GroupsExStart + Index));
}
CMapItemLayer *CLayers::GetLayer(int Index) const
{
return static_cast<CMapItemLayer *>(m_pMap->GetItem(m_LayersStart + Index, 0, 0));
return static_cast<CMapItemLayer *>(m_pMap->GetItem(m_LayersStart + Index));
}

2
src/game/server/gamecontext.cpp
View file

@ -3805,7 +3805,7 @@ void CGameContext::LoadMapSettings()
for(int i = Start; i < Start + Num; i++)
{
int ItemID;
CMapItemInfoSettings *pItem = (CMapItemInfoSettings *)pMap->GetItem(i, 0, &ItemID);
CMapItemInfoSettings *pItem = (CMapItemInfoSettings *)pMap->GetItem(i, nullptr, &ItemID);
int ItemSize = pMap->GetItemSize(i);
if(!pItem || ItemID != 0)
continue;

4
src/tools/map_convert_07.cpp
View file

@ -80,7 +80,7 @@ bool CheckImageDimensions(void *pLayerItem, int LayerType, const char *pFilename
return true;
int Type;
void *pItem = g_DataReader.GetItem(g_aImageIDs[pTMap->m_Image], &Type, nullptr);
void *pItem = g_DataReader.GetItem(g_aImageIDs[pTMap->m_Image], &Type);
if(Type != MAPITEMTYPE_IMAGE)
return true;
@ -191,7 +191,7 @@ int main(int argc, const char **argv)
for(int Index = 0; Index < g_DataReader.NumItems(); Index++)
{
int Type;
g_DataReader.GetItem(Index, &Type, nullptr);
g_DataReader.GetItem(Index, &Type);
if(Type == MAPITEMTYPE_IMAGE)
g_aImageIDs[i++] = Index;
}

4
src/tools/map_create_pixelart.cpp
View file

@ -253,7 +253,7 @@ CMapItemLayerQuads *GetQuadLayer(CDataFileReader &InputMap, const int aLayerID[2
int Start, Num;
InputMap.GetType(MAPITEMTYPE_GROUP, &Start, &Num);
CMapItemGroup *pGroupItem = aLayerID[0] >= Num ? 0x0 : (CMapItemGroup *)InputMap.GetItem(Start + aLayerID[0], 0, 0);
CMapItemGroup *pGroupItem = aLayerID[0] >= Num ? 0x0 : (CMapItemGroup *)InputMap.GetItem(Start + aLayerID[0]);
if(!pGroupItem)
{
@ -264,7 +264,7 @@ CMapItemLayerQuads *GetQuadLayer(CDataFileReader &InputMap, const int aLayerID[2
InputMap.GetType(MAPITEMTYPE_LAYER, &Start, &Num);
*pItemNumber = Start + pGroupItem->m_StartLayer + aLayerID[1];
CMapItemLayer *pLayerItem = aLayerID[1] >= pGroupItem->m_NumLayers ? 0x0 : (CMapItemLayer *)InputMap.GetItem(*pItemNumber, 0, 0);
CMapItemLayer *pLayerItem = aLayerID[1] >= pGroupItem->m_NumLayers ? 0x0 : (CMapItemLayer *)InputMap.GetItem(*pItemNumber);
if(!pLayerItem)
{
dbg_msg("map_create_pixelart", "ERROR: unable to find layer '#%d' in group '#%d'", aLayerID[1], aLayerID[0]);

4
src/tools/map_diff.cpp
View file

@ -41,7 +41,7 @@ bool Process(IStorage *pStorage, const char **pMapNames)
{
for(int i = 0; i < 2; ++i)
{
CMapItemLayer *pItem = (CMapItemLayer *)aMaps[i].GetItem(aStart[i] + j, nullptr, nullptr);
CMapItemLayer *pItem = (CMapItemLayer *)aMaps[i].GetItem(aStart[i] + j);
if(pItem->m_Type == LAYERTYPE_TILES)
(void)aMaps[i].GetData(((CMapItemLayerTilemap *)pItem)->m_Data);
}
@ -52,7 +52,7 @@ bool Process(IStorage *pStorage, const char **pMapNames)
{
CMapItemLayer *apItem[2];
for(int i = 0; i < 2; ++i)
apItem[i] = (CMapItemLayer *)aMaps[i].GetItem(aStart[i] + j, nullptr, nullptr);
apItem[i] = (CMapItemLayer *)aMaps[i].GetItem(aStart[i] + j);
if(apItem[0]->m_Type != LAYERTYPE_TILES || apItem[1]->m_Type != LAYERTYPE_TILES)
continue;

4
src/tools/map_extract.cpp
View file

@ -39,7 +39,7 @@ bool Process(IStorage *pStorage, const char *pMapName, const char *pPathSave)
for(int i = 0; i < Num; i++)
{
CMapItemImage *pItem = (CMapItemImage *)Reader.GetItem(Start + i, nullptr, nullptr);
CMapItemImage *pItem = (CMapItemImage *)Reader.GetItem(Start + i);
char *pName = (char *)Reader.GetData(pItem->m_ImageName);
if(pItem->m_External)
@ -67,7 +67,7 @@ bool Process(IStorage *pStorage, const char *pMapName, const char *pPathSave)
for(int i = 0; i < Num; i++)
{
CMapItemSound *pItem = (CMapItemSound *)Reader.GetItem(Start + i, nullptr, nullptr);
CMapItemSound *pItem = (CMapItemSound *)Reader.GetItem(Start + i);
char *pName = (char *)Reader.GetData(pItem->m_SoundName);
if(pItem->m_External)

4
src/tools/map_find_env.cpp
View file

@ -32,7 +32,7 @@ bool GetLayerGroupIDs(CDataFileReader &InputMap, const int LayerNumber, int &Gro
for(int i = 0; i < Num; i++)
{
CMapItemGroup *pItem = (CMapItemGroup *)InputMap.GetItem(Start + i, 0, 0);
CMapItemGroup *pItem = (CMapItemGroup *)InputMap.GetItem(Start + i);
if(LayerNumber >= pItem->m_StartLayer && LayerNumber <= pItem->m_StartLayer + pItem->m_NumLayers)
{
GroupID = i;
@ -95,7 +95,7 @@ bool FindEnv(const char aFilename[64], const int EnvID)
for(int i = 0; i < LayersCount; i++)
{
CMapItemLayer *pItem;
pItem = (CMapItemLayer *)InputMap.GetItem(LayersStart + i, 0, 0);
pItem = (CMapItemLayer *)InputMap.GetItem(LayersStart + i);
if(pItem->m_Type != LAYERTYPE_QUADS)
continue;

8
src/tools/map_replace_area.cpp
View file

@ -124,7 +124,7 @@ bool ReplaceArea(IStorage *pStorage, const char aaMapNames[3][64], const float a
for(int j = 0; j < 2; j++)
{
apLayerGroups[j] = GetLayerGroup(aInputMaps[j], i + 1);
apItem[j] = (CMapItemLayer *)aInputMaps[j].GetItem(aLayersStart[j] + i, 0, 0);
apItem[j] = (CMapItemLayer *)aInputMaps[j].GetItem(aLayersStart[j] + i);
}
if(!apLayerGroups[0] || !apLayerGroups[1])
@ -205,7 +205,7 @@ bool CompareLayers(const char aaMapNames[3][64], CDataFileReader aInputMaps[2])
{
CMapItemLayer *apItem[2];
for(int j = 0; j < 2; j++)
apItem[j] = (CMapItemLayer *)aInputMaps[j].GetItem(aStart[j] + i, 0, 0);
apItem[j] = (CMapItemLayer *)aInputMaps[j].GetItem(aStart[j] + i);
if(apItem[0]->m_Type != apItem[1]->m_Type)
{
@ -229,7 +229,7 @@ void CompareGroups(const char aaMapNames[3][64], CDataFileReader aInputMaps[2])
{
CMapItemGroup *apItem[2];
for(int j = 0; j < 2; j++)
apItem[j] = (CMapItemGroup *)aInputMaps[j].GetItem(aStart[j] + i, 0, 0);
apItem[j] = (CMapItemGroup *)aInputMaps[j].GetItem(aStart[j] + i);
bool bSameConfig = apItem[0]->m_ParallaxX == apItem[1]->m_ParallaxX && apItem[0]->m_ParallaxY == apItem[1]->m_ParallaxY && apItem[0]->m_OffsetX == apItem[1]->m_OffsetX && apItem[0]->m_OffsetY == apItem[1]->m_OffsetY && apItem[0]->m_UseClipping == apItem[1]->m_UseClipping && apItem[0]->m_ClipX == apItem[1]->m_ClipX && apItem[0]->m_ClipY == apItem[1]->m_ClipY && apItem[0]->m_ClipW == apItem[1]->m_ClipW && apItem[0]->m_ClipH == apItem[1]->m_ClipH;
@ -245,7 +245,7 @@ const CMapItemGroup *GetLayerGroup(CDataFileReader &InputMap, const int LayerNum
for(int i = 0; i < Num; i++)
{
CMapItemGroup *pItem = (CMapItemGroup *)InputMap.GetItem(Start + i, 0, 0);
CMapItemGroup *pItem = (CMapItemGroup *)InputMap.GetItem(Start + i);
if(LayerNumber >= pItem->m_StartLayer && LayerNumber <= pItem->m_StartLayer + pItem->m_NumLayers)
return pItem;
}