質問をすることでしか得られない、回答やアドバイスがある。

15分調べてもわからないことは、質問しよう!

新規登録して質問してみよう
ただいま回答率
85.50%
C

C言語は、1972年にAT&Tベル研究所の、デニス・リッチーが主体となって作成したプログラミング言語です。 B言語の後継言語として開発されたことからC言語と命名。そのため、表記法などはB言語やALGOLに近いとされています。 Cの拡張版であるC++言語とともに、現在世界中でもっとも普及されているプログラミング言語です。

Eclipse

Eclipseは、IBM社で開発された統合開発環境のひとつです。2001年11月にオープンソース化されました。 たくさんのプラグインがあり自由に機能を追加をすることができるため、開発ツールにおける共通プラットフォームとして位置づけられています。 Eclipse自体は、Javaで実装されています。

C++

C++はC言語をもとにしてつくられた最もよく使われるマルチパラダイムプログラミング言語の1つです。オブジェクト指向、ジェネリック、命令型など広く対応しており、多目的に使用されています。

Q&A

解決済

2回答

611閲覧

エラーメッセージはでずアプリケーションエラーになる

ping_100

総合スコア7

C

C言語は、1972年にAT&Tベル研究所の、デニス・リッチーが主体となって作成したプログラミング言語です。 B言語の後継言語として開発されたことからC言語と命名。そのため、表記法などはB言語やALGOLに近いとされています。 Cの拡張版であるC++言語とともに、現在世界中でもっとも普及されているプログラミング言語です。

Eclipse

Eclipseは、IBM社で開発された統合開発環境のひとつです。2001年11月にオープンソース化されました。 たくさんのプラグインがあり自由に機能を追加をすることができるため、開発ツールにおける共通プラットフォームとして位置づけられています。 Eclipse自体は、Javaで実装されています。

C++

C++はC言語をもとにしてつくられた最もよく使われるマルチパラダイムプログラミング言語の1つです。オブジェクト指向、ジェネリック、命令型など広く対応しており、多目的に使用されています。

0グッド

0クリップ

投稿2018/01/21 16:10

編集2018/01/22 01:59

###前提・実現したいこと
出席番号と氏名を入力し名簿を作るプログラムですが、どこが間違っているのかわかりません。main文はmeibo20を読んでいるだけです。
###発生している問題・エラーメッセージ
キーボードで'I'を入力後、学籍番号が表示され、文字を入力できますが、そのあとの名前が文字入力が終わり、エンターを押すとプログラムが止まり、アプリケーションエラーになります。あと'R'を押して文字入力し、そのテキストフォルダがあるはずなのにアプリケーションエラーをはかれ、終了してしまいます。

###該当のソースコード

#include <stdio.h> #include <string.h> void *malloc(size_t size); void free(void *ptr); typedef struct person { char *id; char *name; } Person;
char *my_gets6(){ static int n; static char *s=NULL; if(s==NULL){ n = 10; s = (char *)malloc(n); } int i; for(i = 0; i < n; i++){ char c = getchar(); if(c == '\n'){ s[i] = '\0'; break; } if(i == n - 1){ s[i] ='\0'; n *= 2; char *s2 = (char *)malloc(n); strcpy(s2,s); free(s); s = s2; i = strlen(s); } s[i] = c; } char *s3 = (char *)malloc(strlen(s)+1); strcpy(s3,s); free(s); return s3; }
char *my_fgets6(FILE *fp){ static int n; static char *s=NULL; if(s==NULL){ n = 10; s = (char *)malloc(n); } int i; for(i=0;i<n;i++){ int c = fgetc(stdin); if(c == EOF){ return NULL; } if(c == '\n'){ s[i] = '\0'; break; } if(i == n - 1){ s[i] ='\0'; n *= 2; char *s2 = (char *)malloc(n); strcpy(s2,s); free(s); s = s2; i = strlen(s); } s[i] = c; } char *s3 = (char *)malloc(strlen(s)+1); strcpy(s3,s); free(s); return s3; }
static char *get_file_name(){ printf("ファイル名="); fflush(stdout); char *file_name= my_gets6(); if(strlen(file_name) == 0){ free(file_name); return NULL; } return file_name; } static Person *get_person(){ Person *p=(Person *)malloc(sizeof(Person)); printf("番号="); fflush(stdout); p->id=my_gets6(); if(strlen(p->id)==0){ free(p); return NULL; } printf("名前="); fflush(stdout); p->name =my_gets6(); return p; } static void put_persons(Person **ps, int n){ int i; for (i = 0; i < n; i++) printf("%s %s\n", ps[i]->id, ps[i]->name); }
void meibo20(void) { Person **array = NULL; int array_size = 10; int next_idx = 0; char *file_name; FILE *fp; array = (struct person **) malloc(sizeof(Person *) * array_size); while (1) { char command[2]; printf("コマンドを入力してください="); fflush(stdout); my_gets1(command, 2); switch (command[0]) { Person *workp; case 'I': ; workp = get_person(); if (workp == NULL) { break; } int i; for (i = next_idx - 1; i >= 0; i--) { if (strcmp(array[i]->id, workp->id) < 0) { break; } array[i + 1] = array[i]; } array[i + 1] = workp; next_idx++; break; case 'X': printf("処理を終了します。 \n"); return; case 'W': file_name = get_file_name(); fp = fopen(file_name, "w"); if(fp == NULL){ printf("ファイル %s を作成できませんでした。\n",file_name); continue; } for (i = 0; i < next_idx; i++) { fprintf(fp, "%s\n", array[i]->id); fprintf(fp, "%s\n", array[i]->name); } fclose(fp); break; case 'R': file_name = get_file_name(); fp = fopen(file_name, "r"); if(fp == NULL){ printf("ファイル %s が存在しません。\n", file_name); continue; } while (1) { workp = (Person *) malloc(sizeof(Person)); if((workp->id = my_fgets6(fp)) == NULL){ free(workp); break; } workp->name = my_fgets6(fp); for (i = next_idx - 1; i >= 0; i--) { if (strcmp(array[i]->id, workp->id) < 0) { break; } array[i + 1] = array[i]; } array[i + 1] = workp; next_idx++; break; } break; default: printf("不正なコマンドです。 \n"); break; } } }

###試したこと
my_gets6を読んでいるところをmy_gets1にして出力しても同じエラーになり、case内での間違いを探しても私の力が及ばないせいか見つかりませんでした。どなたかわかる方教えてくださると幸いです。よろしくお願いします。

###補足情報(言語/FW/ツール等のバージョンなど)
c
エクリプス最新

気になる質問をクリップする

クリップした質問は、後からいつでもMYページで確認できます。

またクリップした質問に回答があった際、通知やメールを受け取ることができます。

バッドをするには、ログインかつ

こちらの条件を満たす必要があります。

cateye

2018/01/21 16:58 編集

私の環境(centos,clang,gcc)では、コンパイルエラーになりますが、コンパイル出来てますか?
ping_100

2018/01/21 16:59

プログラミング自体始めた時期が最近とはいえ、誤字、このサイトの利用方法を読み飛ばしてしましました。自己解決できないのに他者への力を借りようとしているにもかかわらず大変失礼なことをしてしまいました。申し訳ありません。
ping_100

2018/01/21 17:13

コンパイルはできています。環境は変えたところはリソースのテキスト・ファイル・エンコードをMS932に変えてるだけです。
cateye

2018/01/21 21:53

環境の問題でしょうか?このようなエラーが出ます。tst.c:130:29: error: subscripted value is not an array, pointer, or vector printf("%s %s\n", ps[i]->id, ps[i]->name);
guest

回答2

0

ベストアンサー

直接的な原因は、 asm さんの仰るとおりで free(s); にあると思われます。これからも使い回す領域を解放してしまうと解放済み領域にアクセスすることになりますし、関数が走るたびに何度も何度も同じ領域を解放することになります。後半は二重解放と言って、やってはいけないことです。 free() した後はポインタを NULL にしておくようにすれば、とりあえずその次の free() では何もしませんから、多重解放は免れることができます。

そこを修正すればとりあえず落ちなくはなると思いますが、それ以外にも全体的にメモリの解放忘れが目立ちます。もしかしたら質問者さんはそのことを分かった上で簡潔さのためわざと省いたのかもしれませんが、その場合は以下を無視してくださって構いません。でもせっかくなので、ぱっと気付いた範囲で丁寧に説明してみることにしました。それでもたぶん抜けや想定外があると思うので、あしからず。

C++ならスマートポインタやデストラクタといった機能に頼れますし、ガベージコレクタを持つ言語ならまったく意識もしなくてよいのですが、Cは素直な子ですからね。

my_gets1()

まず、meibo20()関数内で呼び出している関数 my_gets1() が定義されていません。貼り忘れでしょうか?

とりあえず呼び出され方から、第一引数に与えられたバッファに、第二引数に指定されたサイズだけ標準入力から読み込むという処理のようなので、次のように実装してみました。

ちなみにこの関数は、サイズオーバー時には文字列を NULL 終端しません。

c

1void my_gets1(char *buf, size_t n) { 2 for (int i = 0; i < n; i++) { 3 buf[i] = getchar(); 4 if (buf[i] == '\n') { 5 buf[i] = '\0'; 6 break; 7 } 8 } 9}

my_gets6()

つづいて、my_gets6()を見ていきましょう。

コードを見る限り、sを内部用バッファとして使い回して利用する方針のようですね。sに読み込んだ後で、s3として確保する別領域にコピーし、コピーを返すという動作のようです。確かに内部バッファとして大きな領域を持っておけば、とりあえずサイズをオーバーするまではバッファの拡張を行わなくて済みます。まず、sを使い回すとして話をします。

この場合には、sを次の呼び出し以降も使い回すのですから、関数内で当然free(s);してしまってはダメです。

問題は、この関数内のsを解放するタイミングがなくなってしまうことです。当然、このまま放置するといわゆるメモリリークを起こします。実際問題としてはよほど重要なプログラムでなければ、リークしても終了時にOSが回収してくれてさほどの問題にはなりませんが、しかしメモリリークは避けたいです。この方針で行うならば引数に終了前フラグ変数でも用意して、そのフラグが立っていればsを解放する処理を追加するなど、何らかの対策を講じなければいけません。

メモリ確保はユーザーの入力待ちに比べれば圧倒的にコストが小さいので、この場合はsを使い回さない方針を採る方が楽だと思います。nsから static を外して、最後にs3にコピーするのをやめてそのままsを返すのです。これならば確保したメモリが全て関数外に委ねられるので (関数外できちんと解放すれば) 先の問題は生じません。

ということで、この先ではsを使い回すのをやめ、そのまま返すように実装したとします。

my_fgets6(FILE *fp)

上から順番に見ていきます。この関数内ではせっかく引数にファイルポインタを取っているのに、int c = fgetc(stdin); してしまっているせいでファイルから読み込めなくなってしまっています。stdinfpに直しましょう。

また、それ以外の点は my_gets6() 関数と同一です。このように共通の動作がある場合、できるかぎり同じコードを繰り返す(コピペする)ことは避けるべきです。今回のように共通部分の修正が入ったとき、片方の修正を忘れてしまったり、一部しか修正できていなかったりと余計な混乱を招く元になります。

もちろん最初はコピペでとりあえず動くものを作ることも大切です。でも、ある程度進めた段階でコードを整理するようにしましょう (この作業をリファクタリングといいます) 。

都合のいいことに、stdinもファイルポインタなので、my_gets6()関数はmy_fgets6()関数にstdinを渡したときと同じ動作をします。なので、次のようにまとめることができます:

c

1char *my_gets6() { 2 return my_fgets6(stdin); 3}

ただし、今はmy_gets6()my_fgets6()の前に定義されているので、my_gets6()を定義する段階ではmy_fgets6()が見えていない状態です。見える状態にするにはmy_gets6()の前にmy_fgets6()のプロトタイプ宣言を入れるか、2つの関数の定義順を入れかえてください。

ファイルの先頭に全関数のプロトタイプ宣言を入れてもいいかもしれません。それなりの規模のプログラムになると、ヘッダファイルに公開したい全関数のプロトタイプ宣言をずらっと並べて、ソースファイルではそれを#includeします。

動的配列 array の扱い

最後の〆として終了前に解放をしてあげましょう。case 'X':のところですね。ここで、確保した文字列と Person と、その配列自体のメモリを解放します。配列を順に見ていきつつidname、Person、最後に配列の順で解放しましょう。

c

1for (int i = 0; i < next_idx; i++) { 2 free(array[i]->id); 3 free(array[i]->name); 4 free(array[i]); 5} 6free(array);

file_name の解放

まだあります。file_nameの解放を忘れてはいけません。これはcase 'W':case 'R':内にあります。それぞれ break; の前に追加してあげましょう。忘れがちなのがfp == NULL だった場合に早期 continue; しているところです。ここでもcontinue の前に file_name を解放してやる必要があります。

ちなみに case 'R': の最後、fclose(fp);し忘れていますので、これもつけましょう。念のため取得した順の逆順で解放するとよいです。つまり、fclose →free(file_name) → break; です。

おつかれさまでした。C言語での動的確保はとても面倒ですね。

投稿2018/01/21 23:58

Eki

総合スコア429

バッドをするには、ログインかつ

こちらの条件を満たす必要があります。

ping_100

2018/01/22 00:44

回答ありがとうございます。my_gets1関数はエラーには直接関係ないと省いていましたが呼んでいるところを消し忘れました。申し訳ありません。。。丁寧な説明、間違いを沢山指摘してくださり、勉強不足が大いにわかりました。なのでベストアンサーにさせていただきました。本当に助かりました。
guest

0

// これを付けておくとmallocの戻り値をキャストする必要がなくなります (C言語のみ // C++の場合は必要 #include <stdlib.h>
if(i == n - 1){ s[i] ='\0'; n *= 2; char *s2 = (char *)malloc(n); strcpy(s2,s); free(s); s = s2; i = strlen(s); }

if(i == n - 1) s = realloc(s, n*= 2);

で充分

char *s3 = (char *)malloc(strlen(s)+1); strcpy(s3,s); free(s); return s3;

sがfreeされているのにNULLクリアされていないので次回の呼び出しで解放された領域を使ってしまいます。
free(s); はいらないのでは?
もしくは、nsからstaticを外す。

投稿2018/01/21 17:31

asm

総合スコア15147

バッドをするには、ログインかつ

こちらの条件を満たす必要があります。

ping_100

2018/01/22 00:41

回答ありがとうございます。とても見やすい文で分かりやすかったです。お陰様でエラー回避できました。勉強不足ですいません。
guest

あなたの回答

tips

太字

斜体

打ち消し線

見出し

引用テキストの挿入

コードの挿入

リンクの挿入

リストの挿入

番号リストの挿入

表の挿入

水平線の挿入

プレビュー

15分調べてもわからないことは
teratailで質問しよう!

ただいまの回答率
85.50%

質問をまとめることで
思考を整理して素早く解決

テンプレート機能で
簡単に質問をまとめる

質問する

関連した質問