2016-05-05 5 views
1

文字列をキーとして使用し、カスタムオブジェクトのポインタを値として使用するマップがあります。実行時に、ユーザーはマップに自動的に追加されるこのオブジェクトのインスタンス(ポインタ)を作成できます。 DrMemoryを使用して、リーク(例:メモリリーク)があるかどうかを確認します。終了時に自動的に解放されるポインタを持つC++マップ、削除の処理方法

私はマップを含むポインタを手動で削除せずにアプリケーションをテストしましたが、エラーは発生しませんでした(メモリリークもありません)。安全面を保つために、私はまた、マップを反復して各ポインタを手動で削除するプログラムを終了するときに呼び出されるメソッドでテストを行いました。

私は今それでテストを実行すると、私は同じ種類のいくつかのエラーを取得:解放されたメモリの

アドレス不能アクセスを...

発生のラインがきれいなの場所を指しています。

私の質問は今では:私は、メソッドをコールして(したがって、単にエラーを無視する)を保持する必要がありますまたは私は呼び出しを削除し、手動でポインタを削除する必要はないことが分かっているままにしておく必要があります?

私はポインタを削除し、マップ

void cleanUp(std::map <string, Car*>* map){ 
    for(auto const& x : *map){ 
     delete x.second; 
     map->erase(x.first); 
    } 
} 
+0

マップから値を削除するときは、その値を含むエントリ/キーも消去する必要があります。 – 101010

+0

私はそれをやっています: for(auto const&x:* map){ \t \t delete x.second; \t \t map-> erase(x.first); } – NL3

+1

あなたは何を意味するのかを示す[MCVE]を見せてください。 – BoBTFish

答えて

0

いいえ、それはエラーを無視するのが賢明ではありませんからエントリを消去しています。 DrMemoryを使用してコードに問題があると思われるので、無視してください。あなたのプログラムがうまく動作しているからといって、それがうまくいくというわけではありませコードに未定義の動作がある可能性があります。それは正しく動作するように見えたり、微妙に壊れたりクラッシュしたりすることがあります。そして、あなたのプログラムがあなたのユーザーにリリースされたら、クラッシュすることは間違いありません。

ポインタから無効なポインタを削除するか、スマートポインタを使用する方がよいでしょう。しかし、ポインタ値を含むマップは、アンチパターンのようなものです。これを考慮してください:

map[1] = new Foo(); 
map[1] = new Foo(); // Oops, you're now leaking memory! 

あなたは以前のFoo()をどのように削除しますか?

+0

私はキーの文字列を使用しています。衝突がある場合は、もう1つを入力する必要があります。 – NL3

5

ソースコードを見ることなく伝えるのは難しいですが、どこにでも動的に割り当てられたオブジェクト(ポインタはRAII実装クラスで保護されていません)への裸のポインタは、悪い習慣の兆候です。あなたのループが間違っている

std::map<std::string, std::unique_ptr<std::string>> mm; 
mm["alpha"] = std::make_unique<std::string>("alpha"); 
+0

この点をありがとう、私は今のところそれらを使用します。 – NL3

+2

* "どこでも裸のポインタを保つのは悪い習慣の兆候です" * - いいえ、高レベルのコード(つまり、 'std :: vector'レベルの何か)の裸ポインタは通常最近のC++での悪い習慣。 –

+1

@ChristianHacklこれは、 "裸"で書くことができました。RAIIイディオムを使ってクラスによって管理されていないものを意味しました。 – marcinj

2

:スマートポインタを使用すると、あなたは本当にメモリリークについてworringせずにC++でコーディングすることが可能であり、任意のメモリリーク検出器を使用する必要はありません、unique_ptrを使用するコード例をご覧ください。範囲ベースのforループは、イテレータを使用してマップを反復処理するのと同じですが、現在オンになっている要素を削除すると、イテレータは無効になります。 (++の操作で、次の要素を取得すると、解放されたリンクなどを使用しようとします)。

代わりに書くことができ:

for(auto const& x : *map){ 
    delete x.second; 

map->clear(); 

を、全体のマップを吹き飛ばすために速いが、一度に項目1を削除するのが遅いので、これはおそらく、より効率的です。

NB。マップ内のスマートポインタの使用を検討すると、マップが消去されるとアイテムが自動的に解放されます。

+0

ありがとう、それは実際にエラーを引き起こす行でした。 – NL3

関連する問題