对象创建期间的资源泄漏 [英] Resource leak during object creation

问题描述

我有以下代码用于在图形中创建节点。当我运行一个静态检查工具（coverity）时，我得到资源泄漏的错误。如果您可以指出如何改进代码，请感谢您：

 类节点{
 public：
显式节点（std :: string& name）：m_name（name）{} 
 void setlevel（int level）
 {m_level = level; } 
 private：
 ... 
} 
 class graph {
 public：
 void populateGraph（）
 {
 std :: string nodeName = getNodeName（）; 
 / *我得到错误说变量从新的未释放或指向函数
 nc :: node :: node（const std :: string ...）* / 
 node * NodePtr = new node（nodeName）; 
 / *我得到错误说变量NodePtr未释放或指向函数
 nc :: node :: setLevel（int）* / 
 NodePtr-> setLevel（-1）; 
 if（m_name2NodeMap.find（nodeName）== m_name2NodeMap.end（））
 m_name2NodeMap [nodeName] = NodePtr; 
 NodePtr = NULL; 
} 
 .... 
 private：
 std :: map< std :: string，node *> m_name2NodeMap; 
} 
  

我以为我需要删除 populateGraph 中的NodePtr ，但是后来释放它将调用节点结构（〜node ）并从图中删除节点。所以，我设置 NodePtr = NULL 看看是否有帮助，但不是。

解决方案

我不熟悉coverity或它使用的确切规则，但是如果节点的名称已经在地图中，你将有一个内存泄漏。也就是说，如果你的if语句的主体没有被执行，那么你松开了你刚刚分配的内存的指针。也许你想要的东西像：

  if（m_name2NodeMap.find（nodeName）== m_name2NodeMap.end（））
 m_name2NodeMap [nodeName] = NodePtr; 
 else 
 delete NodePtr; 
 NodePtr = NULL; 
  

编辑：由于我几乎和Daemin同时回答，让我补充更多细节： p>

正如ildjarn所提到的，你还需要通过添加一个析构函数来释放那些最终在地图中的对象：

 〜graph（）
 {
 for（std :: map< std :: string，node *> :: iterator i = m_name2NodeMap.begin（）; 
i！= m_name2NodeMap.end（）; ++ i）
 {
 delete i-> second; 
} 
} 
  

为了完整性，我应该注意：

1. 地图将在析构函数完成后自动删除，因为它是一个成员变量。




3. 当条目被删除时，字符串键将被删除。

---

处理复杂对象生命周期的首选方法是使用智能指针。例如， boost :: shared_ptr 或tr1 :: shared_ptr会像这样工作。注意：我可能没有准确的语法。

 类节点{
 ... 
} 
 
 class graph {
 public：
 void populateGraph（）
 {
 std :: string nodeName = getNodeName（）; 
 boost :: shared_ptr<节点> NodePtr（new node（nodeName））; 
 NodePtr-> setLevel（-1）; 
 if（m_name2NodeMap.find（nodeName）== m_name2NodeMap.end（））
 m_name2NodeMap [nodeName] = NodePtr; 
} 
 .... 
 private：
 std :: map< std :: string，boost :: shared_ptr< node> > m_name2NodeMap; 
} 
}; 
  

看看我们如何消除析构函数和显式调用来删除？

在另一个节点上，您应该查看 std :: map :: insert 函数，它应该将if语句全部消除。

I have the following code for creation of a node inside a graph. I'm getting resource leak error when I run a static checking tool (coverity). I would appreciate if you can point out how to improve the code:

class node {   
   public :  
     explicit node(std::string& name) : m_name(name) { }  
     void setlevel(int level)  
     { m_level = level; }  
   private :    
     ...  
 }  
class graph {  
   public :  
      void populateGraph()  
      {  
         std::string nodeName = getNodeName();   
         /* I get error saying variable from new not freed or pointed-to in function  
            nc::node::node(const std::string...) */  
         node* NodePtr = new node(nodeName);  
         /* I get error saying variable NodePtr not freed or pointed-to in function  
            nc::node::setLevel(int) */   
         NodePtr->setLevel(-1);  
         if (m_name2NodeMap.find(nodeName) == m_name2NodeMap.end())  
             m_name2NodeMap[nodeName] = NodePtr;  
         NodePtr = NULL;  
      }  
....  
private :  
  std::map< std::string, node*> m_name2NodeMap;   
}

I thought I needed to delete NodePtr in populateGraph, but then released it will call node desctructor (~node) and delete the node from the graph. So, I set NodePtr=NULL to see if it helps, but it is not.

解决方案

I am not familiar with coverity or the exact rules that it uses, but it seems that you will have a memory leak if the name of the node is already in the map. That is, if the body of your if statement is not executed, then you loose the pointer to the memory that you just allocated. Perhaps you wanted something like:

if (m_name2NodeMap.find(nodeName) == m_name2NodeMap.end())  
    m_name2NodeMap[nodeName] = NodePtr;  
else
    delete NodePtr;
NodePtr = NULL; 

Edit: since I responded almost at the same time as Daemin, let me add more details:

As ildjarn mentioned, you also need to deallocate those objects that do end up in the map by adding a destructor:

~graph()
{
    for( std::map< std::string, node*>::iterator i = m_name2NodeMap.begin(); 
         i != m_name2NodeMap.end(); ++i )
    {
        delete i->second;
    }
}

For completeness, I should note that:

1. The map will be deleted automatically after the destructor finishes because it's a member variable.
2. The entries in the node map will be deleted automatically when the map is deleted.
3. The string keys will be deleted when the entries are deleted.

---

The preferred way to deal with complex object lifetimes is to use a smart pointer. For example, the boost::shared_ptr or the tr1::shared_ptr would work like this. Note: I may not have the syntax exact.

class node {   
    ...
}

class graph {  
    public :  
    void populateGraph()  
    {  
        std::string nodeName = getNodeName();   
        boost::shared_ptr< node > NodePtr( new node(nodeName) );
        NodePtr->setLevel(-1);  
        if (m_name2NodeMap.find(nodeName) == m_name2NodeMap.end())  
            m_name2NodeMap[nodeName] = NodePtr;
    }  
    ....  
    private :  
        std::map< std::string, boost::shared_ptr<node> > m_name2NodeMap;   
    }
};

See how we've eliminated the destructor and explicit calls to delete? Now the node objects will be destroyed automatically just like the node names.

On another node, you should look into the std::map::insert function which should eliminate that if statement all together.

这篇关于对象创建期间的资源泄漏的文章就介绍到这了，希望我们推荐的答案对大家有所帮助，也希望大家多多支持IT屋！