メモリ関連のエラーを取り除くには・・

質問

下記プログラムで、「.」区切りの文字列を
区切って返すプログラムを作成していますが、
メモリ関連のエラーが出てしまいます。

このようなメモリのエラーを取り除くには、何か
有用なツールでも使用すると楽になるのでしょうか。

自分で追いかけるのは大変なので・・

ちなみにmainの中で関数に渡したResultに結果が
入ってくるはずなのですが、mainの中で参照しようと
するとエラーになってしまったので、bunkatu関数のなかで
表示するように一時的に変えております。

また、メモリの動的確保を行う場合には、同一のブロックで
行う必要があるのでしょうか。

#include <cstdlib>
#include <iostream>

using namespace std;
int bunkatu (char *pOrg, int *iNum, char **Result);

void main ()
{

int iNum;
char **Result = NULL; //結果格納用

bunkatu ("aaaa.fasdfafda.fa.dddd", &iNum, Result);

}

int bunkatu (char *pOrg, int *iNum, char **Result)
{

int iRtn;
int iLocate = 0;
int iTarget[10];
int iCount = 0;
char *pTmp;

pTmp = pOrg;

if (*pTmp == '\0') { //空文字列の場合
iRtn = 1;
}
else { //空文字列以外の場合
//.を探す
while (*pTmp != '\0') { //文字列終わりまで繰り返す。
if (*pTmp == '.') { //.が見つかった場合
iTarget[iCount] = iLocate; //.が見つかった添え字を保存
iCount++; //次の保存用にカウンタをカウントアップ
}
iLocate++; //詮索先を示すものをカウントアップ
pTmp++; //詮索する位置を移動
}

Result = (char **) calloc (iCount+1, sizeof (char *));

//切り出して表示
int iStart, iEnd;
for (int j = 0; j < iCount+ 1; j++) {

if (j == 0) { //はじめ
iStart = 0;
iEnd = iTarget[j] - 1;
}
else if(j < iCount){ //中間
iStart = iTarget[j - 1] + 1;
iEnd = iTarget[j] - 1;
}
else if (j == iCount) { //最後
iStart = iTarget[j - 1] + 1;
iEnd = strlen (pOrg) -1;
}

Result[j] =
(char *) calloc (iEnd - iStart + 2,
sizeof (char));

for (int k = 0; k < iEnd - iStart + 1; k++) {
Result[j][k] = pOrg[iStart + k];
}
Result[j][iEnd - iStart + 1] = '\0';
}

*iNum = iCount;
iRtn = 0;
}

//ちなみに～の部分
for (int i = 0; i < *iNum + 1 ; i++) {
cout << Result[i] << endl;
}

for (int i = 0; i < *iNum + 1 ; i++) {
free (Result[i]);
}

free (Result);

return iRtn;
}

回答No.3

Oh-Orange · ベストアンサー

★アドバイス
＞メモリ関連のエラーが出てしまいます。
　↑
　当然です。
　文字列リテラル(文字列定数)に \0 を書き込んでいます。
　場所はソースの『切り出し表示』の最後
＞Result[j][iEnd - iStart + 1] = '\0';
　↑
　ここです。→メモリ関連のエラーでるよ。
＞何か有用なツールでも使用すると楽になるのでしょうか。
　↑
　今回はエラーメッセージより直ぐに分かります。
　ツールを使う前に文字列定数は定数なので書き換えできないという基本を
　覚えておきましょう。エラーメッセージと共に。
＞自分で追いかけるのは大変なので・・
　↑
　なれる事です。
・この程度のソースでは自分でバグを目で追いかけて探した方が早い気がしますけど。
　ツールを使う前にソースデバッグ(ソースを見てデバッグ)を行って下さい。
　あと適度にコメントしてどこまでは正常に動作しているのか、どこでエラーが発生
　するのか特定します。
＞ちなみにmainの中で関数に渡したResultに結果が
＞入ってくるはずなのですが…
　↑
　ポインタを理解していないだけ。
　『char **Result』を引数に渡して bunkatu 関数内でメモリを動的に確保したポインタを
　Result にセットしたい場合は『&』演算子をつけて下さい。
　つまり main() 関数にある bunkatu 関数の第３引数に注目。
　bunkatu( "aaaa.fasdfafda.fa.dddd", &iNum, &Result );
　↑
　あと第１引数は文字列定数ではなく文字型配列にコピーしてから渡します。
　つまり main() 関数に
　char string[] = "aaaa.fasdfafda.fa.dddd";

　bunkatu( string, &iNum, &Result );
　↑
　こうしないと実行時にエラーになります。
　理由は一番最初にアドバイスしています。上参照。
＞また、メモリの動的確保を行う場合には、同一のブロックで
＞行う必要があるのでしょうか。
　↑
　動的確保したポインタをプログラム全体で管理していれば同一のブロック(関数)で
　行う必要はありません。でもなるべく同一ブロックで処理した方が見やすいです。
　またバグが入り込まずにすみます。
・それから、ここの質問者さんのソースを覗いていつも思うのですが、１つの関数が
　やたらに行数が多いですね。もっと複数の関数に処理を分けて見やすく記述した方が
　いいですよ。
　今回の場合は bunkatu() 関数が 73 行ありますけど。
　処理内容も
　(1)文字列の検索
　(2)切り出し表示
　(3)デバッグ用なのか『ちなみに～の部分』とか
　この３つをサブ関数とかに分けておけばデバッグしやすくなりますよ。
・今回はいろいろとバッグっていますね。
＞Result = (char **) calloc (iCount+1, sizeof (char *));
　↑
　この部分がおかしいです。
　これにより bunkatu() 関数の引数宣言も変ります。
　正しくは
　*Result = (char **)calloc( iCount+1, sizeof(char*) );
　と代入して
　int bunkatu( char *pOrg, int *iNum, char ***Result )
　と関数の引数宣言をして main() 関数では
　bunkatu( string, &iNum, &Result );
　と『&』を付けないと駄目です。
・以上。今後の参考にして下さい。

補足

色々と実験してみて、エラーもなく実行できるように
なりました。
問題部分はユーザ定義関数にダブルポインタを参照渡しして
いなかったことだと思われます。

>　当然です。
>　文字列リテラル(文字列定数)に \0 を書き込んでいます。
の部分が理解できませんでした。
このままでも最終版のソースではエラー出ませんでした。

>この程度のソースでは自分でバグを目で追いかけて探した方が早い気がしますけど。
>　ツールを使う前にソースデバッグ(ソースを見てデバッグ)を行って下さい。
大規模になったときにはどのようなツールを使うのでしょうか。

回答No.6

zwi

#2のzwiです。
>私が遭遇したメモリ関連のバグはとてもデバッガだけでは解決できそうもないものでした。（プログラムは私が作成したわけではなく、とても大規模で複雑なものでした。）
>そこで、このような問題が再び起こったときに、何か便利なツールはないかという意図で質問した次第です。

基本的にカーネルでもからまない限り標準のデバッガでトレース可能だと思います。再現性さえあれば、スタック破壊やメモリ破壊の連鎖でも手間とテクニックで追跡可能です。
「デバッガだけでは解決できそうもない」のではなく、知識不足でデバッガが使いこなせていない可能性が高いのではないでしょうか？

こういう本もありますから参考にされてはどうでしょう？
http://www.amazon.co.jp/Windows%E3%83%97%E3%83%AD%E3%82%B0%E3%83%A9%E3%83%9E%E3%81%AE%E3%81%9F%E3%82%81%E3%81%AE%E3%83%87%E3%83%90%E3%83%83%E3%82%B0%E3%83%86%E3%82%AF%E3%83%8B%E3%83%83%E3%82%AF%E5%BE%B9%E5%BA%95%E8%A7%A3%E8%AA%AC-%E3%82%B8%E3%83%A7%E3%83%B3-%E3%83%AD%E3%83%93%E3%83%B3%E3%82%BA/dp/4891001860
http://www.amazon.co.jp/NET-Windows%E3%83%97%E3%83%AD%E3%82%B0%E3%83%A9%E3%83%9E%E3%81%AE%E3%81%9F%E3%82%81%E3%81%AE%E3%83%87%E3%83%90%E3%83%83%E3%82%B0%E3%83%86%E3%82%AF%E3%83%8B%E3%83%83%E3%82%AF%E5%BE%B9%E5%BA%95%E8%A7%A3%E8%AA%AC-%E3%82%B8%E3%83%A7%E3%83%B3-%E3%83%AD%E3%83%93%E3%83%B3%E3%82%BA/dp/4891003529/ref=pd_sim_b_1/250-0399562-6461017

ちなみに高額な値段や海外ソフトでも問題なければ、高機能なデバッガやテストツールが販売されてます。
http://www.componentsource.co.jp/ese/features/debugging-testing/windows/index.html
http://itpro.nikkeibp.co.jp/data/scate2.jsp?CID=020040010
これらも使いこなすには、高度な知識が不可欠です。

お礼

ご回答ありがとうございます。

たしかにデバッガの使い方に関しては、まだまだ
知識がたりません。
これから覚えていこうと思います。

回答No.5

Oh-Orange

★返信です。
＞なぜなくてもよいのでしょうか。
　↑
　main() 関数を抜けるとメモリが自動的に解放されるためなくても良い。
　これは fopen()、fclose() も同じです。
　main() 関数を抜ける自動的にファイルをクローズします。
・でも main() 以外ではプログラムが終了するまでは自動的にメモリもファイルも
　クローズしません。よって main() でも私は対応をとるために free()、fclose() を
　記述しています。他の関数では必要がなくなった時点で free()、fclose() しましょう。
＞>　当然です。
＞>　文字列リテラル(文字列定数)に \0 を書き込んでいます。
＞の部分が理解できませんでした。
　↑
　良く見たら calloc() でメモリを確保していますね。
　それなら \0 を書き込んでも問題ないようです。
　でも文字列定数を書き換えようとするとエラーダイアログなどが出ますよ。
　例えば
　char *str = "0123456789";

　str[ 3 ] = 'A';
　このようにするとエラーとなります。でも
　char str[] = "0123456789";

　str[ 3 ] = 'A';
　これはエラーにはなりません。
　文字列定数ではないので。
＞大規模になったときにはどのようなツールを使うのでしょうか。
　↑
　エラーダイアログが出たときに『デバッガ』でも起動すれば良い。
　まずは『デバッガ』を使ってみる。
・以上。

補足

ご回答ありがとうございます。

なぜfreeが要らないのかが分かりました。
・他の関数に合わせる
・他人が見て誤解を招かない（mainだと必要ないと知らない人もいる）
ためにもmainにもfreeを使ったほうがよいと思いました。

>　良く見たら calloc() でメモリを確保していますね。
>　それなら \0 を書き込んでも問題ないようです。
calloc()でメモリを確保すると、その領域は文字列定数に
なるのですか？
いまいちその辺りが理解できてないのですが、
calloc()で動的にメモリ確保した所に値を代入したら、
まずかったでしょうか
質問ばかりで申し訳ありません。

>＞大規模になったときにはどのようなツールを使うのでしょうか。
>　↑
>　エラーダイアログが出たときに『デバッガ』でも起動すれば良い。
>　まずは『デバッガ』を使ってみる。
私が遭遇したメモリ関連のバグはとてもデバッガだけでは解決
できそうもないものでした。（プログラムは私が作成したわけで
はなく、とても大規模で複雑なものでした。）
そこで、このような問題が再び起こったときに、何か便利な
ツールはないかという意図で質問した次第です。

回答No.4

Oh-Orange

★追記。
・模範解答も載せておく。
　コメントやソースを読み取って下さい。

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

// 分割
int mySplit( char *pBuff, int *iNum, char ***ppResult )
{
　char **ppList;
　char *pSeek;
　char *pFind;
　int iCount;

　// ピリオドの数を数える
　pSeek = pBuff;
　for ( iCount = 0 ; (pFind = strchr(pSeek,'.')) != NULL ; iCount++ ){
　　pSeek = (pFind + 1);
　}
　if ( *pSeek != '\0' ){
　　iCount++;
　}
　// 空文字列なら終了
　*iNum = iCount;　// 個数をセット
　if ( iCount == 0 ){
　　*ppResult = NULL;
　　return 0;
　}
　// 動的確保
　if ( (*ppResult = ppList = (char**)calloc(iCount + 1,sizeof(char*))) == NULL ){
　　return -1;　// メモリ不足
　}
　// 配列セット
　pSeek = pBuff;
　for ( *ppList++ = pSeek ; (pFind = strchr(pSeek,'.')) != NULL ; *ppList++ = pSeek ){
　　pSeek = pFind;
　　*pSeek++ = '\0';　// ピリオドを \0 に書き換え
　}
　if ( *pSeek != '\0' ){　// 最後がピリオドなら追加しない
　　*ppList++ = pSeek;
　}
　*ppList = NULL;　// 最後に NULL セット
　return iCount;　// 正常(個数を返す)
}

// メイン関数
int main( void )
{
　char string[] = "aaa.bbbb.ccccc.dddddd";
　char **Result;
　int i, iNum;

　// 分割
　mySplit( string, &iNum, &Result );
　// 表示
　for ( i = 0 ; i < iNum ; i++ ){
　　printf( "Result[%d] = %s\n", i, Result[i] );
　}
　printf( "iNum=%d\n", iNum );
　// 解放
　free( Result );　// なくても良い
　return 0;
}

解説：
・mySplit() 関数で分割します。この関数の戻り値は
　0 以上…分割した個数(iNumと同じ)
　0 以下…メモリ不足(Result=NULL保証)
　0 なら…空文字列のため分割しなかった(iNum=0,Result=NULL保証)
　となります。
・以上。

補足

ご回答ありがとうございます。

>free( Result );　// なくても良い
なぜなくてもよいのでしょうか。

回答No.2

zwi

No.1の方の代わりに回答しますとポインタの扱いが大きく間違っている箇所があります。
bunkatu()関数を呼び出すときにResultを値渡しで渡しています。Resultのアドレスを渡してやらないとbunkatu()関数で書き換えて返すことができません。
ポインタのポインタを扱っているので混乱しているのだと思いますが、実体渡しなのか値渡しなのかを常に意識しましょう。

お礼

ご回答ありがとうございます。
指摘内容を考えてみます。

回答No.1

a-saitoh

エラーを取り除くと言うよりは、エラーが起きないように1からちゃんと書きなおす、という方が近道だと思います。このプログラムだと。

まずは、 bunkatu()からResultを戻すところがなぜこれではダメなのか理解できるようになりましょう。

補足

ご回答ありがとうございます。

短いだろうと思ってアルゴリズムを考えつつやって
しまったのが問題でした。
書き直そうとは思いますが、
>bunkatu()からResultを戻すところがなぜこれではダメなのか理解できるようになりましょう。
これがなぜだか分からないのです。
ご教授いただけますでしょうか。