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 · ベストアンサー

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

>No.5の後者のだとうまくいかなかった（countは各kidoで初期化したい）ので前者のでいきました。
そんなことないだろ と思ってよく見たら　kido　と比較してましたね
そりゃうごかんわ 失礼しました

回答No.6

koi1234

追加です　　全体のソースまともに見もしなかったけど軽く見たら
No5の変更したとしても　それ以降で同じことやってるのでそれだけじゃ治りません

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

お礼

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

補足

朝に引き続いての回答ありがとうございます。
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値化されず、真っ黒になったので、現在調査中です。

回答No.4

koi1234

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

ここが怪しいと書かれた上の
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

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

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

回答No.2

samtomsan

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

補足

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

回答No.1

wormhole

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

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

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

補足

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