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

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

ただいまの
回答率

90.09%

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

解決済

回答 2

投稿 編集

  • 評価
  • クリップ 0
  • VIEW 853

ping_100

score 5

前提・実現したいこと

出席番号と氏名を入力し名簿を作るプログラムですが、どこが間違っているのかわかりません。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
エクリプス最新

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

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

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

    クリップを取り消します

  • 良い質問の評価を上げる

    以下のような質問は評価を上げましょう

    • 質問内容が明確
    • 自分も答えを知りたい
    • 質問者以外のユーザにも役立つ

    評価が高い質問は、TOPページの「注目」タブのフィードに表示されやすくなります。

    質問の評価を上げたことを取り消します

  • 評価を下げられる数の上限に達しました

    評価を下げることができません

    • 1日5回まで評価を下げられます
    • 1日に1ユーザに対して2回まで評価を下げられます

    質問の評価を下げる

    teratailでは下記のような質問を「具体的に困っていることがない質問」、「サイトポリシーに違反する質問」と定義し、推奨していません。

    • プログラミングに関係のない質問
    • やってほしいことだけを記載した丸投げの質問
    • 問題・課題が含まれていない質問
    • 意図的に内容が抹消された質問
    • 広告と受け取られるような投稿

    評価が下がると、TOPページの「アクティブ」「注目」タブのフィードに表示されにくくなります。

    質問の評価を下げたことを取り消します

    この機能は開放されていません

    評価を下げる条件を満たしてません

    評価を下げる理由を選択してください

    詳細な説明はこちら

    上記に当てはまらず、質問内容が明確になっていない質問には「情報の追加・修正依頼」機能からコメントをしてください。

    質問の評価を下げる機能の利用条件

    この機能を利用するためには、以下の事項を行う必要があります。

質問への追記・修正、ベストアンサー選択の依頼

  • ping_100

    2018/01/22 01:59

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

    キャンセル

  • ping_100

    2018/01/22 02:13

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

    キャンセル

  • cateye

    2018/01/22 06: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);

    キャンセル

回答 2

checkベストアンサー

+2

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

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

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

 my_gets1()

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

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

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

void my_gets1(char *buf, size_t n) {
    for (int i = 0; i < n; i++) {
        buf[i] = getchar();
        if (buf[i] == '\n') {
            buf[i] = '\0';
            break;
        }
    }
}

 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を渡したときと同じ動作をします。なので、次のようにまとめることができます:

char *my_gets6() {
    return my_fgets6(stdin);
}

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

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

 動的配列 array の扱い

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

for (int i = 0; i < next_idx; i++) {
    free(array[i]->id);
    free(array[i]->name);
    free(array[i]);
}
free(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/22 09:44

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

    キャンセル

+1

// これを付けておくと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/22 09:41

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

    キャンセル

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

  • ただいまの回答率 90.09%
  • 質問をまとめることで、思考を整理して素早く解決
  • テンプレート機能で、簡単に質問をまとめられる

同じタグがついた質問を見る