fclose()でセグメンテーション違反

質問

C言語でのファイル読み込みで、ファイルを開いてデータを読み込んで表示することはできたのですが、最後のfclose(fp)でセグメンテーション違反になります。一番最後のwhileループをコメントアウトしたところセグメンテーション違反にはならなかったので、ここに何か問題があると思うのですが、どこが悪いのかが分かりません...

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

main(void){
FILE *fp;
int i=0 , j=0 ,cnt = 0 , c=0;
char *name="data.txt";
float *buf1,*buf2;

if((fp = fopen(name,"r")) == NULL){
printf("error\n");
return 0;
}
else{
while((c = getc(fp)) != EOF) {
if(c == '\n') cnt++;
}
printf(">>%d<<\n",cnt+1);
rewind(fp);
buf1 = (float *)malloc( (int)(cnt/2) +1);
buf2 = (float *)malloc( (int)(cnt/2) +1);
/*ここからがおかしい？*/
while(1){
if( fscanf( fp ,"%f %f",&buf1[i],&buf2[i])==EOF )break;
printf("%f %f\n",buf1[i],buf2[i]);
i++;
}
fclose(fp);
}
return 0;
}

なぜセグメンテーション違反になっているのでしょうか？

回答No.2

kmee · ベストアンサー

数値が1行に一つ、2行で1セットなら、データ数+番兵用に一つで、 「行数/2 + 1」個、というのは間違いではないでしょう。
※ 1行に2つデータが並んでいる、とかになると、上記計算では間違いです。

問題は、mallocの引数に指定する値は、「データの個数」ではなく、「領域の大きさ」だということ。
配列として使いたいなら、 「1要素の大きさ」×「要素数」以上を指定しないと領域不足になる、ということです。
floatの配列を n/2+1用意したいなら、 sizeof(float)*(n/2+1)です。

エラーで止まるところ≠問題のあるところ、なのが、プログラミングの難しいところ。
ただ、セグメンテーション違反が出るときは、9割方は配列、ポインタ絡みです。
配列の添字が範囲外、確保してないアドレスやNULLへのアクセス、等

お礼

回答有難うございました。

その通り、malloc部分を修正するとセグメンテーション違反にならなくなりました。

「1要素の大きさ」×「要素数」を指定しなければならないというのも、以前に読んだ本の中にしっかり書かれていたのに、完璧に忘れてしまっていましたorz

データは"一行に二つ"ですので、別の回答者のasuncionさんがおっしゃっているように
buf1 = (float *)malloc( sizeof(float)*(cnt +1));
と修正た結果、正しく動作しました。

ありがとうございました・

回答No.3

asuncion

mallocの引数に sizeof(float) *
が抜けているというご指摘は#2さんのおっしゃるとおりですね。
ただ、

＞数値が1行に一つ、2行で1セットなら、データ数+番兵用に一つで、
＞ 「行数/2 + 1」個、というのは間違いではないでしょう。

10行（20個）分のデータを読みたいのに
sizeof(float) * 6
で本当にいいのかどうかは疑問が残ります。

お礼

丁寧な回答、ありがとうございました。エラーのモヤモヤがすっきりしました^^

補足

おそらくkmeeさんが
＞数値が1行に一つ、2行で1セットなら、データ数+番兵用に一つで、
＞ 「行数/2 + 1」個、というのは間違いではないでしょう。
とおっしゃっているのは、
「一行に一つのデータで10行分のデータがあり(全データ数が10個)、それを二行ずつ1セットにして読み込む場合」
ではないかと思います。なのでそれぞれの配列の要素数は5個となり、sizeof(float)*6で大丈夫では無いかと思います。

自分の質問の場合は、asuncionさんのおっしゃっている通りなので間違っていました。

回答No.1

asuncion

変数cntは、ファイルの行数であるとお見受けしました。
違っていたらおっしゃってください。

さて、
＞buf1 = (float *)malloc( (int)(cnt/2) +1);
＞buf2 = (float *)malloc( (int)(cnt/2) +1);

malloc関数の引数をこのようにされた理由は何でしょうか。
ファイルの行数分だけ確保する必要はないのでしょうか。

補足

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

確かに変な記述してますね…気付きませんでした。
以前に作ったものを参考にしながら書いたので、前後との整合性を考えずそのままにしてしまっていました。

ご指摘の通り、cntはファイルの行数を数えるためのものです。