C言語による判別分析法プログラム

判別分析法を行うプログラムを作ったのですが、セグメンテーションエラーが出ます。
//←ここがあやしい
のところに原因があると思います。メモリが足りないのでしょうか。
しかし、mallocやfreeの使い方もいまいち分からず、うまくメモリを空けられません。
解決策をお願いします。

Picture* threshold(FILE* fp,Picture* pPic,int type){
int i,j,k; /* ループ用変数 */
unsigned char tmp; /* 作業用変数 */
unsigned char *r,*g,*b; /* 作業に使用するポインタ */
if(fp == NULL ||
pPic == NULL ||
(type < 1 || type >6)) /* 引数が異常 */
return NULL;
r = pPic->r;
g = pPic->g;
b = pPic->b;
/* ファイルヘッダの出力 */
if(fprintf(fp,"P%d\n%d %d\n",type,pPic->x,pPic->y) < 0)
return NULL;
if(type == 2 || type == 3 || type == 5 || type == 6){
if(fprintf(fp,"255\n") < 0)
return NULL;
}
/* 画像データの出力 */

/* 2値ascii形式 */
int kido,count,hist[256],n,l,sikii[256],Msikiin;
double sumhei1[256],sumbun1[256],heikin1[256],sumhei2[256],sumbun2[256],heikin2[256];
sikii[0]=0;
for(kido=0;kido<=255;kido++){
count=0;
for(i=0 ; i<pPic->y ; i++){
for(j=0 ; j<pPic->x ; j++){
if(*(r++)==kido){
count++;
}
}
}
hist[kido]=count;//←ここがあやしい
//printf("%d\n",count);debug
}
for(n=1;n<=255;n++){
for(l=0;l<n;l++){
sumhei1[n]=sumhei1[n]+hist[l];
}
heikin1[n]=sumhei1[n]/n;
for(l=0;l<n;l++){
sumbun1[n]=sumbun1[n]+pow(hist[l]-heikin1[n],2);
}
for(l=n;l<=255;l++){
sumhei2[n]=sumhei2[n]+hist[l];
}
heikin2[n]=sumhei2[n]/(255-n+1);
for(l=n;l<=255;l++){
sumbun2[n]=sumbun2[n]+pow(hist[l]-heikin2[n],2);
}
sikii[n]=sumhei1[n]*sumhei2[n]*pow(heikin1[n]-heikin2[n],2);
if(sikii[n]>sikii[n-1]){
Msikiin=n;
}
}
for(i=0 ; i<pPic->y ; i++){
for(j=0 ; j<pPic->x ; j++){
if(*(r++) < Msikiin)
tmp = 1;
else
tmp = 0;
if(fprintf(fp,"%d\n",tmp) < 0)
return NULL;
}
}
return pPic;
}

No.5 ベストアンサー 回答者： koi1234 回答日時： 2013/06/24 23:47

No4 編集ミスってました　　以下のように修正

for(kido=0;kido<255;kido++){
count=0;
r = pPic->r;
for(i=0 ; i<pPic->y ; i++){
for(j=0 ; j<pPic->x ; j++){
if(*(r++)==kido){
count++;
}
}
}
hist[kido]=count;//←ここがあやしい
//printf("%d\n",count);debug
}

ただ処理回数増やして実行速度落とすだけで非常に無駄なコードなので私なら
以下のようにする

count=0;
for(i=0 ; i<pPic->y ; i++){
for(j=0 ; j<pPic->x ; j++){
if(*(r++)==kido){
count++;
}
}
}

for(kido=0;kido<255;kido++){
hist[kido]=count;//←ここがあやしい
//printf("%d\n",count);debug
}

No.7 回答者： koi1234 回答日時： 2013/06/25 00:56

>No.5の後者のだとうまくいかなかった（countは各kidoで初期化したい）ので前者のでいきました。

そんなことないだろ と思ってよく見たら　kido　と比較してましたね
そりゃうごかんわ 失礼しました

No.6 回答者： koi1234 回答日時： 2013/06/24 23:56

追加です　　全体のソースまともに見もしなかったけど軽く見たら

No5の変更したとしても　それ以降で同じことやってるのでそれだけじゃ治りません

必要なところに
r = pPic->r;
を加えてポインタを適切な位置に再設定してください
それがどこかはあえて書かない

この回答への補足

朝に引き続いての回答ありがとうございます。
No.5の後者のだとうまくいかなかった（countは各kidoで初期化したい）ので前者のでいきました。
r=pPic->rは
r = pPic->r;
for(i=0 ; i<pPic->y ; i++){
for(j=0 ; j<pPic->x ; j++){
if(*(r++) < Msikiin)
のとこにいれました。

結果、画像が2値化されず、真っ黒になったので、現在調査中です。

補足日時：2013/06/25 00:10

この回答へのお礼

ようやく画像がちゃんと2値化されました。
こんな無能な僕に丁寧に教えてくださり本当にありがとうございました。
よければ勉強法も教えていただきたいです。やはり経験あるのみなのでしょうか。

お礼日時：2013/06/25 00:56

No.4 回答者： koi1234 回答日時： 2013/06/24 23:40

前からの継続かな？（内容は違うけど　構造体自体は同じように見えるので）

ここが怪しいと書かれた上の
for(i=0 ; i<pPic->y ; i++){
for(j=0 ; j<pPic->x ; j++){
if(*(r++)==kido){
count++;
}
}
}
まではいいが それを　for(kido=0;kido<=255;kido++){ でループさせてるために
結果として　*(r++) でとんでもないところをアクセスしてエラーになる

コード全体の検証はしてないからそれで求める結果を得られるかは知らないが
エラーをなくしたいのであれば以下のようにすればエラーはなくなると思われる

for(kido=0;kido<255;kido++){
r = pPic->r;
for(j=0 ; j<pPic->x ; j++){
if(*(r++)==kido){
count++;
}
}
}
hist[kido]=count;//←ここがあやしい
//printf("%d\n",count);debug
}

No.3 回答者： kmee 回答日時： 2013/06/24 23:09

バッファオーバーランの原因は「*(r++)」でしょうね。

あと、sumhei1を始め、配列の中身を確認してください。むちゃくちゃになっていると思います。

No.2 回答者： samtomsan 回答日時： 2013/06/24 22:54

> hist[kido]=count;//←ここがあやしい

この前で kido の値を表示してみれば「あやしい」かどうか判ります。
デバッグの方法を考えてみてください。
配列が hist[256] になっていますから、kido が 256 になっていればご自分で考えた通りに「あやしい」ことになります。

この回答への補足

debugのところをkidoを出力するようにして実行してみたら
0
1
2
3
segmentation fault（コアダンプ）
となりました。別の理由であやしいようですが、見当がつきません・・・。

補足日時：2013/06/24 23:09

No.1 回答者： wormhole 回答日時： 2013/06/24 22:52

>hist[kido]=count;//←ここがあやしい

なぜあやしいと思ったんですか。
あやしいと思ったからには何か根拠があるんですよね？

int hist[256];
で、添字として使って大丈夫なのはいくつからいくつまでかわかりますか？

この回答への補足

いやーお恥ずかしい、<=255を<255に変えました。
ですが、debugのところを//を外して実行すると、
8
37
59
9
segmentation fault(コアダンプ）
となるので、配列をはみ出したとかではないなと考えます。（いやさっきまではみ出してましたが）
４回だけループしてセグフォする理由が分かりません・・・

補足日時：2013/06/24 23:07