デストラクタでポインタを削除するとクラッシュする

Question

私はC++でのポインタの深い理解が不足しているのに対して、 （私はROOTプロットパッケージを使用していますが、私はこれがちょうどだと思う

#include "Skymap.h" 

Skymap::Skymap() 
{ 
    mCanvas.SetCanvasSize(1200,800); 
    mMarkerType=1; 
} 

Skymap::~Skymap() 
{ 
    delete mSkymapBorderBox; 
} 

void Skymap::DrawAitoffSkymap() 
{ 
    TBox* mSkymapBorderBox=new TBox(-200,-100,200,100); 
    //Use the mSkymapBorderBox pointer for a while 
} 

：として定義された関数で

class Skymap { 
public: 
    Skymap(); 
    ~Skymap(); 
    void DrawAitoffSkymap(); 
private: 
    TCanvas mCanvas; 

    TBox* mSkymapBorderBox; 
}; 

：私は、次の定義が含まれSkymapと呼ばれるクラスを、書きました一般的なC + +の質問）。

さて、次のプログラムは、skymap2のデストラクタに到達するとクラッシュします：

int main(){ 
    Skymap skymap1; 
    Skymap skymap2; 
    skymap1.DrawAitoffSkymap(); 
    skymap2.DrawAitoffSkymap(); 
    return(0); 
} 

ただし、以下はクラッシュしません：

また

int main(){ 
    Skymap skymap1; 
    skymap1.DrawAitoffSkymap(); 
    return(0); 
} 

、私はNULLポインタのmSkymapBorderBoxを初期化する場合コンストラクタで、私はもはや最初のプログラム（2つのSkymapオブジェクトを持つ）の実行に続くクラッシュを経験しません。

誰でもこの根本的な原因を説明できますか？ 2番目のSkymapオブジェクトのポインタに問題があるように見えますが、私はそれが何であるかわかりません。

Answer 1

TBox* mSkymapBorderBox=new TBox(-200,-100,200,100); 

ここでは、メンバー変数ではなくローカル変数にメモリを割り当てています。また、メンバー変数にメモリを割り当てていないので、deleteを呼び出すと未定義の動作が呼び出され、ケースがクラッシュします。今メンバ変数にメモリを割り当て

mSkymapBorderBox=new TBox(-200,-100,200,100); 

：

はあなたがやるべきことはこれです。これは、ローカル変数の名前をメンバー変数と異なるようにする理由の1つです。命名規則は、このようなバグを回避するのに役立ちます。追記、または、むしろ非常に重要な注意点としては

、あなたのクラスはリソースを管理しているため、適切にデストラクタと一緒にコピーセマンティクスの実装を検討：このルールは、一般にthe-rule-of-threeと呼ばれているが。または、std::shared_ptr、std::unique_ptrなどのスマートポインタを使用しても構いません。

Answer 2

ナワズの答えは正しいです。しかし、それを超えて、あなたのコードは、いくつかの可能性のある問題を抱えている：

1. 誰もがSkyMapを作成し、決してそれにDrawAitoffSkymap呼び出した場合、あなたは、mSkymapBorderBoxが初期化されることはありませんので、ランダムな値を持つことになります（未定義の動作を取得しますそれを削除します）。
2. 誰かが特定のSkyMapでDrawAitoffSkymapを複数回呼び出すと、メモリリークが発生します。

   を（1）自分のコンストラクタでゼ​​ロにmSkymapBorderBoxを初期化します。修正するには

。

（2）DrawAitoffSkymapが複数回呼び出された場合にどのようにすべきかを決定します。一方

void Skymap::DrawAitoffSkymap() { 
    if (!mSkymapBorderBox) mSkymapBorderBox = new TBox(...); 
    ... 
} 

新しいTBOXがたびに作成する必要がある場合は、あなたがしたい：

void Skymap::DrawAitoffSkymap() { 
    delete mSkymapBorderBox; // note: does nothing if mSkymapBorderBox == 0 
    mSkymapBorderBox = new TBox(...); 
    ... 
} 

Answer 3

それは古いmSkymapBorderBoxを再利用する必要がある場合は、何かのように言いたいでしょうTBox* mSkymapBorderBox=new TBox(-200,-100,200,100);データメンバーではない新しいTBox*ポインターを作成しています。あなたはこのクラスTBOXのオブジェクトを作成し、これを宣言したとき

は

Answer 4

TBox* mSkymapBorderBox=new TBox(-200,-100,200,100); 

...あなたが同じ論理ユニット/スコープで、newを実装した後、適切deleteを実装することを検討してください。その時点で DrawAitoffSkymapを終了すると、この割り当てられたメモリの参照が失われます。

デストラクタが時刻に呼び出されると、ガベージメモリが解放されます。

この使用を避けるために、この
mSkymapBorderBox=new TBox(-200,-100,200,100);
代わりのTBox* mSkymapBorderBox=new TBox(-200,-100,200,100);