0

我有以下代码用于在图中创建节点。运行静态检查工具（覆盖率）时出现资源泄漏错误。如果您能指出如何改进代码，我将不胜感激：

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;   
}


我以为我需要delete NodePtrin populateGraph，但随后发布它会调用节点析构函数 ( ~node) 并从图中删除节点。所以，我开始NodePtr=NULL看看它是否有帮助，但事实并非如此。

4

4 回答 4

3

我不熟悉覆盖率或它使用的确切规则，但如果节点的名称已经在地图中，您似乎会有内存泄漏。也就是说，如果你的 if 语句的主体没有被执行，那么你就会失去指向你刚刚分配的内存的指针。也许你想要类似的东西：

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

编辑：由于我几乎与 Daemin 同时回复，让我添加更多细节：

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

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

为了完整起见，我应该指出：

  1. 析构函数完成后映射将被自动删除，因为它是一个成员变量。
  2. 节点映射中的条目会在映射被删除时被自动删除。
  3. 删除条目时将删除字符串键。

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

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;   
    }
};

看看我们如何消除析构函数和对删除的显式调用？现在节点对象将像节点名称一样自动销毁。

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

于 2011-04-12T02:06:09.893 回答
2

你需要做的是给graph一个析构函数，在它里面，delete所有的node*s in m_name2NodeMap。当然，因为需要析构函数，所以还需要复制构造函数和复制赋值运算符，否则肯定会导致内存损坏。

您还需要一个toelse子句。if (m_name2NodeMap.find(nodeName) == m_name2NodeMap.end())delete NodePtr;

于 2011-04-12T02:05:37.743 回答
1

NodePtr当您不将其添加到列表时，您并没有释放它。该if语句需要一个 else 你在哪里delete NodePtr;

if (m_name2NodeMap.find(nodeName) == m_name2NodeMap.end())
{
    m_name2NodeMap[nodeName] = NodePtr;
}
else
{
    delete NodePtr;
}
NodePtr = NULL;
于 2011-04-12T02:05:48.417 回答
1

其他人已经涵盖了泄漏的问题。事实上有很多泄漏，所以我什至不会费心评论它们......（，，，至少populateGraph...... ）~GraphGraph(Graph const&)Graph& operator=(Graph const&)

我更喜欢提供一个有效的简单解决方案：

class Graph
{
public:
  void addNode(std::string name) {
    _nodes.insert(std::make_pair(name, Node(name));
  }

private:
  std::map<std::string, Node> _nodes;
};

这里发生了什么 ？

  • 动态内存分配是不必要的，map可以完美地包含Node按值，这样你就不会有任何泄漏。
  • std::map::insert只有在没有等效键存在的情况下才会执行插入，不需要执行find+ []（这是复杂的两倍，因为您计算了存储元素的位置的两倍）
于 2011-04-12T06:41:03.823 回答