fix: add missing memory cleanups for result classes and structs

isaac-mason · Dec 8, 2024 · 475323b · 475323b
1 parent 1ff8471
commit 475323b
Show file tree

Hide file tree

Showing 7 changed files with 74 additions and 105 deletions.
diff --git a/.changeset/rare-guests-camp.md b/.changeset/rare-guests-camp.md
@@ -0,0 +1,6 @@
+---
+'@recast-navigation/wasm': patch
+'recast-navigation': patch
+---
+
+fix: add missing memory cleanups for result classes and structs
diff --git a/packages/recast-navigation-wasm/src/Detour.cpp b/packages/recast-navigation-wasm/src/Detour.cpp
@@ -64,8 +64,8 @@ CreateNavMeshDataResult *DetourNavMeshBuilder::createNavMeshData(dtNavMeshCreate
     if (!dtCreateNavMeshData(&params, &navMeshData->data, &navMeshData->size))
     {
         result->success = false;
-        navMeshData->data = 0;
-        navMeshData->size = 0;
+        delete navMeshData;
+        result->navMeshData = nullptr;
     }
     else
     {

diff --git a/packages/recast-navigation-wasm/src/NavMesh.cpp b/packages/recast-navigation-wasm/src/NavMesh.cpp
@@ -21,20 +21,20 @@ dtStatus NavMesh::addTile(UnsignedCharArray *navMeshData, int flags, dtTileRef l
 
 NavMeshRemoveTileResult NavMesh::removeTile(dtTileRef ref)
 {
-    NavMeshRemoveTileResult *result = new NavMeshRemoveTileResult;
+    NavMeshRemoveTileResult result;
 
-    result->status = m_navMesh->removeTile(ref, &result->data, &result->dataSize);
+    result.status = m_navMesh->removeTile(ref, &result.data, &result.dataSize);
 
-    return *result;
+    return result;
 }
 
 NavMeshCalcTileLocResult NavMesh::calcTileLoc(const float *pos) const
 {
-    NavMeshCalcTileLocResult *result = new NavMeshCalcTileLocResult;
+    NavMeshCalcTileLocResult result;
 
-    m_navMesh->calcTileLoc(pos, &result->tileX, &result->tileY);
+    m_navMesh->calcTileLoc(pos, &result.tileX, &result.tileY);
 
-    return *result;
+    return result;
 }
 
 void NavMesh::decodePolyId(dtPolyRef ref, UnsignedIntRef *salt, UnsignedIntRef *it, UnsignedIntRef *ip) {
@@ -52,14 +52,14 @@ const dtMeshTile *NavMesh::getTileAt(const int x, const int y, const int tlayer)
 
 NavMeshGetTilesAtResult NavMesh::getTilesAt(const int x, const int y, const int maxTiles) const
 {
-    NavMeshGetTilesAtResult *result = new NavMeshGetTilesAtResult;
+    NavMeshGetTilesAtResult result;
 
     const dtMeshTile *tiles[maxTiles];
 
-    result->tileCount = m_navMesh->getTilesAt(x, y, tiles, maxTiles);
-    result->tiles = *tiles;
+    result.tileCount = m_navMesh->getTilesAt(x, y, tiles, maxTiles);
+    result.tiles = *tiles;
 
-    return *result;
+    return result;
 }
 
 dtTileRef NavMesh::getTileRefAt(int x, int y, int layer) const
@@ -84,32 +84,32 @@ int NavMesh::getMaxTiles() const
 
 NavMeshGetTileAndPolyByRefResult NavMesh::getTileAndPolyByRef(dtPolyRef ref) const
 {
-    NavMeshGetTileAndPolyByRefResult *result = new NavMeshGetTileAndPolyByRefResult;
+    NavMeshGetTileAndPolyByRefResult result;
 
     const dtMeshTile *tile;
     const dtPoly *poly;
 
-    result->status = m_navMesh->getTileAndPolyByRef(ref, &tile, &poly);
+    result.status = m_navMesh->getTileAndPolyByRef(ref, &tile, &poly);
 
-    result->tile = tile;
-    result->poly = poly;
+    result.tile = tile;
+    result.poly = poly;
 
-    return *result;
+    return result;
 }
 
 NavMeshGetTileAndPolyByRefResult NavMesh::getTileAndPolyByRefUnsafe(dtPolyRef ref) const
 {
-    NavMeshGetTileAndPolyByRefResult *result = new NavMeshGetTileAndPolyByRefResult;
+    NavMeshGetTileAndPolyByRefResult result;
 
     const dtMeshTile *tile;
     const dtPoly *poly;
 
     m_navMesh->getTileAndPolyByRefUnsafe(ref, &tile, &poly);
 
-    result->tile = tile;
-    result->poly = poly;
+    result.tile = tile;
+    result.poly = poly;
 
-    return *result;
+    return result;
 }
 
 bool NavMesh::isValidPolyRef(dtPolyRef ref) const
@@ -159,12 +159,12 @@ int NavMesh::getTileStateSize(const dtMeshTile *tile) const
 
 NavMeshStoreTileStateResult NavMesh::storeTileState(const dtMeshTile *tile, const int maxDataSize) const
 {
-    NavMeshStoreTileStateResult *result = new NavMeshStoreTileStateResult;
+    NavMeshStoreTileStateResult result;
 
-    result->status = m_navMesh->storeTileState(tile, result->data, maxDataSize);
-    result->dataSize = maxDataSize;
+    result.status = m_navMesh->storeTileState(tile, result.data, maxDataSize);
+    result.dataSize = maxDataSize;
 
-    return *result;
+    return result;
 }
 
 dtStatus NavMesh::restoreTileState(dtMeshTile *tile, const unsigned char *data, const int maxDataSize)

diff --git a/packages/recast-navigation-wasm/src/NavMeshQuery.cpp b/packages/recast-navigation-wasm/src/NavMeshQuery.cpp
@@ -24,6 +24,8 @@ dtStatus NavMeshQuery::findPath(dtPolyRef startRef, dtPolyRef endRef, const floa
     dtStatus status = m_navQuery->findPath(startRef, endRef, startPos, endPos, filter, pathArray, &pathCount, maxPath);
 
     path->copy(pathArray, pathCount);
+    delete[] pathArray;
+
     return status;
 }
 
@@ -109,6 +111,7 @@ dtStatus NavMeshQuery::moveAlongSurface(dtPolyRef startRef, const float *startPo
     dtStatus status = m_navQuery->moveAlongSurface(startRef, startPos, endPos, filter, &resultPos->x, visitedArray, &size, maxVisitedSize);
 
     visited->copy(visitedArray, size);
+    delete[] visitedArray;
 
     return status;
 }

diff --git a/packages/recast-navigation-wasm/src/NavMeshSerdes.cpp b/packages/recast-navigation-wasm/src/NavMeshSerdes.cpp
@@ -37,8 +37,8 @@ struct NavMeshTileHeader
 
 NavMeshImporterResult NavMeshImporter::importNavMesh(NavMeshExport *navMeshExport, TileCacheMeshProcessJsImpl &meshProcess)
 {
-    NavMeshImporterResult *result = new NavMeshImporterResult;
-    result->success = false;
+    NavMeshImporterResult result;
+    result.success = false;
 
     unsigned char *bits = (unsigned char *)navMeshExport->dataPointer;
 
@@ -51,19 +51,19 @@ NavMeshImporterResult NavMeshImporter::importNavMesh(NavMeshExport *navMeshExpor
     if (recastHeader.magic == NAVMESHSET_MAGIC)
     {
         NavMeshSetHeader header;
-        size_t readLen = sizeof(NavMeshSetHeader);
+        readLen = sizeof(NavMeshSetHeader);
         memcpy(&header, bits, readLen);
         bits += readLen;
 
         if (recastHeader.version != NAVMESHSET_VERSION)
         {
-            return *result;
+            return result;
         }
 
         NavMesh *navMesh = new NavMesh;
         if (!navMesh->initTiled(&header.params))
         {
-            return *result;
+            return result;
         }
 
         // Read tiles.
@@ -96,24 +96,24 @@ NavMeshImporterResult NavMeshImporter::importNavMesh(NavMeshExport *navMeshExpor
             navMesh->addTile(navMeshData, DT_TILE_FREE_DATA, tileHeader.tileRef, nullptr);
         }
 
-        result->navMesh = navMesh;
+        result.navMesh = navMesh;
     }
     else if (recastHeader.magic == TILECACHESET_MAGIC)
     {
         if (recastHeader.version != TILECACHESET_VERSION)
         {
-            return *result;
+            return result;
         }
 
         TileCacheSetHeader header;
-        size_t readLen = sizeof(TileCacheSetHeader);
+        readLen = sizeof(TileCacheSetHeader);
         memcpy(&header, bits, readLen);
         bits += readLen;
 
         NavMesh *navMesh = new NavMesh;
         if (!navMesh->initTiled(&header.meshParams))
         {
-            return *result;
+            return result;
         }
 
         RecastLinearAllocator *allocator = new RecastLinearAllocator(32000);
@@ -122,14 +122,14 @@ NavMeshImporterResult NavMeshImporter::importNavMesh(NavMeshExport *navMeshExpor
         TileCache *tileCache = new TileCache;
         if (!tileCache->init(&header.cacheParams, allocator, compressor, meshProcess))
         {
-            return *result;
+            return result;
         }
 
         // Read tiles.
         for (int i = 0; i < recastHeader.numTiles; ++i)
         {
             TileCacheTileHeader tileHeader;
-            size_t readLen = sizeof(tileHeader);
+            readLen = sizeof(tileHeader);
             memcpy(&tileHeader, bits, readLen);
             bits += readLen;
 
@@ -154,26 +154,26 @@ NavMeshImporterResult NavMeshImporter::importNavMesh(NavMeshExport *navMeshExpor
             tileCacheData->data = data;
             tileCacheData->size = tileHeader.dataSize;
 
-            TileCacheAddTileResult result = tileCache->addTile(tileCacheData, DT_COMPRESSEDTILE_FREE_DATA);
-            if (dtStatusFailed(result.status))
+            TileCacheAddTileResult addTileResult = tileCache->addTile(tileCacheData, DT_COMPRESSEDTILE_FREE_DATA);
+            if (dtStatusFailed(addTileResult.status))
             {
                 dtFree(data);
             }
 
-            if (result.tileRef)
+            if (addTileResult.tileRef)
             {
-                tileCache->buildNavMeshTile(&result.tileRef, navMesh);
+                tileCache->buildNavMeshTile(&addTileResult.tileRef, navMesh);
             }
         }
 
-        result->navMesh = navMesh;
-        result->tileCache = tileCache;
-        result->allocator = allocator;
-        result->compressor = compressor;
+        result.navMesh = navMesh;
+        result.tileCache = tileCache;
+        result.allocator = allocator;
+        result.compressor = compressor;
     }
 
-    result->success = true;
-    return *result;
+    result.success = true;
+    return result;
 }
 
 NavMeshExport NavMeshExporter::exportNavMesh(NavMesh *navMesh, TileCache *tileCache) const

diff --git a/packages/recast-navigation-wasm/src/Refs.h b/packages/recast-navigation-wasm/src/Refs.h
@@ -52,43 +52,3 @@ struct FloatRef
     FloatRef() {}
     ~FloatRef() {}
 };
-
-
-// template <typename T>
-// class PrimitiveRefTemplate
-// {
-// public:
-//     T value;
-//     PrimitiveRefTemplate() {}
-//     virtual ~PrimitiveRefTemplate() {}
-// };
-
-// class BoolRef : public PrimitiveRefTemplate<bool>
-// {
-//     BoolRef() : PrimitiveRefTemplate() {}
-// };
-
-// class IntRef : public PrimitiveRefTemplate<int>
-// {
-//     IntRef() : PrimitiveRefTemplate() {}
-// };
-
-// class UnsignedIntRef : public PrimitiveRefTemplate<unsigned int>
-// {
-//     UnsignedIntRef() : PrimitiveRefTemplate() {}
-// };
-
-// class UnsignedCharRef : public PrimitiveRefTemplate<unsigned char>
-// {
-//     UnsignedCharRef() : PrimitiveRefTemplate() {}
-// };
-
-// class UnsignedShortRef : public PrimitiveRefTemplate<unsigned short>
-// {
-//     UnsignedShortRef() : PrimitiveRefTemplate() {}
-// };
-
-// class FloatRef : public PrimitiveRefTemplate<float>
-// {
-//     FloatRef() : PrimitiveRefTemplate() {}
-// };
diff --git a/packages/recast-navigation-wasm/src/TileCache.cpp b/packages/recast-navigation-wasm/src/TileCache.cpp
@@ -24,11 +24,11 @@ bool TileCache::init(const dtTileCacheParams *params, RecastLinearAllocator *all
 
 TileCacheAddTileResult TileCache::addTile(UnsignedCharArray *tileCacheData, unsigned char flags)
 {
-    TileCacheAddTileResult *result = new TileCacheAddTileResult;
+    TileCacheAddTileResult result;
 
-    result->status = m_tileCache->addTile(tileCacheData->data, tileCacheData->size, flags, &result->tileRef);
+    result.status = m_tileCache->addTile(tileCacheData->data, tileCacheData->size, flags, &result.tileRef);
 
-    return *result;
+    return result;
 }
 
 dtStatus TileCache::buildNavMeshTile(const dtCompressedTileRef *ref, NavMesh *navMesh)
@@ -43,53 +43,53 @@ dtStatus TileCache::buildNavMeshTilesAt(const int tx, const int ty, NavMesh *nav
 
 TileCacheUpdateResult TileCache::update(NavMesh *navMesh)
 {
-    TileCacheUpdateResult *result = new TileCacheUpdateResult;
+    TileCacheUpdateResult result;
 
-    result->status = m_tileCache->update(0, navMesh->getNavMesh(), &result->upToDate);
+    result.status = m_tileCache->update(0, navMesh->getNavMesh(), &result.upToDate);
 
-    return *result;
-};
+    return result;
+}
 
 TileCacheAddObstacleResult TileCache::addCylinderObstacle(const Vec3 &position, float radius, float height)
 {
     dtObstacleRef ref(-1);
 
-    TileCacheAddObstacleResult *result = new TileCacheAddObstacleResult;
+    TileCacheAddObstacleResult result;
 
     if (!m_tileCache)
     {
-        result->status = DT_FAILURE;
-        return *result;
+        result.status = DT_FAILURE;
+        return result;
     }
 
-    result->status = m_tileCache->addObstacle(&position.x, radius, height, &ref);
+    result.status = m_tileCache->addObstacle(&position.x, radius, height, &ref);
 
     m_obstacles.push_back(ref);
 
-    result->ref = &m_obstacles.back();
+    result.ref = &m_obstacles.back();
 
-    return *result;
+    return result;
 }
 
 TileCacheAddObstacleResult TileCache::addBoxObstacle(const Vec3 &position, const Vec3 &extent, float angle)
 {
     dtObstacleRef ref(-1);
 
-    TileCacheAddObstacleResult *result = new TileCacheAddObstacleResult;
+    TileCacheAddObstacleResult result;
 
     if (!m_tileCache)
     {
-        result->status = DT_FAILURE; 
-        return *result;
+        result.status = DT_FAILURE; 
+        return result;
     }
 
-    result->status = m_tileCache->addBoxObstacle(&position.x, &extent.x, angle, &ref);
+    result.status = m_tileCache->addBoxObstacle(&position.x, &extent.x, angle, &ref);
 
     m_obstacles.push_back(ref);
 
-    result->ref = &m_obstacles.back();
+    result.ref = &m_obstacles.back();
 
-    return *result;
+    return result;
 }
 
 dtStatus TileCache::removeObstacle(dtObstacleRef *obstacle)