edg-l/ddnet
1
0
Fork 0
mirror of https://github.com/ddnet/ddnet.git synced 2024-11-10 01:58:19 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge #5430

Browse source 
5430: Add `io_read_all` and `io_read_all_str` to system, add `IStorage::ReadFile` and `ReadFileStr` r=heinrich5991 a=Robyt3

- `io_read_all` reads all bytes from a file handle into a new buffer. It should only need one allocation per file in cases where the actual file size matches the expected file size. Otherwise it falls back to doubling the buffer size if the actual file size is larger than expected to avoid TOCTOU problems.
- `io_read_all_str` reads all bytes from a file handle into a new buffer and also ensures that the buffer is null-terminated and contains no other null-characters.
- `mem_has_null` is a utility used by `io_read_all_str` to ensure that no null-characters exist in the bytes read from the file.
- `IStorage::ReadFile` and `ReadFileStr` are storage wrappers around `io_read_all` and `io_read_all_str`

The system functions are the same as on upstream (https://github.com/teeworlds/teeworlds/pull/3141) when `@heinrich5991` last reviewed them, with an additional check for the result of `io_length` added, as this can return -1 on error (0af8f7796c (diff-d169179e18778d751b412fb173b338885f6db0c4d268c54ddf0e8488a2f7049cR271-R277)).

- Remove duplicate `Kernel()->RequestInterface<IEngineTextRender>()` in client.
- Fix potential memory leak of font data when default font already exists.
- Fix memory leak in editor when loading opus file fails.
- No need for `IOFLAG_SKIP_BOM` on json files because the json parser already does it: b8a82f71aa/src/engine/external/json-parser/json.c (L228-L236)

## 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 if it works standalone, system.c especially
- [ ] 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] 2022-06-16 13:44:49 +00:00 committed by GitHub
parent edab7c0e8c dc52377fc5
commit 992723d71e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 197 additions and 147 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

59
src/base/system.cpp
View file

@ -266,6 +266,51 @@ unsigned io_read(IOHANDLE io, void *buffer, unsigned size)
return fread(buffer, 1, size, (FILE *)io);
}
void io_read_all(IOHANDLE io, void **result, unsigned *result_len)
{
long signed_len = io_length(io);
unsigned len = signed_len < 0 ? 1024 : (unsigned)signed_len; // use default initial size if we couldn't get the length
char *buffer = (char *)malloc(len + 1);
unsigned read = io_read(io, buffer, len + 1); // +1 to check if the file size is larger than expected
if(read < len)
{
buffer = (char *)realloc(buffer, read + 1);
len = read;
}
else if(read > len)
{
unsigned cap = 2 * read;
len = read;
buffer = (char *)realloc(buffer, cap);
while((read = io_read(io, buffer + len, cap - len)) != 0)
{
len += read;
if(len == cap)
{
cap *= 2;
buffer = (char *)realloc(buffer, cap);
}
}
buffer = (char *)realloc(buffer, len + 1);
}
buffer[len] = 0;
*result = buffer;
*result_len = len;
}
char *io_read_all_str(IOHANDLE io)
{
void *buffer;
unsigned len;
io_read_all(io, &buffer, &len);
if(mem_has_null(buffer, len))
{
free(buffer);
return nullptr;
}
return (char *)buffer;
}
unsigned io_skip(IOHANDLE io, int size)
{
fseek((FILE *)io, size, SEEK_CUR);
@ -3244,6 +3289,20 @@ int mem_comp(const void *a, const void *b, int size)
return memcmp(a, b, size);
}
int mem_has_null(const void *block, unsigned size)
{
const unsigned char *bytes = (const unsigned char *)block;
unsigned i;
for(i = 0; i < size; i++)
{
if(bytes[i] == 0)
{
return 1;
}
}
return 0;
}
void net_stats(NETSTATS *stats_inout)
{
*stats_inout = network_stats;

43
src/base/system.h
View file

@ -163,6 +163,19 @@ void mem_zero(void *block, unsigned size);
*/
int mem_comp(const void *a, const void *b, int size);
/**
* Checks whether a block of memory contains null bytes.
*
* @ingroup Memory
*
* @param block Pointer to the block to check for nulls.
* @param size Size of the block.
*
* @return 1 - The block has a null byte.
* @return 0 - The block does not have a null byte.
*/
int mem_has_null(const void *block, unsigned size);
/**
* @defgroup File-IO
*
@ -217,6 +230,36 @@ IOHANDLE io_open(const char *filename, int flags);
*/
unsigned io_read(IOHANDLE io, void *buffer, unsigned size);
/**
* Reads the rest of the file into a buffer.
*
* @ingroup File-IO
*
* @param io Handle to the file to read data from.
* @param result Receives the file's remaining contents.
* @param result_len Receives the file's remaining length.
*
* @remark Does NOT guarantee that there are no internal null bytes.
* @remark The result must be freed after it has been used.
*/
void io_read_all(IOHANDLE io, void **result, unsigned *result_len);
/**
* Reads the rest of the file into a zero-terminated buffer with
* no internal null bytes.
*
* @ingroup File-IO
*
* @param io Handle to the file to read data from.
*
* @return The file's remaining contents or null on failure.
*
* @remark Guarantees that there are no internal null bytes.
* @remark Guarantees that result will contain zero-termination.
* @remark The result must be freed after it has been used.
*/
char *io_read_all_str(IOHANDLE io);
/**
* Skips data in a file.
*

17
src/engine/client/backend/vulkan/backend_vulkan.cpp
View file

@ -4384,18 +4384,15 @@ public:
auto it = m_ShaderFiles.find(pFileName);
if(it == m_ShaderFiles.end())
{
auto *pShaderCodeFile = m_pStorage->OpenFile(pFileName, IOFLAG_READ, IStorage::TYPE_ALL);
void *pShaderBuff;
unsigned FileSize;
if(!m_pStorage->ReadFile(pFileName, IStorage::TYPE_ALL, &pShaderBuff, &FileSize))
return false;
std::vector<uint8_t> vShaderBuff;
if(pShaderCodeFile)
{
long FileSize = io_length(pShaderCodeFile);
vShaderBuff.resize(FileSize);
io_read(pShaderCodeFile, vShaderBuff.data(), FileSize);
io_close(pShaderCodeFile);
}
else
return false;
vShaderBuff.reserve(FileSize);
mem_copy(vShaderBuff.data(), pShaderBuff, FileSize);
free(pShaderBuff);
it = m_ShaderFiles.insert({pFileName, {std::move(vShaderBuff)}}).first;
}

23
src/engine/client/client.cpp
View file

@ -4139,14 +4139,16 @@ void CClient::LoadFont()
IOHANDLE File = Storage()->OpenFile(pFontFile, IOFLAG_READ, IStorage::TYPE_ALL, aFilename, sizeof(aFilename));
if(File)
{
size_t Size = io_length(File);
unsigned char *pBuf = (unsigned char *)malloc(Size);
io_read(File, pBuf, Size);
io_close(File);
IEngineTextRender *pTextRender = Kernel()->RequestInterface<IEngineTextRender>();
pDefaultFont = pTextRender->GetFont(aFilename);
if(pDefaultFont == NULL)
pDefaultFont = pTextRender->LoadFont(aFilename, pBuf, Size);
{
void *pBuf;
unsigned Size;
io_read_all(File, &pBuf, &Size);
pDefaultFont = pTextRender->LoadFont(aFilename, (unsigned char *)pBuf, Size);
}
io_close(File);
for(auto &pFallbackFontFile : apFallbackFontFiles)
{
@ -4154,12 +4156,11 @@ void CClient::LoadFont()
File = Storage()->OpenFile(pFallbackFontFile, IOFLAG_READ, IStorage::TYPE_ALL, aFilename, sizeof(aFilename));
if(File)
{
Size = io_length(File);
pBuf = (unsigned char *)malloc(Size);
io_read(File, pBuf, Size);
void *pBuf;
unsigned Size;
io_read_all(File, &pBuf, &Size);
io_close(File);
pTextRender = Kernel()->RequestInterface<IEngineTextRender>();
FontLoaded = pTextRender->LoadFallbackFont(pDefaultFont, aFilename, pBuf, Size);
FontLoaded = pTextRender->LoadFallbackFont(pDefaultFont, aFilename, (unsigned char *)pBuf, Size);
}
if(!FontLoaded)
@ -4169,7 +4170,7 @@ void CClient::LoadFont()
}
}
Kernel()->RequestInterface<IEngineTextRender>()->SetDefaultFont(pDefaultFont);
pTextRender->SetDefaultFont(pDefaultFont);
}
if(!pDefaultFont)

20
src/engine/client/serverbrowser.cpp
View file

@ -1480,26 +1480,14 @@ int CServerBrowser::HasRank(const char *pMap)
void CServerBrowser::LoadDDNetInfoJson()
{
IOHANDLE File = m_pStorage->OpenFile(DDNET_INFO, IOFLAG_READ | IOFLAG_SKIP_BOM, IStorage::TYPE_SAVE);
if(!File)
void *pBuf;
unsigned Length;
if(!m_pStorage->ReadFile(DDNET_INFO, IStorage::TYPE_SAVE, &pBuf, &Length))
return;
const int Length = io_length(File);
if(Length <= 0)
{
io_close(File);
return;
}
char *pBuf = (char *)malloc(Length);
pBuf[0] = '\0';
io_read(File, pBuf, Length);
io_close(File);
json_value_free(m_pDDNetInfo);
m_pDDNetInfo = json_parse(pBuf, Length);
m_pDDNetInfo = json_parse((json_char *)pBuf, Length);
free(pBuf);

58
src/engine/client/sound.cpp
View file

@ -609,32 +609,23 @@ int CSound::LoadOpus(const char *pFilename)
if(!m_pStorage)
return -1;
IOHANDLE File = m_pStorage->OpenFile(pFilename, IOFLAG_READ, IStorage::TYPE_ALL);
if(!File)
{
dbg_msg("sound/opus", "failed to open file. filename='%s'", pFilename);
return -1;
}
int SampleID = AllocID();
int DataSize = io_length(File);
if(SampleID < 0 || DataSize <= 0)
if(SampleID < 0)
{
io_close(File);
File = NULL;
dbg_msg("sound/opus", "failed to open file. filename='%s'", pFilename);
dbg_msg("sound/opus", "failed to allocate sample ID. filename='%s'", pFilename);
return -1;
}
// read the whole file into memory
char *pData = new char[DataSize];
io_read(File, pData, DataSize);
void *pData;
unsigned DataSize;
if(!m_pStorage->ReadFile(pFilename, IStorage::TYPE_ALL, &pData, &DataSize))
{
dbg_msg("sound/opus", "failed to open file. filename='%s'", pFilename);
return -1;
}
SampleID = DecodeOpus(SampleID, pData, DataSize);
delete[] pData;
io_close(File);
File = NULL;
free(pData);
if(g_Config.m_Debug)
dbg_msg("sound/opus", "loaded %s", pFilename);
@ -658,32 +649,23 @@ int CSound::LoadWV(const char *pFilename)
if(!m_pStorage)
return -1;
IOHANDLE File = m_pStorage->OpenFile(pFilename, IOFLAG_READ, IStorage::TYPE_ALL);
if(!File)
{
dbg_msg("sound/wv", "failed to open file. filename='%s'", pFilename);
return -1;
}
int SampleID = AllocID();
int DataSize = io_length(File);
if(SampleID < 0 || DataSize <= 0)
if(SampleID < 0)
{
io_close(File);
File = NULL;
dbg_msg("sound/wv", "failed to open file. filename='%s'", pFilename);
dbg_msg("sound/wv", "failed to allocate sample ID. filename='%s'", pFilename);
return -1;
}
// read the whole file into memory
char *pData = new char[DataSize];
io_read(File, pData, DataSize);
void *pData;
unsigned DataSize;
if(!m_pStorage->ReadFile(pFilename, IStorage::TYPE_ALL, &pData, &DataSize))
{
dbg_msg("sound/wv", "failed to open file. filename='%s'", pFilename);
return -1;
}
SampleID = DecodeWV(SampleID, pData, DataSize);
delete[] pData;
io_close(File);
File = NULL;
free(pData);
if(g_Config.m_Debug)
dbg_msg("sound/wv", "loaded %s", pFilename);

8
src/engine/client/text.cpp
View file

@ -699,11 +699,11 @@ public:
IOHANDLE File = pStorage->OpenFile(pFontFile, IOFLAG_READ, IStorage::TYPE_ALL, aFilename, sizeof(aFilename));
if(File)
{
size_t Size = io_length(File);
unsigned char *pBuf = (unsigned char *)malloc(Size);
io_read(File, pBuf, Size);
void *pBuf;
unsigned Size;
io_read_all(File, &pBuf, &Size);
io_close(File);
LoadFont(aFilename, pBuf, Size);
LoadFont(aFilename, (unsigned char *)pBuf, Size);
}
}

13
src/engine/client/updater.cpp
View file

@ -242,17 +242,12 @@ bool CUpdater::ReplaceServer()
void CUpdater::ParseUpdate()
{
char aPath[IO_MAX_PATH_LENGTH];
IOHANDLE File = m_pStorage->OpenFile(m_pStorage->GetBinaryPath("update/update.json", aPath, sizeof aPath), IOFLAG_READ | IOFLAG_SKIP_BOM, IStorage::TYPE_ABSOLUTE);
if(!File)
void *pBuf;
unsigned Length;
if(!m_pStorage->ReadFile(m_pStorage->GetBinaryPath("update/update.json", aPath, sizeof aPath), IStorage::TYPE_ABSOLUTE, &pBuf, &Length))
return;
long int Length = io_length(File);
char *pBuf = (char *)malloc(Length);
mem_zero(pBuf, Length);
io_read(File, pBuf, Length);
io_close(File);
json_value *pVersions = json_parse(pBuf, Length);
json_value *pVersions = json_parse((json_char *)pBuf, Length);
free(pBuf);
if(pVersions && pVersions->type == json_array)

17
src/engine/server/server.cpp
View file

@ -2490,20 +2490,18 @@ int CServer::LoadMap(const char *pMapName)
// load complete map into memory for download
{
IOHANDLE File = Storage()->OpenFile(aBuf, IOFLAG_READ, IStorage::TYPE_ALL);
m_aCurrentMapSize[MAP_TYPE_SIX] = (unsigned int)io_length(File);
free(m_apCurrentMapData[MAP_TYPE_SIX]);
m_apCurrentMapData[MAP_TYPE_SIX] = (unsigned char *)malloc(m_aCurrentMapSize[MAP_TYPE_SIX]);
io_read(File, m_apCurrentMapData[MAP_TYPE_SIX], m_aCurrentMapSize[MAP_TYPE_SIX]);
io_close(File);
void *pData;
Storage()->ReadFile(aBuf, IStorage::TYPE_ALL, &pData, &m_aCurrentMapSize[MAP_TYPE_SIX]);
m_apCurrentMapData[MAP_TYPE_SIX] = (unsigned char *)pData;
}
// load sixup version of the map
if(Config()->m_SvSixup)
{
str_format(aBuf, sizeof(aBuf), "maps7/%s.map", pMapName);
IOHANDLE File = Storage()->OpenFile(aBuf, IOFLAG_READ, IStorage::TYPE_ALL);
if(!File)
void *pData;
if(!Storage()->ReadFile(aBuf, IStorage::TYPE_ALL, &pData, &m_aCurrentMapSize[MAP_TYPE_SIXUP]))
{
Config()->m_SvSixup = 0;
if(m_pRegister)
@ -2516,11 +2514,8 @@ int CServer::LoadMap(const char *pMapName)
}
else
{
m_aCurrentMapSize[MAP_TYPE_SIXUP] = (unsigned int)io_length(File);
free(m_apCurrentMapData[MAP_TYPE_SIXUP]);
m_apCurrentMapData[MAP_TYPE_SIXUP] = (unsigned char *)malloc(m_aCurrentMapSize[MAP_TYPE_SIXUP]);
io_read(File, m_apCurrentMapData[MAP_TYPE_SIXUP], m_aCurrentMapSize[MAP_TYPE_SIXUP]);
io_close(File);
m_apCurrentMapData[MAP_TYPE_SIXUP] = (unsigned char *)pData;
m_aCurrentMapSha256[MAP_TYPE_SIXUP] = sha256(m_apCurrentMapData[MAP_TYPE_SIXUP], m_aCurrentMapSize[MAP_TYPE_SIXUP]);
m_aCurrentMapCrc[MAP_TYPE_SIXUP] = crc32(0, m_apCurrentMapData[MAP_TYPE_SIXUP], m_aCurrentMapSize[MAP_TYPE_SIXUP]);

24
src/engine/shared/storage.cpp
View file

@ -409,6 +409,30 @@ public:
return 0;
}
bool ReadFile(const char *pFilename, int Type, void **ppResult, unsigned *pResultLen) override
{
IOHANDLE File = OpenFile(pFilename, IOFLAG_READ, Type);
if(!File)
{
*ppResult = nullptr;
*pResultLen = 0;
return false;
}
io_read_all(File, ppResult, pResultLen);
io_close(File);
return true;
}
char *ReadFileStr(const char *pFilename, int Type) override
{
IOHANDLE File = OpenFile(pFilename, IOFLAG_READ | IOFLAG_SKIP_BOM, Type);
if(!File)
return nullptr;
char *pResult = io_read_all_str(File);
io_close(File);
return pResult;
}
struct CFindCBData
{
CStorage *m_pStorage;

2
src/engine/storage.h
View file

@ -28,6 +28,8 @@ public:
virtual void ListDirectory(int Type, const char *pPath, FS_LISTDIR_CALLBACK pfnCallback, void *pUser) = 0;
virtual void ListDirectoryInfo(int Type, const char *pPath, FS_LISTDIR_CALLBACK_FILEINFO pfnCallback, void *pUser) = 0;
virtual IOHANDLE OpenFile(const char *pFilename, int Flags, int Type, char *pBuffer = nullptr, int BufferSize = 0) = 0;
virtual bool ReadFile(const char *pFilename, int Type, void **ppResult, unsigned *pResultLen) = 0;
virtual char *ReadFileStr(const char *pFilename, int Type) = 0;
virtual bool FindFile(const char *pFilename, const char *pPath, int Type, char *pBuffer, int BufferSize) = 0;
virtual bool RemoveFile(const char *pFilename, int Type) = 0;
virtual bool RenameFile(const char *pOldFilename, const char *pNewFilename, int Type) = 0;

43
src/game/editor/editor.cpp
View file

@ -3744,36 +3744,26 @@ void CEditor::AddSound(const char *pFileName, int StorageType, void *pUser)
}
// load external
IOHANDLE SoundFile = pEditor->Storage()->OpenFile(pFileName, IOFLAG_READ, StorageType);
if(!SoundFile)
void *pData;
unsigned DataSize;
if(!pEditor->Storage()->ReadFile(pFileName, StorageType, &pData, &DataSize))
{
dbg_msg("sound/opus", "failed to open file. filename='%s'", pFileName);
return;
}
// read the whole file into memory
int DataSize = io_length(SoundFile);
if(DataSize <= 0)
{
io_close(SoundFile);
dbg_msg("sound/opus", "failed to open file. filename='%s'", pFileName);
return;
}
void *pData = malloc((unsigned)DataSize);
io_read(SoundFile, pData, (unsigned)DataSize);
io_close(SoundFile);
// load sound
int SoundId = pEditor->Sound()->LoadOpusFromMem(pData, (unsigned)DataSize, true);
if(SoundId == -1)
{
free(pData);
return;
}
// add sound
CEditorSound *pSound = new CEditorSound(pEditor);
pSound->m_SoundID = SoundId;
pSound->m_DataSize = (unsigned)DataSize;
pSound->m_DataSize = DataSize;
pSound->m_pData = pData;
str_copy(pSound->m_aName, aBuf, sizeof(pSound->m_aName));
pEditor->m_Map.m_vpSounds.push_back(pSound);
@ -3796,27 +3786,14 @@ void CEditor::ReplaceSound(const char *pFileName, int StorageType, void *pUser)
CEditor *pEditor = (CEditor *)pUser;
// load external
IOHANDLE SoundFile = pEditor->Storage()->OpenFile(pFileName, IOFLAG_READ, StorageType);
if(!SoundFile)
void *pData;
unsigned DataSize;
if(!pEditor->Storage()->ReadFile(pFileName, StorageType, &pData, &DataSize))
{
dbg_msg("sound/opus", "failed to open file. filename='%s'", pFileName);
return;
}
// read the whole file into memory
int DataSize = io_length(SoundFile);
if(DataSize <= 0)
{
io_close(SoundFile);
dbg_msg("sound/opus", "failed to open file. filename='%s'", pFileName);
return;
}
void *pData = malloc((unsigned)DataSize);
io_read(SoundFile, pData, (unsigned)DataSize);
io_close(SoundFile);
CEditorSound *pSound = pEditor->m_Map.m_vpSounds[pEditor->m_SelectedSound];
// unload sample

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

@ -545,22 +545,9 @@ int CEditorMap::Load(class IStorage *pStorage, const char *pFileName, int Storag
str_format(aBuf, sizeof(aBuf), "mapres/%s.opus", pName);
// load external
IOHANDLE SoundFile = pStorage->OpenFile(pName, IOFLAG_READ, IStorage::TYPE_ALL);
if(SoundFile)
if(pStorage->ReadFile(pName, IStorage::TYPE_ALL, &pSound->m_pData, &pSound->m_DataSize))
{
// read the whole file into memory
pSound->m_DataSize = io_length(SoundFile);
if(pSound->m_DataSize > 0)
{
pSound->m_pData = malloc(pSound->m_DataSize);
io_read(SoundFile, pSound->m_pData, pSound->m_DataSize);
}
io_close(SoundFile);
if(pSound->m_DataSize > 0)
{
pSound->m_SoundID = m_pEditor->Sound()->LoadOpusFromMem(pSound->m_pData, pSound->m_DataSize, true);
}
pSound->m_SoundID = m_pEditor->Sound()->LoadOpusFromMem(pSound->m_pData, pSound->m_DataSize, true);
}
}
else