2012-04-16 24 views
2

C++の基本的なメモリ管理の原則を理解するのに問題があります。このコードは、迷路ファイルを2Dベクトルに読み込む関数の一部であるループの一部です。メモリ管理Confusion C++

次のコードは、メモリリークを引き起こしているValgrindのによれば、... tMazeNodeオブジェクトとvertsあること

注意を(とオブジェクトノードへのポインタを保持するtクラス内のベクター混同されるべきではありませんMazeNodeオブジェクト):

node* top = new node(TOP, rowCount, i, t.type); 
node* bot = new node(BOTTOM, rowCount, i, t.type); 
node* left = new node(LEFT, rowCount, i, t.type); 
node* right = new node(RIGHT, rowCount, i, t.type); 

t.verts.push_back(top); 
t.verts.push_back(bot); 
t.verts.push_back(left); 
t.verts.push_back(right); 

temp.push_back(t); 

top = NULL; 
bot = NULL; 
left = NULL; 
right = NULL; 
delete top; 
delete bot; 
delete left; 
delete right; 

当初、私はそれらを削除する前に、NULLへのポインタのそれぞれに設定されていませんでしたが、割り当てエラーになるだろう。だから私はNULLに設定し、私のコードは動作します。私はなぜこれがメモリリークを引き起こすのか、なぜポインタをNULLに設定する必要があるのか​​、本当に混乱していると思います。これを行うには、おそらく簡単な非ポインタ方法がありますが、おそらくこの問題はメモリ管理をよりよく理解するのに役立ちます。

ありがとうございました。

編集:ここではMazeNodeクラスは

class MazeNode 
{ 
public: 
    void setType(char c); 
    char getChar(); 

    NodeType type; 
    vector<Direction> visitedFrom; 

    vector<node*> verts; 
}; 

そして、ノードクラス

(また、構造体のようなeverythign公衆を作り、このクラスを書くことで私のlazynessを言い訳)(「T」が何であるかである)です。

class node 
{ 
public: 
    node(); 
    node(Direction d, int r, int c, NodeType t); 
    ~node(); //empty definition 
    node(const node* n); 
    node& operator=(const node& n); 

    void addAdj(node* a, int w); 
    void printAdj() const; 
    string direction() const; 

    void print() const; 
    bool operator<(const node& n) const; 


    int distance; //from start 
    bool visited; 
    node* prev; 
    vector<Edge> adj; 
    Direction dir; 
    int row, col; 
    NodeType type; 
}; 

EDIT2:ありがとうございました。私は今問題を理解している。ポインタオブジェクトのベクトルを変更して、ポインタをもう使用していないようにしました。

+0

ポインタをNULLに設定する必要はありません。それがまさにリークの原因となっているようです。 –

+0

t.vertsの定義を追加してください。 – ebo

+0

それは私が思ったものですが、なぜ私はちょうど削除を使用するとエラーが発生するのですか? – Slims

答えて

9

前ヌル割り当てを追加することに、あなたのコード私は物事のこれらの種類のマクロを作成する理由

+0

"本当の解決策は、あなたがそれらの上で削除を呼び出した後にどこでもポインタを保持することではありません。 ....まあ、依存しています。シンプルなシナリオではそれは良い経験則かもしれませんが、複雑なものについては、再利用するポインタの配列などがあります。あなたは絶対的でないところで絶対的でないようにあなたの言葉を少し寛大にしたいかもしれません。 – Kaganar

+0

申し訳ありませんが、私はこれで新しいことですが、どのようにしてポインタを戻すことなくポインタを使用することができますか?私はちょうどポインタオブジェクトのベクトルを使用しないでください? – Slims

+0

@Kaganar - これは確かに単純化されていますが、SIGABORTを最初に見ている間は実用的なものです。 –

2

削除する前に値をNULLに設定しているため、NULLを削除しようとしていて、何も削除されていません。削除呼び出しをNULL呼び出しの上に移動してみてください。

0

変更するには:

delete top; 
delete bot; 
delete left; 
delete right; 

top = NULL; 
bot = NULL; 
left = NULL; 
right = NULL; 

そして、それは動作するはずです。

#define delobj(obj) (delete obj, obj = NULL) 

そして、あなたはこのようにそれを使用します:

delobj(top); 
delobj(bot); 
delobj(left); 
delobj(right); 
2

この混乱は、正確ですメモリリークとは異なる(悪い)問題がありました。おそらくは、迷惑ポインタ、つまり割り当てられていないメモリを指すポインタを格納しています。

ヌル割り当てを追加してメモリリークを行うと、それは改善されますが、あまり効果はありません。

本当の解決策は、削除を呼び出した後にどこでもポインタを保持しないことです。 つまり、push_backを入力しないか、deleteを入力しないでください。

4

ポインタをコンテナに配置してから、ポインタを削除します。あなたのコードが後でそれらのポインタを使用しようとすると、それらは無効であり、クラッシュを引き起こします。

ポインタを削除する前にNULLに設定すると、ポインタはまったく削除されません。NULLポインタを削除しても何も行われません。しかし、今は後でオブジェクトを削除するものは何もなく、メモリリークが発生します。

コード内でポインターを使用していない部分を見つけて削除する必要があります。

編集:もちろん、std::shared_ptrのようなスマートなポインタは、オブジェクトを自動的に削除するので、この問題を完全に排除すると述べておきます。

2

エラーはポインタのベクトルを使用しています。

vector<node*> verts; 

しかし、何がそうあるべきことはこれです:あなたによると、Vertsにはこれです)あなたが(を一back最初のケースでは、

vector<node> verts; 

ポインタ、それはOKですが、あなたはpop_backときそうでなければベクトルのサイズを変更すると、ポインタはベクトルの「内容」であり、割り当てられていないが、ポインタが何を指しているかはノードではない。したがって、ノードはリークします。しかし、第2の場合、ノードはベクトルの「一部」であり、ベクトルのサイズ変更の一部として割り当て/割り当て解除されます。

あなたのパターンは、おそらくJava/C#の背景を示していると思います。コンテナに「new-ing」するのは非常に一般的ですが、C++で行うには、スマートポインタのコンテナ(vector<shared_ptr<node>>や何か)、おそらく質問の範囲を超えています。しかし、これらの言語では、参照型への参照はすべて「スマートポインタ」(多かれ少なかれ)であるため、これは自動的に行われます。 C++はそうではありません。

vector<node>を使用するようにコードを変更する必要があります(また、その方法を変更する)か、ベクターが縮小したときにノードを明示的に割り当て解除する必要があります。

+0

私は参照してください。さて、次にベクトルが縮小します。これはプログラム全体で使用されます。それはただの迷路を保持する構造体です。それは縮小してはいけません。 – Slims