ページ 1 / 1
デバック用の関数がうまく動きません
Posted: 2010年2月20日(土) 18:01
by dic
デバック用にstringに格納されている文字列をすべて
テキストファイルに出力する関数DebugOutを作ったのですが
画像のように、forループの中でエラーが起きます
ブレークポイント1からブレークポイント2の間に
エラーがおきてます
何が原因でしょうか?
VC++6.0です
void DebugOut( const vector<string> *v, char *filename ) {
int i;
remove( filename );
FILE *file = fopen( filename, "wt" );
for( i=0; i<v->size(); i++ ) {
static char buf[1024000];
memset( buf, 0, sizeof(buf) );
sprintf( buf, v->at(i).c_str() );
fputs( buf, file );
}
fclose( file );
}
Re:デバック用の関数がうまく動きません
Posted: 2010年2月20日(土) 21:52
by たかぎ
肝心のウォッチ表示部分が含まれていないので正確なことはわかりませんが、いくつも可能性があります。
1. vやfilenameが正統なオブジェクトを指していることは呼び出し元で保証しているのでしょうか?
2. filenameがディレクトリ名であったり、ファイル名として不正な文字を含んでいたり、存在しないディレクトリを含むパスであったり、FOPEN_MAX個のストリームがすでに作られていたりということはないのでしょうか?
3. 呼び出し元ですでにおかしなことをやっていて、この部分で問題が顕在化しただけということはないのでしょうか?
Re:デバック用の関数がうまく動きません
Posted: 2010年2月20日(土) 23:53
by dic
たかぎさん
ご指摘ありがとうございます
イテレータを使用したらぶじに動作しました
原因は・・・なぞです・・・
ありがとうございました
void DebugOut( vector<string> *out, char *filename ) {
remove( filename );
FILE *file = fopen( filename, "wt" );
vector<string>::iterator p = out->begin();
static char buf[10240];
while( p != out->end() )
{
memset( buf, 0, sizeof(buf) );
strcpy( buf, p->c_str() );
fputs( buf, file );
p++;
}
fclose( file );
}
Re:デバック用の関数がうまく動きません
Posted: 2010年2月20日(土) 23:54
by たかぎ
> イテレータを使用したらぶじに動作しました
> 原因は・・・なぞです・・・
たまたま動いた可能性大です。
Re:デバック用の関数がうまく動きません
Posted: 2010年2月21日(日) 08:39
by tk-xleader
ある程度で書き換えてみました。
void DebugOut(vector<string> &out,const char *filename ) {
FILE *file = fopen( filename, "wt" );
if(file==NULL)return;
vector<string>::iterator p = out->begin();
while( p != out->end() )
{
fputs( (*p).c_str(), file );
p++;
}
fclose( file );
}
bufは不要だと思います、fopenのエラーチェックぐらいは有ったほうがいいと思います。
Re:デバック用の関数がうまく動きません
Posted: 2010年2月21日(日) 09:00
by dic
たまたまですか・・・
デバック用なんで、製品版には影響ないんで大丈夫ってことにしておきます・・・
改良したらbuf取れますね
ありがとうございます
Re:デバック用の関数がうまく動きません
Posted: 2010年2月21日(日) 09:07
by たかぎ
> デバック用なんで、製品版には影響ないんで大丈夫ってことにしておきます・・・
この手の現象は、DebugOutではなく、その呼び出し元ですでにおかしくなっている可能性が考えられます(つまり製品版でも影響あり)。
Re:デバック用の関数がうまく動きません
Posted: 2010年2月21日(日) 10:57
by sun
v->at(i).c_str()の文字列に%や*などの書式指定に該当する文字が含まれるからではないでしょうか?
だとすれば、sprintf( buf, "%s", v->at(i).c_str() );
にすれば直ると思います。
Re:デバック用の関数がうまく動きません
Posted: 2010年2月21日(日) 11:55
by dic
ん~
3万プロセスくらい作成して実行すると
微妙にメモリくってるので200MBくらいスワップしてますねぇ~
たかぎさんのおっしゃるとおり
呼び出し元ですでにバグが混入してますね
Re:デバック用の関数がうまく動きません
Posted: 2010年2月21日(日) 12:10
by たかぎ
実際に問題がある箇所と、問題が顕在化する箇所が遠くはなれている場合、設計とコーディングの両方に問題があります。
可能な限りコンパイル時に問題を検出できるように静的にできるところは静的にすべきですし、assertや例外送出処理を入れておくなどの対策をお勧めします(今回は手遅れなら次回からでも)。
あと、標準関数の動作を正確に理解しておきましょう。
元のDebugOutを見ても、bufが不要というだけでなく、いろいろ問題があります。
・filenameはなぜconst char*ではなくchar*なのでしょうか?
・removeはなぜ必要なのでしょうか?
・fopenでわざわざVCの独自仕様("wt")を使う理由は?
・memsetでのゼロクリアはどんな価値があるのでしょうか?
・strcpyでバッファオーバーランを起こしている可能性もあります。
・p++とすると一時オブジェクトが生成され、これが例外を送出する可能性もあります。++pとすべきでは?
など、すでに指摘済みの問題以外にもいろいろ気になる点があります。

Re:デバック用の関数がうまく動きません
Posted: 2010年2月21日(日) 12:26
by dic
>filenameはなぜconst char*ではなくchar*なのでしょうか?
気づきませんでした、牢固にするにはconst char*がいいですね
>removeはなぜ必要なのでしょうか?
何回も処理するので、前の処理で、すでにファイルがある場合、削除するためです
>fopenでわざわざVCの独自仕様("wt")を使う理由は?
VC独自とは知りませんでした・・・
Windowsを想定してるものでして
>memsetでのゼロクリアはどんな価値があるのでしょうか?
ループ内で、ゴミが入っていないように、安全にするためです
>strcpyでバッファオーバーランを起こしている可能性もあります。
これも気づきませんでした、追加します
>p++とすると一時オブジェクトが生成され、これが例外を送出する可能性もあります。++pとすべきでは?
ここだけがよくわかりません
昔のソースだとp++をよく見かけるのですが、今作ってる方のソースは++pが主流になってるようですが
p++と++pではどのような違いがあらわれてくるのでしょうか?
Re:デバック用の関数がうまく動きません
Posted: 2010年2月21日(日) 12:35
by たかぎ
> >removeはなぜ必要なのでしょうか?
> 何回も処理するので、前の処理で、すでにファイルがある場合、削除するためです
fopenで"w"(またはVCの独自仕様を使うなら"wt")を指定した場合、既存のファイルはサイズゼロに切り詰められます。
つまり、普通なら事前にremoveの必要はありません。
それをあえてやっているのは何か理由があるのかな? と考えたのです。
> >memsetでのゼロクリアはどんな価値があるのでしょうか?
> ループ内で、ゴミが入っていないように、安全にするためです
これで安全にはなりません。
ナル文字の処理が不適切になっても、問題の顕在化を遅らせるだけです。
> >p++とすると一時オブジェクトが生成され、これが例外を送出する可能性もあります。++pとすべきでは?
> ここだけがよくわかりません
> 昔のソースだとp++をよく見かけるのですが、今作ってる方のソースは++pが主流になってるようですが
> p++と++pではどのような違いがあらわれてくるのでしょうか?
簡単にいうと、後置形式の++は、
iterator iterator::operator++(int)
{
iterator temp(*this);
++*this;
return temp;
}
のように実装される場合が多く、tempおよび返却値の2箇所でコンストラクタが呼ばれます。
ここで例外が送出される可能性があるのです。
もちろんオーバーヘッドにもなります。
Re:デバック用の関数がうまく動きません
Posted: 2010年2月22日(月) 19:07
by dic
すいません、仕事やら付き合いで返事おくれました
>これで安全にはなりません。
>ナル文字の処理が不適切になっても、問題の顕在化を遅らせるだけです。
ということは、memsetで、ゼロクリアした領域を作るだけでは安全ではないのでしょうか?
どういった場合が挙げられるのでしょうか?
>のように実装される場合が多く、tempおよび返却値の2箇所でコンストラクタが呼ばれます。
>ここで例外が送出される可能性があるのです。
>もちろんオーバーヘッドにもなります。
たしかに try文で例外キャッチする方のソースもたまに見ますね
表面的な使いたしか見ておらず、実装までは見てませんでした
参考になります、ありがとうございます
Re:デバック用の関数がうまく動きません
Posted: 2010年2月22日(月) 22:24
by たかぎ
> ということは、memsetで、ゼロクリアした領域を作るだけでは安全ではないのでしょうか?
> どういった場合が挙げられるのでしょうか?
ゼロクリアしたい動機というのは、ナル文字の管理が杜撰だからではないですか?
文字列の長さと終端は必ず意識しなければなりません。それが嫌ならstd::stringなどを使うべきです。
今回のバッファオーバーランもその辺りに原因のひとつがあるように思います。
また、DebugOutの呼び出し元に不具合があっても、たまたま動けば(問題が顕在化しなければ)OKとしているところを見ても、根は同じではないかと思います。
Re:デバック用の関数がうまく動きません
Posted: 2010年2月22日(月) 22:27
by たかぎ
連続で失礼します。
> たしかに try文で例外キャッチする方のソースもたまに見ますね
> 表面的な使いたしか見ておらず、実装までは見てませんでした
例外安全は設計時に保障しなければなりません。
動かしてみて、問題が出たからもぐら叩きをしていたのでは、非常に不安定なものができあがります。
Re:デバック用の関数がうまく動きません
Posted: 2010年2月23日(火) 17:46
by dic
ありがとうございました