Add functions for reading/writing strings from/to datafile

Simplify the usage of datafile reader and writer by adding utility functions to read and write zero-terminated UTF-8 strings.

Improve validation of string data read from datafiles. It is ensure that string data is null-terminated, has no internal NUL-characters and is valid UTF-8.

Fix loading of external sounds in the editor. The wrong path variable was being used, so the sound files would not be loaded from correct folder.

Add tests for new datafile reader/writer functions.

src/engine/map.h

@@ -18,6 +18,7 @@ class IMap : public IInterface
  virtual int GetDataSize(int Index) const = 0;
  virtual void *GetData(int Index) = 0;
  virtual void *GetDataSwapped(int Index) = 0;
+ virtual const char *GetDataString(int Index) = 0;
  virtual void UnloadData(int Index) = 0;
  virtual int NumData() const = 0;
 

src/engine/shared/datafile.cpp

@@ -402,6 +402,19 @@ void *CDataFileReader::GetDataSwapped(int Index)
  return GetDataImpl(Index, true);
 }
 
+const char *CDataFileReader::GetDataString(int Index)
+{
+ if(Index == -1)
+ return "";
+ const int DataSize = GetDataSize(Index);
+ if(!DataSize)
+ return nullptr;
+ const char *pData = static_cast<char *>(GetData(Index));
+ if(pData == nullptr || mem_has_null(pData, DataSize - 1) || pData[DataSize - 1] != '\0' || !str_utf8_check(pData))
+ return nullptr;
+ return pData;
+}
+
 void CDataFileReader::ReplaceData(int Index, char *pData, size_t Size)
 {
  dbg_assert(Index >= 0 && Index < m_pDataFile->m_Header.m_NumRawData, "Index invalid");
@@ -747,6 +760,13 @@ int CDataFileWriter::AddDataSwapped(int Size, const void *pData)
 #endif
 }
 
+int CDataFileWriter::AddDataString(const char *pStr)
+{
+ if(pStr[0] == '\0')
+ return -1;
+ return AddData(str_length(pStr) + 1, pStr);
+}
+
 void CDataFileWriter::Finish()
 {
  dbg_assert((bool)m_File, "file not open");

src/engine/shared/datafile.h

@@ -38,6 +38,7 @@ class CDataFileReader
  int GetDataSize(int Index) const;
  void *GetData(int Index);
  void *GetDataSwapped(int Index); // makes sure that the data is 32bit LE ints when saved
+ const char *GetDataString(int Index);
  void ReplaceData(int Index, char *pData, size_t Size); // memory for data must have been allocated with malloc
  void UnloadData(int Index);
  int NumData() const;
@@ -129,6 +130,7 @@ class CDataFileWriter
  bool Open(class IStorage *pStorage, const char *pFilename, int StorageType = IStorage::TYPE_SAVE);
  int AddData(int Size, const void *pData, int CompressionLevel = Z_DEFAULT_COMPRESSION);
  int AddDataSwapped(int Size, const void *pData);
+ int AddDataString(const char *pStr);
  int AddItem(int Type, int ID, int Size, const void *pData);
  void Finish();
 };

src/engine/shared/map.cpp

@@ -23,6 +23,11 @@ void *CMap::GetDataSwapped(int Index)
  return m_DataFile.GetDataSwapped(Index);
 }
 
+const char *CMap::GetDataString(int Index)
+{
+ return m_DataFile.GetDataString(Index);
+}
+
 void CMap::UnloadData(int Index)
 {
  m_DataFile.UnloadData(Index);

src/engine/shared/map.h

@@ -20,6 +20,7 @@ class CMap : public IEngineMap
  int GetDataSize(int Index) const override;
  void *GetData(int Index) override;
  void *GetDataSwapped(int Index) override;
+ const char *GetDataString(int Index) override;
  void UnloadData(int Index) override;
  int NumData() const override;
 

src/game/client/components/mapimages.cpp

@@ -1,18 +1,19 @@
 /* (c) Magnus Auvinen. See licence.txt in the root of the distribution for more information. */
 /* If you are missing that file, acquire a complete release at teeworlds.com. */
+#include <base/log.h>
+
 #include <engine/graphics.h>
 #include <engine/map.h>
 #include <engine/storage.h>
 #include <engine/textrender.h>
-#include <game/generated/client_data.h>
-#include <game/mapitems.h>
 
+#include <game/client/gameclient.h>
+#include <game/generated/client_data.h>
 #include <game/layers.h>
+#include <game/mapitems.h>
 
 #include "mapimages.h"
 
-#include <game/client/gameclient.h>
-
 const char *const gs_apModEntitiesNames[] = {
  "ddnet",
  "ddrace",
@@ -99,37 +100,37 @@ void CMapImages::OnMapLoadImpl(class CLayers *pLayers, IMap *pMap)
  }
  }
 
- int TextureLoadFlag = Graphics()->HasTextureArrays() ? IGraphics::TEXLOAD_TO_2D_ARRAY_TEXTURE : IGraphics::TEXLOAD_TO_3D_TEXTURE;
+ const int TextureLoadFlag = Graphics()->HasTextureArrays() ? IGraphics::TEXLOAD_TO_2D_ARRAY_TEXTURE : IGraphics::TEXLOAD_TO_3D_TEXTURE;
 
  // load new textures
  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));
+ const int LoadFlag = (((m_aTextureUsedByTileOrQuadLayerFlag[i] & 1) != 0) ? TextureLoadFlag : 0) | (((m_aTextureUsedByTileOrQuadLayerFlag[i] & 2) != 0) ? 0 : (Graphics()->IsTileBufferingEnabled() ? IGraphics::TEXLOAD_NO_2D_TEXTURE : 0));
  const CMapItemImage_v2 *pImg = (CMapItemImage_v2 *)pMap->GetItem(Start + i);
  const int Format = pImg->m_Version < CMapItemImage_v2::CURRENT_VERSION ? CImageInfo::FORMAT_RGBA : pImg->m_Format;
+
+ const char *pName = pMap->GetDataString(pImg->m_ImageName);
+ if(pName == nullptr || pName[0] == '\0')
+ {
+ log_error("mapimages", "Failed to load map image %d: failed to load name.", i);
+ continue;
+ }
+
  if(pImg->m_External)
  {
  char aPath[IO_MAX_PATH_LENGTH];
- char *pName = (char *)pMap->GetData(pImg->m_ImageName);
  str_format(aPath, sizeof(aPath), "mapres/%s.png", pName);
  m_aTextures[i] = Graphics()->LoadTexture(aPath, IStorage::TYPE_ALL, CImageInfo::FORMAT_AUTO, LoadFlag);
- pMap->UnloadData(pImg->m_ImageName);
- }
- else if(Format != CImageInfo::FORMAT_RGBA)
- {
- m_aTextures[i] = Graphics()->InvalidTexture();
- pMap->UnloadData(pImg->m_ImageName);
  }
- else
+ else if(Format == CImageInfo::FORMAT_RGBA)
  {
  void *pData = pMap->GetData(pImg->m_ImageData);
- char *pName = (char *)pMap->GetData(pImg->m_ImageName);
- char aTexName[128];
- str_format(aTexName, sizeof(aTexName), "%s %s", "embedded:", pName);
+ char aTexName[IO_MAX_PATH_LENGTH];
+ str_format(aTexName, sizeof(aTexName), "embedded: %s", pName);
  m_aTextures[i] = Graphics()->LoadTextureRaw(pImg->m_Width, pImg->m_Height, Format, pData, CImageInfo::FORMAT_RGBA, LoadFlag, aTexName);
- pMap->UnloadData(pImg->m_ImageName);
  pMap->UnloadData(pImg->m_ImageData);
  }
+ pMap->UnloadData(pImg->m_ImageName);
  }
 }
 

src/game/client/components/mapsounds.cpp

@@ -1,12 +1,11 @@
+#include <base/log.h>
+
 #include <engine/demo.h>
 #include <engine/sound.h>
 
 #include <game/client/components/camera.h>
-#include <game/client/components/maplayers.h> // envelope
 #include <game/client/components/sounds.h>
-
 #include <game/client/gameclient.h>
-
 #include <game/layers.h>
 #include <game/mapitems.h>
 
@@ -30,15 +29,20 @@ void CMapSounds::OnMapLoad()
  // load new samples
  for(int i = 0; i < m_Count; i++)
  {
- m_aSounds[i] = 0;
-
  CMapItemSound *pSound = (CMapItemSound *)pMap->GetItem(Start + i);
  if(pSound->m_External)
  {
+ const char *pName = pMap->GetDataString(pSound->m_SoundName);
+ if(pName == nullptr || pName[0] == '\0')
+ {
+ log_error("mapsounds", "Failed to load map sound %d: failed to load name.", i);
+ continue;
+ }
+
  char aBuf[IO_MAX_PATH_LENGTH];
- char *pName = (char *)pMap->GetData(pSound->m_SoundName);
  str_format(aBuf, sizeof(aBuf), "mapres/%s.opus", pName);
  m_aSounds[i] = Sound()->LoadOpus(aBuf);
+ pMap->UnloadData(pSound->m_SoundName);
  }
  else
  {

src/game/editor/io.cpp

@@ -66,23 +66,10 @@ bool CEditorMap::Save(const char *pFileName)
  {
  CMapItemInfoSettings Item;
  Item.m_Version = 1;
-
- if(m_MapInfo.m_aAuthor[0])
- Item.m_Author = Writer.AddData(str_length(m_MapInfo.m_aAuthor) + 1, m_MapInfo.m_aAuthor);
- else
- Item.m_Author = -1;
- if(m_MapInfo.m_aVersion[0])
- Item.m_MapVersion = Writer.AddData(str_length(m_MapInfo.m_aVersion) + 1, m_MapInfo.m_aVersion);
- else
- Item.m_MapVersion = -1;
- if(m_MapInfo.m_aCredits[0])
- Item.m_Credits = Writer.AddData(str_length(m_MapInfo.m_aCredits) + 1, m_MapInfo.m_aCredits);
- else
- Item.m_Credits = -1;
- if(m_MapInfo.m_aLicense[0])
- Item.m_License = Writer.AddData(str_length(m_MapInfo.m_aLicense) + 1, m_MapInfo.m_aLicense);
- else
- Item.m_License = -1;
+ Item.m_Author = Writer.AddDataString(m_MapInfo.m_aAuthor);
+ Item.m_MapVersion = Writer.AddDataString(m_MapInfo.m_aVersion);
+ Item.m_Credits = Writer.AddDataString(m_MapInfo.m_aCredits);
+ Item.m_License = Writer.AddDataString(m_MapInfo.m_aLicense);
 
  Item.m_Settings = -1;
  if(!m_vSettings.empty())
@@ -123,7 +110,7 @@ bool CEditorMap::Save(const char *pFileName)
  Item.m_Width = pImg->m_Width;
  Item.m_Height = pImg->m_Height;
  Item.m_External = pImg->m_External;
- Item.m_ImageName = Writer.AddData(str_length(pImg->m_aName) + 1, pImg->m_aName);
+ Item.m_ImageName = Writer.AddDataString(pImg->m_aName);
  if(pImg->m_External)
  {
  Item.m_ImageData = -1;
@@ -162,7 +149,7 @@ bool CEditorMap::Save(const char *pFileName)
  Item.m_Version = 1;
 
  Item.m_External = 0;
- Item.m_SoundName = Writer.AddData(str_length(pSound->m_aName) + 1, pSound->m_aName);
+ Item.m_SoundName = Writer.AddDataString(pSound->m_aName);
  Item.m_SoundData = Writer.AddData(pSound->m_DataSize, pSound->m_pData);
  Item.m_SoundDataSize = pSound->m_DataSize;
 
@@ -478,14 +465,25 @@ bool CEditorMap::Load(const char *pFileName, int StorageType, const std::functio
  if(!pItem || ItemID != 0)
  continue;
 
- if(pItem->m_Author > -1)
- str_copy(m_MapInfo.m_aAuthor, (char *)DataFile.GetData(pItem->m_Author));
- if(pItem->m_MapVersion > -1)
- str_copy(m_MapInfo.m_aVersion, (char *)DataFile.GetData(pItem->m_MapVersion));
- if(pItem->m_Credits > -1)
- str_copy(m_MapInfo.m_aCredits, (char *)DataFile.GetData(pItem->m_Credits));
- if(pItem->m_License > -1)
- str_copy(m_MapInfo.m_aLicense, (char *)DataFile.GetData(pItem->m_License));
+ const auto &&ReadStringInfo = [&](int Index, char *pBuffer, size_t BufferSize, const char *pErrorContext) {
+ const char *pStr = DataFile.GetDataString(Index);
+ if(pStr == nullptr)
+ {
+ char aBuf[128];
+ str_format(aBuf, sizeof(aBuf), "Error: Failed to read %s from map info.", pErrorContext);
+ ErrorHandler(aBuf);
+ pBuffer[0] = '\0';
+ }
+ else
+ {
+ str_copy(pBuffer, pStr, BufferSize);
+ }
+ };
+
+ ReadStringInfo(pItem->m_Author, m_MapInfo.m_aAuthor, sizeof(m_MapInfo.m_aAuthor), "author");
+ ReadStringInfo(pItem->m_MapVersion, m_MapInfo.m_aVersion, sizeof(m_MapInfo.m_aVersion), "version");
+ ReadStringInfo(pItem->m_Credits, m_MapInfo.m_aCredits, sizeof(m_MapInfo.m_aCredits), "credits");
+ ReadStringInfo(pItem->m_License, m_MapInfo.m_aLicense, sizeof(m_MapInfo.m_aLicense), "license");
 
  if(pItem->m_Version != 1 || ItemSize < (int)sizeof(CMapItemInfoSettings))
  break;
@@ -512,17 +510,26 @@ bool CEditorMap::Load(const char *pFileName, int StorageType, const std::functio
  for(int i = 0; i < Num; i++)
  {
  CMapItemImage_v2 *pItem = (CMapItemImage_v2 *)DataFile.GetItem(Start + i);
- char *pName = (char *)DataFile.GetData(pItem->m_ImageName);
 
  // copy base info
  std::shared_ptr<CEditorImage> pImg = std::make_shared<CEditorImage>(m_pEditor);
  pImg->m_External = pItem->m_External;
 
+ const char *pName = DataFile.GetDataString(pItem->m_ImageName);
+ if(pName == nullptr || pName[0] == '\0')
+ {
+ char aBuf[128];
+ str_format(aBuf, sizeof(aBuf), "Error: Failed to read name of image %d.", i);
+ ErrorHandler(aBuf);
+ }
+ else
+ str_copy(pImg->m_aName, pName);
+
  const int Format = pItem->m_Version < CMapItemImage_v2::CURRENT_VERSION ? CImageInfo::FORMAT_RGBA : pItem->m_Format;
  if(pImg->m_External || (Format != CImageInfo::FORMAT_RGB && Format != CImageInfo::FORMAT_RGBA))
  {
  char aBuf[IO_MAX_PATH_LENGTH];
- str_format(aBuf, sizeof(aBuf), "mapres/%s.png", pName);
+ str_format(aBuf, sizeof(aBuf), "mapres/%s.png", pImg->m_aName);
 
  // load external
  CEditorImage ImgInfo(m_pEditor);
@@ -554,10 +561,6 @@ bool CEditorMap::Load(const char *pFileName, int StorageType, const std::functio
  pImg->m_Texture = m_pEditor->Graphics()->LoadTextureRaw(pImg->m_Width, pImg->m_Height, pImg->m_Format, pImg->m_pData, CImageInfo::FORMAT_AUTO, TextureLoadFlag);
  }
 
- // copy image name
- if(pName)
- str_copy(pImg->m_aName, pName);
-
  // load auto mapper file
  pImg->m_AutoMapper.Load(pImg->m_aName);
 
@@ -576,18 +579,27 @@ bool CEditorMap::Load(const char *pFileName, int StorageType, const std::functio
  for(int i = 0; i < Num; i++)
  {
  CMapItemSound *pItem = (CMapItemSound *)DataFile.GetItem(Start + i);
- char *pName = (char *)DataFile.GetData(pItem->m_SoundName);
 
  // copy base info
  std::shared_ptr<CEditorSound> pSound = std::make_shared<CEditorSound>(m_pEditor);
 
+ const char *pName = DataFile.GetDataString(pItem->m_SoundName);
+ if(pName == nullptr || pName[0] == '\0')
+ {
+ char aBuf[128];
+ str_format(aBuf, sizeof(aBuf), "Error: Failed to read name of sound %d.", i);
+ ErrorHandler(aBuf);
+ }
+ else
+ str_copy(pSound->m_aName, pName);
+
  if(pItem->m_External)
  {
  char aBuf[IO_MAX_PATH_LENGTH];
- str_format(aBuf, sizeof(aBuf), "mapres/%s.opus", pName);
+ str_format(aBuf, sizeof(aBuf), "mapres/%s.opus", pSound->m_aName);
 
  // load external
- if(m_pEditor->Storage()->ReadFile(pName, IStorage::TYPE_ALL, &pSound->m_pData, &pSound->m_DataSize))
+ if(m_pEditor->Storage()->ReadFile(aBuf, IStorage::TYPE_ALL, &pSound->m_pData, &pSound->m_DataSize))
  {
  pSound->m_SoundID = m_pEditor->Sound()->LoadOpusFromMem(pSound->m_pData, pSound->m_DataSize, true);
  }
@@ -603,10 +615,6 @@ bool CEditorMap::Load(const char *pFileName, int StorageType, const std::functio
  pSound->m_SoundID = m_pEditor->Sound()->LoadOpusFromMem(pSound->m_pData, pSound->m_DataSize, true);
  }
 
- // copy image name
- if(pName)
- str_copy(pSound->m_aName, pName);
-
  m_vpSounds.push_back(pSound);
 
  // unload image