デバック用の関数がうまく動きません

フォーラム(掲示板)ルール
フォーラム(掲示板)ルールはこちら  ※コードを貼り付ける場合は [code][/code] で囲って下さい。詳しくはこちら
dic

デバック用の関数がうまく動きません

#1

投稿記事 by dic » 15年前

デバック用に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:デバック用の関数がうまく動きません

#2

投稿記事 by たかぎ » 15年前

肝心のウォッチ表示部分が含まれていないので正確なことはわかりませんが、いくつも可能性があります。

1. vやfilenameが正統なオブジェクトを指していることは呼び出し元で保証しているのでしょうか?
2. filenameがディレクトリ名であったり、ファイル名として不正な文字を含んでいたり、存在しないディレクトリを含むパスであったり、FOPEN_MAX個のストリームがすでに作られていたりということはないのでしょうか?
3. 呼び出し元ですでにおかしなことをやっていて、この部分で問題が顕在化しただけということはないのでしょうか?

dic

Re:デバック用の関数がうまく動きません

#3

投稿記事 by dic » 15年前

たかぎさん
ご指摘ありがとうございます

イテレータを使用したらぶじに動作しました
原因は・・・なぞです・・・

ありがとうございました
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:デバック用の関数がうまく動きません

#4

投稿記事 by たかぎ » 15年前

> イテレータを使用したらぶじに動作しました
> 原因は・・・なぞです・・・

たまたま動いた可能性大です。

tk-xleader

Re:デバック用の関数がうまく動きません

#5

投稿記事 by tk-xleader » 15年前

ある程度で書き換えてみました。

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のエラーチェックぐらいは有ったほうがいいと思います。

dic

Re:デバック用の関数がうまく動きません

#6

投稿記事 by dic » 15年前

たまたまですか・・・
デバック用なんで、製品版には影響ないんで大丈夫ってことにしておきます・・・

改良したらbuf取れますね

ありがとうございます

たかぎ

Re:デバック用の関数がうまく動きません

#7

投稿記事 by たかぎ » 15年前

> デバック用なんで、製品版には影響ないんで大丈夫ってことにしておきます・・・

この手の現象は、DebugOutではなく、その呼び出し元ですでにおかしくなっている可能性が考えられます(つまり製品版でも影響あり)。

sun

Re:デバック用の関数がうまく動きません

#8

投稿記事 by sun » 15年前

v->at(i).c_str()の文字列に%や*などの書式指定に該当する文字が含まれるからではないでしょうか?
だとすれば、sprintf( buf, "%s", v->at(i).c_str() );
にすれば直ると思います。

dic

Re:デバック用の関数がうまく動きません

#9

投稿記事 by dic » 15年前

ん~
3万プロセスくらい作成して実行すると
微妙にメモリくってるので200MBくらいスワップしてますねぇ~

たかぎさんのおっしゃるとおり
呼び出し元ですでにバグが混入してますね

たかぎ

Re:デバック用の関数がうまく動きません

#10

投稿記事 by たかぎ » 15年前

実際に問題がある箇所と、問題が顕在化する箇所が遠くはなれている場合、設計とコーディングの両方に問題があります。
可能な限りコンパイル時に問題を検出できるように静的にできるところは静的にすべきですし、assertや例外送出処理を入れておくなどの対策をお勧めします(今回は手遅れなら次回からでも)。

あと、標準関数の動作を正確に理解しておきましょう。
元のDebugOutを見ても、bufが不要というだけでなく、いろいろ問題があります。

・filenameはなぜconst char*ではなくchar*なのでしょうか?
・removeはなぜ必要なのでしょうか?
・fopenでわざわざVCの独自仕様("wt")を使う理由は?
・memsetでのゼロクリアはどんな価値があるのでしょうか?
・strcpyでバッファオーバーランを起こしている可能性もあります。
・p++とすると一時オブジェクトが生成され、これが例外を送出する可能性もあります。++pとすべきでは?

など、すでに指摘済みの問題以外にもいろいろ気になる点があります。 画像

dic

Re:デバック用の関数がうまく動きません

#11

投稿記事 by dic » 15年前

>filenameはなぜconst char*ではなくchar*なのでしょうか?
気づきませんでした、牢固にするにはconst char*がいいですね

>removeはなぜ必要なのでしょうか?
何回も処理するので、前の処理で、すでにファイルがある場合、削除するためです

>fopenでわざわざVCの独自仕様("wt")を使う理由は?
VC独自とは知りませんでした・・・
Windowsを想定してるものでして

>memsetでのゼロクリアはどんな価値があるのでしょうか?
ループ内で、ゴミが入っていないように、安全にするためです

>strcpyでバッファオーバーランを起こしている可能性もあります。
これも気づきませんでした、追加します

>p++とすると一時オブジェクトが生成され、これが例外を送出する可能性もあります。++pとすべきでは?
ここだけがよくわかりません
昔のソースだとp++をよく見かけるのですが、今作ってる方のソースは++pが主流になってるようですが
p++と++pではどのような違いがあらわれてくるのでしょうか?

たかぎ

Re:デバック用の関数がうまく動きません

#12

投稿記事 by たかぎ » 15年前

> >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箇所でコンストラクタが呼ばれます。
ここで例外が送出される可能性があるのです。
もちろんオーバーヘッドにもなります。

dic

Re:デバック用の関数がうまく動きません

#13

投稿記事 by dic » 15年前

すいません、仕事やら付き合いで返事おくれました

>これで安全にはなりません。
>ナル文字の処理が不適切になっても、問題の顕在化を遅らせるだけです。
ということは、memsetで、ゼロクリアした領域を作るだけでは安全ではないのでしょうか?
どういった場合が挙げられるのでしょうか?

>のように実装される場合が多く、tempおよび返却値の2箇所でコンストラクタが呼ばれます。
>ここで例外が送出される可能性があるのです。
>もちろんオーバーヘッドにもなります。
たしかに try文で例外キャッチする方のソースもたまに見ますね
表面的な使いたしか見ておらず、実装までは見てませんでした
参考になります、ありがとうございます

たかぎ

Re:デバック用の関数がうまく動きません

#14

投稿記事 by たかぎ » 15年前

> ということは、memsetで、ゼロクリアした領域を作るだけでは安全ではないのでしょうか?
> どういった場合が挙げられるのでしょうか?

ゼロクリアしたい動機というのは、ナル文字の管理が杜撰だからではないですか?
文字列の長さと終端は必ず意識しなければなりません。それが嫌ならstd::stringなどを使うべきです。
今回のバッファオーバーランもその辺りに原因のひとつがあるように思います。
また、DebugOutの呼び出し元に不具合があっても、たまたま動けば(問題が顕在化しなければ)OKとしているところを見ても、根は同じではないかと思います。

たかぎ

Re:デバック用の関数がうまく動きません

#15

投稿記事 by たかぎ » 15年前

連続で失礼します。

> たしかに try文で例外キャッチする方のソースもたまに見ますね
> 表面的な使いたしか見ておらず、実装までは見てませんでした

例外安全は設計時に保障しなければなりません。
動かしてみて、問題が出たからもぐら叩きをしていたのでは、非常に不安定なものができあがります。


閉鎖

“C言語何でも質問掲示板” へ戻る