Fixed potential NULL pointer and leak when setting node metadata

This commit is contained in:
MetaDucky 2013-11-20 22:11:57 +01:00 committed by kwolekr
parent 747bc40840
commit 5be786c804
5 changed files with 56 additions and 19 deletions

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

@ -307,7 +307,22 @@ public:
*/ */
NodeMetadata* getNodeMetadata(v3s16 p); 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); void removeNodeMetadata(v3s16 p);
/* /*

@ -340,7 +340,13 @@ bool RollbackAction::applyRevert(Map *map, InventoryManager *imgr, IGameDef *gam
if(n_old.meta != ""){ if(n_old.meta != ""){
if(!meta){ if(!meta){
meta = new NodeMetadata(gamedef); 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); std::istringstream is(n_old.meta, std::ios::binary);
meta->deSerialize(is); meta->deSerialize(is);

@ -42,10 +42,12 @@ NodeMetaRef* NodeMetaRef::checkobject(lua_State *L, int narg)
NodeMetadata* NodeMetaRef::getmeta(NodeMetaRef *ref, bool auto_create) NodeMetadata* NodeMetaRef::getmeta(NodeMetaRef *ref, bool auto_create)
{ {
NodeMetadata *meta = ref->m_env->getMap().getNodeMetadata(ref->m_p); 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()); 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; return meta;
} }
@ -227,17 +229,21 @@ int NodeMetaRef::l_from_table(lua_State *L)
NodeMetaRef *ref = checkobject(L, 1); NodeMetaRef *ref = checkobject(L, 1);
int base = 2; int base = 2;
// clear old metadata first
ref->m_env->getMap().removeNodeMetadata(ref->m_p);
if(lua_isnil(L, base)){ if(lua_isnil(L, base)){
// No metadata // No metadata
ref->m_env->getMap().removeNodeMetadata(ref->m_p);
lua_pushboolean(L, true); lua_pushboolean(L, true);
return 1; return 1;
} }
// Has metadata; clear old one first
ref->m_env->getMap().removeNodeMetadata(ref->m_p);
// Create new metadata // Create new metadata
NodeMetadata *meta = getmeta(ref, true); NodeMetadata *meta = getmeta(ref, true);
if(meta == NULL){
lua_pushboolean(L, false);
return 1;
}
// Set fields // Set fields
lua_getfield(L, base, "fields"); lua_getfield(L, base, "fields");
int fieldstable = lua_gettop(L); int fieldstable = lua_gettop(L);

@ -39,6 +39,19 @@ private:
static NodeMetaRef *checkobject(lua_State *L, int narg); 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 NodeMetadata* getmeta(NodeMetaRef *ref, bool auto_create);
static void reportMetadataChange(NodeMetaRef *ref); static void reportMetadataChange(NodeMetaRef *ref);