- ベストアンサー
期待する文字列
フリーウェアのBorland c++compiler 5.5で 動的メモリーを確保して、期待する文字列(実数文字)が入力されるまで 繰り返すというプログラムを作っています。 以下のプログラムだけだと正常に動作しますが、その関数をループさせて 期待する文字列までループさせると数字のみなら実効可能ですが、 数字以外の文字が混じって15文字以上になると「不正な処理~~~」と強制終了させられます。 以下が そのプログラムの考え方です。 1 関数 A() 動的メモリーの確保 2へ 2 関数 B() 文字の読み込み(必要に応じてメモリー拡張) 3へ 3 関数 C() '.'個数チェック 2=エラー表示 1=小数点の書き換え(数字) 4へ 0= 4へ 4 関数 D() 関数isdigitで各メモリーをチェック 無限ループを使って int i=1; while(i) { 関数 A() }iのアドレス(ポインタ)を4まで引数にして iの値を0に書き換えループから脱出させています c言語を勉強して2週間の初心者です。どこがいけないのか 分かりません。 どなたか教えてください。
- みんなの回答 (6)
- 専門家の回答
質問者が選んだベストアンサー
ソース全部読みました。 まぁ2週間という初心者にしては上出来でしょう。←元プログラミング講師の経験より 一言いうなら、これっぽっちの要件なのに、問題を大きくし過ぎです。 もっと問題を整理してから取り組んで下さい。 とりあえずこのプログラムが直ったら、再設計して1から作り直してみましょう。 さて問題のバグですが、原因はやはり、reallocのサイズが拡張されていないことです。 関数expand_memory内の変数sizeがauto変数として宣言されているので、これをstatic宣言すれば直るでしょう。 (intの前にstatic を付けるだけです。) では、なぜ「数字以外が混じって~」だとダメになるのかというと、これは2度目のmalloc(NULLのrealloc)が正常に行えないからです。 数字以外が含まれていると再度入力させるためにNULLでrealloc(malloc)しようとしますが、それまでの動作でmallocが必要としている次のメモリブロックのヘッダまで壊されてしまっているため、mallocが不正な処理を引き起こすのです。 (同じ理由で、15文字以上の小数点を2個以上含む数字列を入力した場合でも、不正な処理が出るはずです。) ※それまで何度も実行されているはずのreallocは、同じメモリブロックのヘッダを参照するのでこの問題は起きませんが、たまたま他のエラーが出ていないだけであることを、よく意識してください。 下記に参考になりそうなページを挙げておきます。
その他の回答 (5)
関数名の動詞・名詞の並び順を統一されると良いのではと思います。 reallocするのでしたら、誰かがサイズを保持していなければうまくいきません。 例えば、 char *input(int *option) { char *str = NULL; int index = 0; int alloc_size = 0; for (;;) { if (index >= alloc_size) { alloc_size += 5; str = realloc(str, alloc_size); } index++; } } でも、エラーでリトライさせるのならば、誰かがfreeしてあげないとよろしくないので、基本的な構造を再考する必要がありそうです。 malloc/realloc/freeは使い方が比較的むずかしいので、固定サイズの配列で、入力文字数制限をするというのも1つのやり方だと思います。 引数に構造体へのポインタを使ってみるとか、戻り値をエラーコードにしてみるとかも試してみてはどうでしょう。
お礼
励ましのお言葉とアドバイスありがとうございました。 いろいろ試してみたいと思います。
- KojiS
- ベストアンサー率46% (145/312)
何となく設計せずにいきなり書いたコードに見えます。 まずアルゴリズムを考えて、設計をしましょう。 あと、これワーニングは出ていませんか? ワーニングが出ていると思うのですが、そのワーニングの原因を調べましょう。もし出ていないなら、ワーニングレベルが低いと思われるので、ワーニングレベルは常に最高にしておきましょう。 これは「数字のみなら実行可能」と書かれていますが、正常に動作しているでしょうか?見たところ正常に動作しないと思われます。 例えば、 char *expand_memory(char *buff, int *rem) ですが、バッファサイズが変わりません。ですので、意図する動作がつかめません。 getchar()の使い方を間違っていませんか? また、全体的にエラーチェックがほとんどありません。 その他もいろいろあるのですが、コーディングスタイルには触れません。が、言わせてもらうと、呼び出された関数の内部でエラー表示はやめた方が良いと思います。標準関数のように、エラーを返して呼出元でエラー表示しましょう。また、必要のない「ポインタの引きずり回し」はやめた方が良いでしょう。データの流れがわかりにくくなります。 もう一度全体をきちんと設計し直した方がよいかと思います。
お礼
そうですね、設計も何もなく処理の順序をただ書いただけだと 思いました。 ポインタも、ループを脱出するためのもので そこまでの関数には関係ないのに引きずってるなとは思っていました。 kojiSさんのご指摘参考になりました。ありがとうございます。
- KojiS
- ベストアンサー率46% (145/312)
> 考え方がおかしかったのでしょうか? > 関数の使い方は合ってると思うんですが。 > 関数の使い方が間違っている、正確にはどのように動いているかを把握しきれていない、という感じです。 とりあえず、実際にどのようにコーディングしているかを、要所を抜粋して書き込んでくだされば、何かアドバイスできるかもしれません。
補足
心強いお言葉ありがとうございます。 ソースファイル貼りつけましたので、ご面倒かけますがアドバイスお願いいたします。 #include <stdio.h> #include <stdlib.h> #include <string.h> #include <ctype.h> char *true_num(void); /* 期待数値入力関数宣言 */ char *input(int *); /* 数値入力関数宣言 */ char *expand_memory(char *, int*); /* バッファ拡張関数宣言 */ char *decimal(char *, int *); /* 小数点チェック関数宣言 */ char *num_check(char *, int*); /* 数字チェック関数宣言 */ int main() { printf("data\n"); printf("data\n%s\n",true_num() ); // 動作確認のため return 0; } /* 期待数値入力関数 */ char *true_num(void) { int *p_option, option = 1; char *number; p_option = &option; while(option) //期待する文字列が入力されるまでループ { number = input(p_option); } return number; } /* 数値入力関数 f(バッファ拡張関数) */ char *input(int *option) { char *str = NULL, *ent = "(enter)", // 改行のみのときenter表示のため *number; // realloc第1引き数にNULL指定でmallocとして機能 int index = 0, remainder = 0; // メモリーの残り変数 for(;;) { if(remainder == 0) str = expand_memory(str, &remainder); *(str + index) = getchar(); if( *str == '\n') { str = ent; // 改行のみ入力時、文字列エンター表示 break; } if( *(str + index) == '\n') // 文字列の終わりの書き込み { *(str + index) = '\0'; break; } index++; remainder--; } return decimal(str,option); } /* バッファ拡張関数 */ char *expand_memory(char *buff, int *rem) { const int SIZE = 5; int size = 0; size += SIZE; *rem = SIZE; buff= realloc(buff, size); return buff; } /* 小数点チェック関数 */ char *decimal(char *data,int *option) { int flag=0, posit; // 確認フラグ char *pflag, // '.'の位置 *number; pflag = data; while(flag < 2) // '.'の個数検索 { pflag = strchr(pflag + flag, '.'); if(pflag == NULL) break; else flag++; } switch(flag) { case 2: // 小数点2個以上 printf("set error!\a\n"); break; case 1: posit = strchr(data,'.') - data; // 小数点1個を含む文字列 *(data + posit) = '0'; // 小数点の書き換え isdigitでエラーになるため number = num_check(data,option); *(data + posit) = '.'; // チェック後データの復元 break; case 0: // 小数点を含まない文字列 number = num_check(data,option); break; } return number; } /* 数字チェック関数 */ char *num_check(char *data,int *option) { int len, flag, posit, *end; len = strlen(data); for(posit=0; posit < len; posit++) // 各配列の値を1~0か参照 { flag = isdigit( *(data + posit) ); if(flag == NULL) { printf("set error!\a\n"); break; } } if(flag != NULL) { end = option; // ここまでデーターが流れてくると文字列の実数になる *end = 0; // ループ脱出のための書き換え } return data; }
うーん、確かに抽象的すぎて・・・ 始めて2週間ということだから、間違いだらけで当然です。 そんなことは気にせずに、該当する部分のソース全部載せましょう。 1つの関数の大きさとか、関数のネストのさせ方、戻り値の使い方など、色々な意見を聞けるのではないかと思います。
- KojiS
- ベストアンサー率46% (145/312)
貴方の書かれた考え方だと、実際にどのように書かれているか全くわかりません。 ただし、この考え方の通りですと、グローバル変数を使って、alloc関連の関数を使って、なんだかんだとしているようですが、正常に動作する方がおかしいと思われます。(例え数字のみでも) C言語を勉強していると言うことですが、参考書は何をお使いですか? これが正常に動作しない原因がわからないということは、一度参考書(もしくはヘルプファイルでしょうか)の貴方が使った関数の部分を読み直した方が良いかと思われます。
補足
アドバイスありがとうございます。 考え方がおかしかったのでしょうか? 関数の使い方は合ってると思うんですが。
お礼
詳しい説明とURLまで教えていただき、本当にありがとうございます。 みなさんが言うように勉強して作り直したいと思います。 いろいろお時間を頂き感謝しております。