コンストラクタ内での動的メモリ割り当ての対処

　　こんにちは。　
　　VS.NET2003でＣ++を使いプログラミングをしています。
　　コンストラクタ内で、動的にメモリを割り当てたクラスを何度も生成すると動きがおかしくなるようなんです。
　　メモリブロックでのエラーがでたり、アプリがフリーズします。
　　以下はそのクラスのコンストラクタのコードです。

ClassA::ClassA(double x, double y, WORD tm, WORD len, WORD rat, ...)
: CBullet(x, y, 0, 0, 0, 0, 20), LT(tm), Tp(rat), Len(len), Tc(0), pt(NULL)
{
pt = new WORD[rat];
va_start( list, rat );
for(WORD i = 0; i < rat; i++) {
pt[i] = va_arg(list, WORD);
}
va_end(list);
}
　new で動的に割り当て、va_startマクロを使って値を代入しています。
　二重解放にならないように、関数内でdeleteしたptにＮＵＬＬを代入させ、ifを使って弾くようにしているんですが、それでも万全ではないのでしょうか？
　フリーズしたりメモリブロックでエラーを起こしたりする原因が分からない状態です。
　　よろしくお願いします。

No.3 ベストアンサー 回答者： Tacosan 回答日時： 2011/02/14 23:19

プログラムがこの通りなら (コピーコンストラクタが private なので) 引数 (と返り値) がらみの話はないようですね＞#2.

とはいえやはり「メモリ管理がなにかおかしい」感じはあるので, std::vector などを使って「なるべく自分では管理しない」という方針は考えられますな. 代入でこけるとか, そういう腐ったネタだったりする?

あと, 「WORD が int より短いとアウト」というのは「可変引数を持つ関数に実引数をどのように渡すか」に由来します. 「(signed/unsigned を含む) char と (unsigned を含む) short」は (signed あるいは unsigned の) int に, また float は double に, それぞれ自動的に変換されます. 一方, va_arg の方はそんな事情などおかまいなしに指定された型と思って値を取り出そうとします. つまり, 今の場合 「int で渡された引数を WORD で取り出す」ことになってしまい, sizeof(WORD) < sizeof(int) だと正しい値を取り出すことができなくなってしまいます. 実際, pt[i] を表示させると期待した結果にはなっていないはずです.

もっとも, 「va_list をクラスメンバにする」理由がさっぱり思い付きませんが. こいつをメンバにしてうれしいことなど全くないとしか思えない.

この回答へのお礼

　　なるほど。
マクロではint型で割り当てられているという事でしょうか？

>もっとも, 「va_list をクラスメンバにする」理由がさっぱり思い付きませんが. こいつをメンバにしてうれしいことなど全くないとしか思えない.
　なるほど、勉強になりますね。
va_listをクラスメンバにせず別の方法を使うんでしょうか？
全くない理由を言って頂ければとてもありがたいです。

お礼日時：2011/02/15 11:21

No.6 回答者： Tacosan 回答日時： 2011/02/15 16:57

「今まで作る必要がなかったからデストラクタを作らない」て....

どんな理屈だ, それは. 「今まで」がどうであろうと, このクラスは「このクラス」でしょ? だとしたら「今まで」とは無関係に「このクラスで必要かどうか」を考えるべきだし, インスタンスを 1個しか作らないことが保証できるならともかくそうでなければ「コンストラクタでメモリを確保する以上デストラクタで解放する」というのは (完全に「絶対」ではないけど) ごく普通だよ.

なぜ何度も「std::vector」と言い続けているのか, その理由はわかりますか?

この回答への補足

　　あれから暫く調べてみたんですが、今のところスマートポインタでは駄目なんだなぁというところまで来ています。
　　catch & tryでコンストラクタで保護→ガチガチに固めるならスマートポインタがいい→しかしスマートポインタは配列管理ができない。　なん・・・だと・・・　←今ここ

補足日時：2011/02/15 18:40

この回答へのお礼

そのクラスには親を継承させているので、その状態でデストラクタを呼ぶ場合についてあまり把握していなかったので、あまり追加しようとは思っていませんでした。
　たださっき動かしてみたんですが、ちゃんとデストラクタは呼ばれているみたいなんでここで解放処理をしてみようと考えています。
　ちょうどコンストラクタ内での動的割り当てでの対処ができたのでそれでいこうと思います。

　std::vectorはまだ使った事がないのであまり把握していません。
ただ今まで読んできたあたり、vectorは可変長配列で、解放などの後処理をちゃんとサポートしてくれる。　という感じです。
　なのでその辺の反応は鈍いです。

お礼日時：2011/02/15 17:19

No.5 回答者： Tacosan 回答日時： 2011/02/15 12:27

まず va_list については「当該関数が動いていなければ意味のない値」ですから, これをクラスのメンバにする意味は全くありません. 関数のローカル変数にすれば十分. たとえば, 最初の質問に挙がっているコンストラクタでも変数 i はメンバにしてませんよね. これと同レベルの話です.

むしろ, 「なぜ va_list をクラスのメンバにしたのか」の方を聞きたい.

で「マクロではint型で割り当てられているという事でしょうか？」については何を聞いているのかが分かりません. 単純に＊言語仕様＊として「可変長引数として渡した (int より短い) 実引数は必ず int (か unsigned int) に変換される」というだけであり, ここにはマクロは関係ありません.

本題については, 何度か書いてるけど
・std::vector を使う
・代入演算子とデストラクタを適切に宣言 (and/or 定義) する
あたり? 現状コピーはできないのでコンパイルエラーがなければあえてコピーコンストラクタを定義する必要はないと思いますが, 特に代入演算子が宣言されていないために「メンバごとの代入」をする代入演算子が自動的に作られている可能性がなきにしもあらず. あと, なんでデストラクタを作らない?

この回答への補足

　　デストラクタを作らないのは今まで作る必要がなかったからです。
動的に割り当てて動かすクラスは滅多にないので。
　　でもデストラクタ内で解放処理を行った方がいいんでしょうか？
コピーコンストラクタについてもう少し勉強してみようと思います。

補足日時：2011/02/15 14:52

この回答へのお礼

　　回答ありがとうございます。
上のva_listの理由とWORDとshortの問題は理解できました。
・代入演算子とデストラクタを適切に宣言 (and/or 定義) する
という方針を採ってみようと思います

お礼日時：2011/02/15 14:58

No.4 回答者： tsuduki123 回答日時： 2011/02/14 23:19

他のひともいってますが、このコンストラクタ自身に問題はないように思います。

sakuraエディタでも何でもいいから、ClassAを生成している行をピックアップして
ratの値が嘘ついていないことを確認して、問題なければ
pt に アクセスしている部分に片っ端からassert()を仕込んでみたらどうですか。

この回答へのお礼

>ptに アクセスしている部分に片っ端からassert()を仕込んでみたらどうですか。
assertでそのアクセスしている部分がＮＵＬＬでないか？を調べるということでしょうか？

お礼日時：2011/02/15 11:27

No.2 回答者： akayoroshi 回答日時： 2011/02/14 18:41

　「class Aのオブジェクトを関数の実引数にしていて、その関数の対応する仮引数が参照型でない」ということはありませんか。

この場合、関数が引用されたとき、仮引数のオブジェクトが作られて実引数のメンバ変数の値がコピーされます。関数の終了時に仮引数オブジェクトのディストラクタが呼び出されますが、その中にdeleteがあると、実引数のオブジェクトが確保したメモリが解放されてしまいます。ディストラクタの中でポインタをNULLにしても、実引数のオブジェクトには反映されません。
　関数の仮引数を参照型にするか、コピーコンストラクタを作ってディープコピーを行うようにすれば回避できます。

この回答への補足

　NULLを代入しているのはクラスのメソッドの中です。解放しているのもそのメソッドの中です。

補足日時：2011/02/15 11:11

この回答へのお礼

　コピーコンストラクタを作ってディープコピーをするんですね？
　やってみます。

お礼日時：2011/02/15 11:11

No.1 回答者： Tacosan 回答日時： 2011/02/14 16:27

ん～, これだけしか情報がないと分かんないなぁ.

「メモリブロックでのエラー」が何を意味しているのか分からないし, WORD が本当に正しいのかどうかも不明 (WORD が int より短いとアウト). あと, 「list」の定義がないのはなぜ?

あるいは, new を使って自前でメモリを管理する代わりに std::vector を使ったらどうなるんだろう.

この回答への補足

>WORD が本当に正しいのかどうかも不明 (WORD が int より短いとアウト).
とはどういうことなんでしょうか？
　WORDはunsigned shortで、intより短いです。
　これに何か原因があるのでしょうか？

補足日時：2011/02/14 16:44

この回答へのお礼

listがなんであるのか下記のコードを載せておきます。
クラス内のメンバ変数で、マクロです。

class ClassA : public Class
{
double SPD;
WORD Tp, Tc, LT, Len;
WORD *pt;
va_list list;//va_listというマクロです。

private:
ClassA(const ClassA &src);
public:
ClassA(double x, double y, WORD tm, WORD l en, WORD rat, ...);
};
メモリブロックのエラーはWORD* ptでの二重解放で起きたことがあるんですが、他の箇所にもでてきてるようです。

お礼日時：2011/02/14 16:44