Fixed potential NULL pointer and leak when setting node metadata

diff --git a/src/map.cpp b/src/map.cpp
index 0dbfd42f49b8d486da9293880906af6bdb8631b6..0f9c82c3cc1f00b8ff3d4b602aad91339402cd96 100644 (file)
--- a/src/map.cpp
+++ b/src/map.cpp
@@ -2279,7 +2279,7 @@ void Map::transformLiquids(std::map<v3s16, MapBlock*> & modified_blocks)
        updateLighting(lighting_modified_blocks, modified_blocks);
 }
 
-NodeMetadata* Map::getNodeMetadata(v3s16 p)
+NodeMetadata *Map::getNodeMetadata(v3s16 p)
 {
        v3s16 blockpos = getNodeBlockPos(p);
        v3s16 p_rel = p - blockpos*MAP_BLOCKSIZE;
@@ -2289,8 +2289,7 @@ NodeMetadata* Map::getNodeMetadata(v3s16 p)
                                <<PP(blockpos)<<std::endl;
                block = emergeBlock(blockpos, false);
        }
-       if(!block)
-       {
+       if(!block){
                infostream<<"WARNING: Map::getNodeMetadata(): Block not found"
                                <<std::endl;
                return NULL;
@@ -2299,7 +2298,7 @@ NodeMetadata* Map::getNodeMetadata(v3s16 p)
        return meta;
 }
 
-void Map::setNodeMetadata(v3s16 p, NodeMetadata *meta)
+bool Map::setNodeMetadata(v3s16 p, NodeMetadata *meta)
 {
        v3s16 blockpos = getNodeBlockPos(p);
        v3s16 p_rel = p - blockpos*MAP_BLOCKSIZE;
@@ -2309,13 +2308,13 @@ void Map::setNodeMetadata(v3s16 p, NodeMetadata *meta)
                                <<PP(blockpos)<<std::endl;
                block = emergeBlock(blockpos, false);
        }
-       if(!block)
-       {
+       if(!block){
                infostream<<"WARNING: Map::setNodeMetadata(): Block not found"
                                <<std::endl;
-               return;
+               return false;
        }
        block->m_node_metadata.set(p_rel, meta);
+       return true;
 }
 
 void Map::removeNodeMetadata(v3s16 p)
@@ -2342,8 +2341,7 @@ NodeTimer Map::getNodeTimer(v3s16 p)
                                <<PP(blockpos)<<std::endl;
                block = emergeBlock(blockpos, false);
        }
-       if(!block)
-       {
+       if(!block){
                infostream<<"WARNING: Map::getNodeTimer(): Block not found"
                                <<std::endl;
                return NodeTimer();
@@ -2362,8 +2360,7 @@ void Map::setNodeTimer(v3s16 p, NodeTimer t)
                                <<PP(blockpos)<<std::endl;
                block = emergeBlock(blockpos, false);
        }
-       if(!block)
-       {
+       if(!block){
                infostream<<"WARNING: Map::setNodeTimer(): Block not found"
                                <<std::endl;
                return;

diff --git a/src/map.h b/src/map.h
index a6480c569fb706ed77aeff56b83e68fcb73c21b2..8e55af4377ba55bcc935191b6b880a19d9095d1c 100644 (file)
--- a/src/map.h
+++ b/src/map.h
@@ -307,7 +307,22 @@ class Map /*: public NodeContainer*/
        */
 
        NodeMetadata* getNodeMetadata(v3s16 p);
-       void setNodeMetadata(v3s16 p, NodeMetadata *meta);
+
+       /**
+        * Sets metadata for a node.
+        * This method sets the metadata for a given node.
+        * On success, it returns @c true and the object pointed to
+        * by @p meta is then managed by the system and should
+        * not be deleted by the caller.
+        *
+        * In case of failure, the method returns @c false and the
+        * caller is still responsible for deleting the object!
+        *
+        * @param p node coordinates
+        * @param meta pointer to @c NodeMetadata object
+        * @return @c true on success, false on failure
+        */
+       bool setNodeMetadata(v3s16 p, NodeMetadata *meta);
        void removeNodeMetadata(v3s16 p);
 
        /*

diff --git a/src/rollback_interface.cpp b/src/rollback_interface.cpp
index 70a9e9457fe01e9a684cf4c66136cd8f8910e8df..808b07fed9545a7b5032d54e6898a6ca29e2a6df 100644 (file)
--- a/src/rollback_interface.cpp
+++ b/src/rollback_interface.cpp
@@ -340,7 +340,13 @@ bool RollbackAction::applyRevert(Map *map, InventoryManager *imgr, IGameDef *gam
                                if(n_old.meta != ""){
                                        if(!meta){
                                                meta = new NodeMetadata(gamedef);
-                                               map->setNodeMetadata(p, meta);
+                                               if(!map->setNodeMetadata(p, meta)){
+                                                       delete meta;
+                                                       infostream<<"RollbackAction::applyRevert(): "
+                                                                       <<"setNodeMetadata failed at "
+                                                                       <<PP(p)<<" for "<<n_old.name<<std::endl;
+                                                       return false;
+                                               }
                                        }
                                        std::istringstream is(n_old.meta, std::ios::binary);
                                        meta->deSerialize(is);

diff --git a/src/script/lua_api/l_nodemeta.cpp b/src/script/lua_api/l_nodemeta.cpp
index f9c8794d502d065e6b99cd42499949405e5dda6d..4f20e56f90bd37dc133a0c71bc4bc4c0cf5009b6 100644 (file)
--- a/src/script/lua_api/l_nodemeta.cpp
+++ b/src/script/lua_api/l_nodemeta.cpp
@@ -42,10 +42,12 @@ NodeMetaRef* NodeMetaRef::checkobject(lua_State *L, int narg)
 NodeMetadata* NodeMetaRef::getmeta(NodeMetaRef *ref, bool auto_create)
 {
        NodeMetadata *meta = ref->m_env->getMap().getNodeMetadata(ref->m_p);
-       if(meta == NULL && auto_create)
-       {
+       if(meta == NULL && auto_create) {
                meta = new NodeMetadata(ref->m_env->getGameDef());
-               ref->m_env->getMap().setNodeMetadata(ref->m_p, meta);
+               if(!ref->m_env->getMap().setNodeMetadata(ref->m_p, meta)) {
+                       delete meta;
+                       return NULL;
+               }
        }
        return meta;
 }
@@ -227,17 +229,21 @@ int NodeMetaRef::l_from_table(lua_State *L)
        NodeMetaRef *ref = checkobject(L, 1);
        int base = 2;
 
+       // clear old metadata first
+       ref->m_env->getMap().removeNodeMetadata(ref->m_p);
+
        if(lua_isnil(L, base)){
                // No metadata
-               ref->m_env->getMap().removeNodeMetadata(ref->m_p);
                lua_pushboolean(L, true);
                return 1;
        }
 
-       // Has metadata; clear old one first
-       ref->m_env->getMap().removeNodeMetadata(ref->m_p);
        // Create new metadata
        NodeMetadata *meta = getmeta(ref, true);
+       if(meta == NULL){
+               lua_pushboolean(L, false);
+               return 1;
+       }
        // Set fields
        lua_getfield(L, base, "fields");
        int fieldstable = lua_gettop(L);

diff --git a/src/script/lua_api/l_nodemeta.h b/src/script/lua_api/l_nodemeta.h
index ed06ff0fa56cd531efe1c3ca5f027ead3c049159..e39ac3931ac07d51a19c2446c8a0920d38e4f2e9 100644 (file)
--- a/src/script/lua_api/l_nodemeta.h
+++ b/src/script/lua_api/l_nodemeta.h
@@ -39,6 +39,19 @@ class NodeMetaRef : public ModApiBase {
 
        static NodeMetaRef *checkobject(lua_State *L, int narg);
 
+       /**
+        * Retrieve metadata for a node.
+        * If @p auto_create is set and the specified node has no metadata information
+        * associated with it yet, the method attempts to attach a new metadata object
+        * to the node and returns a pointer to the metadata when successful.
+        *
+        * However, it is NOT guaranteed that the method will return a pointer,
+        * and @c NULL may be returned in case of an error regardless of @p auto_create.
+        *
+        * @param ref specifies the node for which the associated metadata is retrieved.
+        * @param auto_create when true, try to create metadata information for the node if it has none.
+        * @return pointer to a @c NodeMetadata object or @c NULL in case of error.
+        */
        static NodeMetadata* getmeta(NodeMetaRef *ref, bool auto_create);
 
        static void reportMetadataChange(NodeMetaRef *ref);