PGM画像を読み込むプログラム（セグメントエラー発生）※追加質問あり

[tasuke]

現在画像処理の勉強をしているものです。
PGM画像をファイルポインタから読み込んで、構造体変数に格納するプログラムを作成しました。
しかし、セグメントエラーが起きてしまいます。

簡単にプログラムの流れを説明します。
・main関数
画像読み込み関数load_imageを呼び出しています。
この関数の引数には読み込みたい画像のファイル名の文字列になります。
文字列を入れない場合はload_image関数で文字列の入力を要求するようになっております。

・load_image関数
まず受け取った引数に文字列が入っているかを調べ、なければ入力を要求します。
fopenでファイルを開きます。
続いてヘッダの読み込みをします。
今回取り扱う画像はPGM形式のためヘッダはコメント文を除いて3行になります。
コメント文を読み飛ばしながら
画像の形式、画素数、最大階調の情報を読み出します。
最後に各座標の画素値を読み出して終了します。

原因がわからず困っております。どうかご教授のほどよろしくお願いいたします。

コード:

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

/* 定数宣言 */
#define MAX_IMAGESIZE       1280 /* 想定する縦・横の最大画素数 */
#define MAX_BRIGHTNESS       255 /* 想定する最大階調値 */
#define GRAYLEVEL            256 /* 想定する階調数(=最大階調値+1) */
#define MAX_FILENAME         256 /* 想定するファイル名の最大長 */
#define MAX_BUFFERSIZE       256 /* 利用するバッファ最大長 */
#define MAX_NUM_OF_IMAGES      5 /* 利用する画像の枚数 */

typedef struct PGM{
	char format[MAX_BUFFERSIZE];
	int width;
	int height;
	int brightness;
	int image[MAX_IMAGESIZE][MAX_IMAGESIZE];
}pgm;

/* 関数のプロトタイプ宣言 */
/* 階調画像を入力する関数 */
struct PGM load_image(char name[] );
/* name[]:ファイル名（""のときはキーボード入力）*/
/* 階調画像を出力する関数*/
void save_image( int n, char name[] );
/*name[]:ファイル名（""のときはキーボード入力）*/


int main(void){

	pgm im;
	im = load_image("");

	return 0;
}

struct PGM load_image(char name[] ){

	FILE *fp;
	char file_name[MAX_FILENAME];
	char buff[MAX_BUFFERSIZE];
	int header = 3;
	pgm img;
	int x,y;


	if(name[0] != '\0'){
		strcpy(file_name, name);
	}else{
		printf("入力ファイル名(*.pgm)：");
		scanf("%s", file_name);
	}
	if((fp = fopen(file_name,"r")) == NULL){
		printf("ファイルを開くことができませんでした\n");
		exit(1);
	}

	while(header){
		fgets(buff, MAX_BUFFERSIZE, fp);
		if(buff[0] != '#'){
			switch(header){
			case 3:
				if(buff[0] != 'P' || buff[1] != '2'){
					printf("P2形式の画像ではありません\n");
					exit(1);
				}
				strcpy(img.format, buff);
				break;

			case 2:
				sscanf(buff,"%d %d", &img.width, &img.height);
				break;

			case 1:
				sscanf(buff, "%d", &img.brightness);
				break;
			}
                 header--;
		}
	}

	for(y = 0; y < img.width; y++){
		for(x = 0; x < img.height; x++){
			img.image[x][y] = (int)fgetc(fp);
		}
	}

	return img;

	fclose(fp);
}

[box]

画像ファイルというからには、中身はバイナリーだと思います。それを

コード:

				sscanf(buff,"%d %d", &img.width, &img.height);
				sscanf(buff, "%d", &img.brightness);

こんな風にテキストファイルっぽく扱っていいのでしょうか。本当にテキストファイルならばそれでいいのかもしれませんが…。
上に挙げた変数の値はどうなっていますか？sscanfの直後にprintfを入れて確認してみてください。

コード:

	return img;

	fclose(fp);

return文の後ろに実行文が来るのはよろしくないと思います。コンパイル時に警告が出ませんでしたか？

[みけCAT]

しかし、セグメントエラーが起きてしまいます。

具体的にどこでエラーが発生しますか？
大きなデータを自動変数で確保しているので、スタックオーバーフローの疑いがあります。

[tasuke]

>>box　様、回答ありがとうございます。

ご指摘いただいた2点についてお答えします。
・画像の取り扱いについて
今回取り扱う画像はP2形式のPGM画像であるため、ascii形式でデータを記録します。　（このことを伝え忘れておりました、すみません...）
P2形式のPGM画像をテキストエディタで開くと以下のような構成になっております。

----------------------------------------------------------------------------------------
P2
512 512
255
＜(0,0)の画素値＞ ＜(1,0)の画素値＞ ＜(1,0)の画素値＞ ・・・
　　　　　　　　　　　　　　　　　　　　・
　　　　　　　　　　　　　　　　　　　　・
　　　　　　　　　　　　　　　　　　　　・

----------------------------------------------------------------------------------------

よって今回はテキストファイルとして扱うことができると思われます。
printfでの確認を実行しようとしましたが、セグメントエラーでやはり実行できませんでした。

・fcloseの位置について
こちらは訂正させていただきました。
しかし、セグメントエラーはなくなりませんでした。

[tasuke]

みけCAT　様、回答ありがとうございます。

pgm構造体を宣言することでセグメントエラーが発生しているようです。
構造体宣言部を消して実行するとセグメントエラーはなくなりました。
しかし、この構造体のどこに問題があるのか自分ではよくわかりません。

スタックオーバーフローということはこの構造体変数のデータが大きすぎるということでしょうか？

[みけCAT]

スタックオーバーフローということはこの構造体変数のデータが大きすぎるということでしょうか？

スタックに置くには大きすぎるということですね。
malloc()関数などを用いて領域を確保するといいでしょう。

[tasuke]

>>みけCAT　様、回答ありがとうございます。
早速試してみます、ありがとうございました！

[tasuke]

追加の質問をさせてください。
構造体をメモリの動的確保をしてから宣言することで先ほどのセグメントエラーが消えました。
しかし、今度はファイル名入力後に再びセグメントエラーが発生しました。
エラーは26~48行目のヘッダ読み飛ばし処理の中で起きているようです。（この処理をコメントアウトするとセグメントエラーは消えました。）
この処理の中ではすでに宣言済みの変数しか使用していないため
先ほどのオーバーフローと同じ問題ではないと考えていますが、自分ではよくわかりません。
何度も質問して申し訳ありませんが、よろしくお願いいたします。
以下に今回使用したPGM画像のヘッダ部分を載せます。

--------------------------------------------------------------------
P2
# Created by IrfanView
512 512
255
162 162 162 161 162 157 163 161 165 161 ・・・

--------------------------------------------------------------------

コード:

struct PGM *load_image(char name[] ){

	FILE *fp;
	char file_name[MAX_FILENAME];
	char buff[MAX_BUFFERSIZE];
	int header = 3;
	pgm *img;
	int x,y;

	img = malloc(256 + sizeof(int)*3 + 256*256*sizeof(int));
	if(img == NULL)
		printf("記憶域の確保に失敗しました。\n");


	if(name[0] != '\0'){
		strcpy(file_name, name);
	}else{
		printf("入力ファイル名(*.pgm)：");
		scanf("%s", file_name);
	}
	if((fp = fopen(file_name,"r")) == NULL){
		printf("ファイルを開くことができませんでした\n");
		exit(1);
	}

	while(header){
		fgets(buff, MAX_BUFFERSIZE, fp);
		if(buff[0] != '#'){
			switch(header){
			case 3:
				if(buff[0] != 'P' || buff[1] != '2'){
					printf("P2形式の画像ではありません\n");
					exit(1);
				}
				strcpy(img->format, buff);
				break;

			case 2:
				sscanf(buff,"%d %d", &img->width, &img->height);
				break;

			case 1:
				sscanf(buff, "%d", &img->brightness);
				break;
			}
			header--;
		}
	}

	for(y = 0; y < img->width; y++){
		for(x = 0; x < img->height; x++){
			img->image[x][y] = (int)fgetc(fp);
		}
	}

	free(img);
	fclose(fp);
	return img;

}

[みけCAT]

このセグメントエラーの問題は後で考えるとして、freeした後のゴミアドレスを返しているのはなぜですか？

[みけCAT]

コード:

	img = malloc(256 + sizeof(int)*3 + 256*256*sizeof(int));

こんな無駄に複雑で間違えたり環境依存になったりする可能性の高い式を書くのではなく、素直に構造体の大きさ(sizeof(pgm)またはsizeof(*img))だけ確保しましょう。
実際、imageは1280×1280のはずなのに、256×256しか確保していないので、明らかに確保したバッファが足りなくなるでしょう。

エラーは26~48行目のヘッダ読み飛ばし処理の中で起きているようです。（この処理をコメントアウトするとセグメントエラーは消えました。）

セグメントエラーが消えたのは、malloc()関数で確保して初期化していない領域の不定の値を用い、未定義動作を起こした結果たまたまではないですよね？
きちんと別途必要なメンバを初期化してテストしましたよね？

[tasuke]

みけCAT 様、回答ありがとうございます。

きちんとsizeof(pgm)としたところ問題なく動作しました。
申し訳ありませんでした。