Fix destructors causing double free

rozukke · Aug 12, 2024 · 85f7e6c · 85f7e6c
1 parent 667ff6e
commit 85f7e6c
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 4 deletions.
diff --git a/include/mcpp/util.h b/include/mcpp/util.h
@@ -100,6 +100,8 @@ struct Chunk {
 
     ~Chunk();
 
+    Chunk& operator=(const Chunk& other) noexcept;
+
     /**
      * Accesses the Minecraft block at absolute position pos and returns its
      * BlockType if it is in the included area.
@@ -145,10 +147,10 @@ struct Chunk {
 
   private:
     Coordinate _base_pt;
-    BlockType* raw_data;
     int _y_len;
     int _x_len;
     int _z_len;
+    BlockType* raw_data;
 };
 
 /**
@@ -159,6 +161,10 @@ struct HeightMap {
     HeightMap(const Coordinate& loc1, const Coordinate& loc2,
               const std::vector<int>& heights);
 
+    ~HeightMap();
+
+    HeightMap& operator=(const HeightMap& other) noexcept;
+
     /**
      * Get the height using an offset from the origin/base point of the heights
      * area
@@ -201,9 +207,9 @@ struct HeightMap {
     Coordinate base_pt() const;
 
   private:
+    Coordinate _base_pt;
     int _x_len;
     int _z_len;
-    Coordinate _base_pt;
     int* raw_heights;
 };
 

diff --git a/src/util.cpp b/src/util.cpp
@@ -73,6 +73,23 @@ Chunk::Chunk(const Coordinate& loc1, const Coordinate& loc2,
 
 Chunk::~Chunk() { delete[] raw_data; }
 
+Chunk& Chunk::operator=(const Chunk& other) noexcept {
+    if (this != &other) {
+        // Clean up existing resource
+        delete[] raw_data;
+
+        // Copy data from the other object
+        _base_pt = other._base_pt.clone();
+        _x_len = other._x_len;
+        _y_len = other._y_len;
+        _z_len = other._z_len;
+        raw_data = new BlockType[_x_len * _y_len * _z_len];
+        std::copy(other.raw_data, other.raw_data + _x_len * _y_len * _z_len,
+                  raw_data);
+    }
+    return *this;
+}
+
 BlockType Chunk::get(int x, int y, int z) {
     if ((x < 0 || y < 0 || z < 0) ||
         (x > _x_len - 1 || y > _y_len - 1 || z > _z_len - 1)) {
@@ -118,6 +135,26 @@ HeightMap::HeightMap(const Coordinate& loc1, const Coordinate& loc2,
     std::copy(heights.begin(), heights.end(), raw_heights);
 }
 
+HeightMap::~HeightMap() { delete[] raw_heights; }
+
+HeightMap& HeightMap::operator=(const HeightMap& other) noexcept {
+    if (this != &other) {
+        // Free the existing resource
+        delete[] raw_heights;
+
+        // Copy data from the other object
+        _base_pt = other._base_pt.clone();
+        _x_len = other._x_len;
+        _z_len = other._z_len;
+
+        // Allocate memory and copy the heights
+        raw_heights = new int[_x_len * _z_len];
+        std::copy(other.raw_heights, other.raw_heights + _x_len * _z_len,
+                  raw_heights);
+    }
+    return *this;
+}
+
 int HeightMap::get(int x, int z) const {
     if ((x < 0 || x >= _x_len) || (z < 0 || z >= _z_len)) {
         throw new std::out_of_range(

diff --git a/test/minecraft_tests.cpp b/test/minecraft_tests.cpp
@@ -143,14 +143,16 @@ TEST_CASE("getBlocks and Chunk operations") {
     SUBCASE("getters") {
         Chunk data = mc.getBlocks(loc1, loc2);
 
-        CHECK_EQ(data.base_pt(), Coordinate{100, 100, 100});
+        CHECK_EQ(data.base_pt(), loc1);
         CHECK_EQ(data.x_len(), 11);
+        CHECK_EQ(data.y_len(), 11);
         CHECK_EQ(data.z_len(), 11);
 
         data = mc.getBlocks(loc2, loc1);
 
-        CHECK_EQ(data.base_pt(), Coordinate{100, 100, 100});
+        CHECK_EQ(data.base_pt(), loc1);
         CHECK_EQ(data.x_len(), 11);
+        CHECK_EQ(data.y_len(), 11);
         CHECK_EQ(data.z_len(), 11);
     }