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 NodePtrで 必要だと思ったのですpopulateGraphが、リリースするとノードdesctructor（~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です。そしてもちろん、デストラクタが必要なため、コピー コンストラクタとコピー代入演算子も必要です。そうしないと、メモリが破損することが保証されます。

toのelse節も必要です。if (m_name2NodeMap.find(nodeName) == m_name2NodeMap.end())delete NodePtr;

于 2011-04-12T02:05:37.743 に答える
1

他の人はすでにリークの問題をカバーしています。実際、たくさんのリークがあるので、それらすべてについてわざわざコメントすることさえしません... ( populateGraph、~Graph、Graph(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;
};

何が起きてる ？

  • 動的メモリ割り当ては不要で、 by 値mapを完全に含めることができNode、この方法でリークが発生することはありません。
  • std::map::insert同等のキーがまだ存在しない場合にのみ挿入を実行し、find+を実行する必要はありません[](要素を格納する場所を 2 倍計算するため、2 倍複雑になります)
于 2011-04-12T06:41:03.823 に答える
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 に答える