2009-07-06 10 views
3

私は頂点と呼ばれる構造体を持っていて、そこにいくつかのポインタを作成しました。私がしたいことは、それらのポインタをリストに追加することです。下の私のコードは、ポインタをリストに挿入しようとすると、セグメンテーションフォールトを作成します。誰かが何が起こっているか説明できますか?構造体ポインタを保持するSTLリスト

#include <iostream> 
#include <list> 

#define NUM_VERTICES 8 

using namespace std; 

enum { WHITE, GRAY, BLACK }; 

struct vertex 
{ 
    int color; 
    int distance; 
    char parent; 
}; 

int main() 
{ 
    //create the vertices 
    vertex r = {WHITE, NULL, NULL}; 

    //create pointer to the vertex structures 
    vertex *pr = &r; 

    //create a list to hold the vertices 
    list<vertex*> *r_list = new list<vertex*>; 

    list<vertex*>::iterator it; 

    r_list->insert(it, pr); 
} 
+0

main関数の戻り値がありません – rfcoder89

答えて

10

ありますここでいくつかのことが間違っている

まず第一に、あなたは他のと同様に、イテレータを初期化していないと述べている:

list<vertex*>::iterator it = r_list->begin(); 

この操作を行うと、あなたのコードは、罰金になります。しかし、あなたのコードは悪い方法で行われます。

なぜヒープからリストを割り当てていますか?あなたのコードを見てください:あなたはメモリリークを持っています。あなたはどこにでもdelete r_listを呼んでいません。

//create a list to hold the vertices 
list<vertex*> r_list; 

list<vertex*>::iterator it = r_list->begin(); 

r_list.insert(it, pr); 

しかし、いっそのは、単にスタックにそれを行う:(boost::scoped_ptrboost::shared_ptrをあなたがそうでなければC++ 11、ブースト同等物を持っている場合std::unique_ptrstd::shared_ptr)を使用すると、スマートポインタを使用する必要がある理由ですさらに、イテレーターを使って挿入することは、物事を遠くに進めています。ただ、push front()またはpush back()を使用します。

//create a list to hold the vertices 
list<vertex*> r_list; 

r_list.push_back(pr); 

もう一つ:あなたのリストは、あなたが構築してきた頂点をoutlives場合、それは無効なものを指します。例えば

// global 
list<vertex*> r_list; 

void some_function(void) 
{ 
    //create the vertices 
    vertex r = {WHITE, NULL, NULL}; 

    //create pointer to the vertex structures 
    vertex *pr = &r; 

    r_list.push_back(pr); 
} // right here, vertex r stops existing: the list now contains an 
    // invalid pointer. 

一つの解決策は、ヒープ割り当てられた頂点へのポインタを格納することです:

// global 
list<vertex*> r_list; 

void some_function(void) 
{ 
    //create the vertices 
    vertex *r = new vertex; 
    r->color = WHITE; 
    r->distance = 0; 
    r->parent = 0; 

    r_list.push_back(r); 
} 

は今も、関数の後にリストが有効なヒープ割り当てられた頂点を指しています。これで、リストの使用が終了したら、lsitを実行して各要素のdeleteに電話する必要があるという問題があります。この問題はBoost Pointer Container Libraryを使用することによって助けられます。

最良の方法は、しかし、ちょうど店が自分(というよりも、それらへのポインタ)を頂点にある:

//create a list to hold the vertices 
list<vertex> r_list; 

//create the vertices 
vertex r = {WHITE, NULL, NULL}; 

r_list.push_back(r); 

あなたはコンストラクタ頂点与えれば、あなたもちょうどその場でそれらを構築することができます。

struct vertex 
{ 
    int color; 
    int distance; 
    char parent; 

    vertex(int _color, int _distance, char _parent) : 
    color(_color), 
    distance(_distance), 
    parent(_parent) 
    { 
    } 
}; 

//create a list to hold the vertices 
list<vertex> r_list; 

r_list.push_back(vertex(WHITE, NULL, NULL)); 

(これらは問題外なりました)ポインタを扱うときにまず、NULLが一般的にのみ使用され

。 、第二に

//create the vertices 
vertex r = {WHITE, 0, 0}; 

constantsむしろ#defineよりも使用します:distanceparentはないポインタであるため、NULLではなく、それらを初期化するために0を使用

#define NUM_VERTICES 8 // <- bad 
const int NumberVertices = 8; // <- good 

最後には、あなたの列挙型の名前を与える、または場所それは名前空間内にあります:

enum Color { WHITE, GRAY, BLACK }; 

これらの助けが欲しいです!

+1

すべての細かい点の良いコンパイル。私はあなたがポインタの寿命とスタックとヒープに割り当てることの違いに重点を置いてうれしいです。 – Tom

1

あなたはitを初期化していないので、あなたは/ポインタランダム/初期化されていない場所で挿入しています。

std::listにアイテムを追加する通常の方法は、そのメソッドpush_backpush_frontを含みます。以前に他のアイテムを挿入したい特定のスポットを以前に決めていた場合にのみ、通常はinsertを使用します。

2

まず、itを何かに初期化していません。あなたはどういう意味ですか:

list<vertex*>::iterator it = r_list->begin(); 

また、intとcharをNULLに初期化するのはなぜですか?通常、人々はポインタにNULLを使用します。

また、列挙型に名前を付けて、列挙型の型の安全性から利益を得るのはどうですか?

また、頂点へのポインタを作るために新しい変数を作成する必要はありません。あなたが挿入を呼び出すと、&rを渡すことができます。

また、Peterが指摘するように、なぜpush_back()を使用しないのですか?

あなたのコードはより次のようになります。

あなたはイテレータを初期化していないので、それはして挿入し、有効ではありません

using namespace std; 

enum Color { 
    WHITE, 
    GRAY, 
    BLACK 
}; 

struct vertex 
{ 
    Color color; 
    int distance; 
    char parent; 
}; 

int main(int argc, char** argv) { 
    //create the vertices 
    vertex r = {WHITE, 0, ''}; 

    //create a list to hold the vertices 
    list* r_list = new list(); 

    list::iterator it = r_list->begin(); 
    r_list->insert(it, &r); 

    // Or even better, use push_back (or front) 
    r_list->push_back(&r); 
} 
2

。たとえば、代わりにr_list->push_back(pr)を使用できます。

また、rが範囲外になると、リスト内のポインタは有効になりません。明らかに、それはmain()にあるので、このケースでは問題ではありませんが、コードを使用する正確な例ではないと想定しています。

+0

push_back()を使用しようとしたときにmingwコンパイラがこのエラーを出しました エラー: 'std :: list :: :: push_back(頂点&) '| しかし、g ++でコンパイルしたときにエラーが出ることはありませんでした。とにかく、この話題は別のスレッドに属していると思います。 – unknown

+0

このコンパイラエラーは、頂点*の代わりに頂点をpush_backメソッドに渡そうとしたようです。しかし、私は間違っている可能性があります... – Tom

+0

それは正しいトムです。ありがとう。 私はエラーメッセージを読むことでより良くなる必要があります – unknown

関連する問題