fgetsとstrcmp

[tomo.xxx]

こんばんは
tomo.xxxです。

コンソールに担当者名を入力してコンソールに担当者名に"end"と打ち込まれるまでテキストファイルに担当者名を書いていくというプログラムをC言語で作りました。

下のプログラムの26行目のようにif(strcmp(tantou, "end\n") == 0)と書くと、このプログラムは自分の思ったように動作する(endをコンソールに打ち込み、リターンするとプログラムが終了する)のですが、if(strcmp(tantou, "end") == 0)と書くとendとコンソールに打ち込み、リターンしてもループを回り続けるプログラムになりました。

自分はfgetsの仕様の問題だと思っているのですが、どなたかこの理由を詳しく教えていただけませんか?



実行コマンド
./a.exe filename(テキストファイル名) mode(モード)

コード:

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

void input_tantou();
void write_file();

int main(int argc, char *argv[])
{
  FILE *fp;
  int i,j;
  char *tantou;

  fp = fopen(argv[1], argv[2]);
  if(fp == NULL){
    printf("Error!!\n");
    exit(1);
  }

  fprintf(fp, "tantou\n");
  fprintf(fp, "------------\n");
  
  while(1){
    // 担当者名入力
    input_tantou(tantou);
    if(strcmp(tantou, "end\n") == 0){
      break;
    }
    else{
      // 担当者をテキストファイルに記入
      write_file(tantou, fp);
      printf("%s", tantou);
    }
    fflush(stdin);
  }

  fclose(fp);
  return 0;
}

// 担当者の名前を入力
void input_tantou(char *tantou)
{
  printf("tantou:");
  fgets(tantou, 20, stdin);
}

// ファイルに担当者の名前を書き込む
void write_file(char *tantou, FILE *fp)
{
  fprintf(fp, "%s\n", tantou);
}

[box]

fgets()の仕様として、入力時の改行コード '\n' を含めることになっています。
ところで、

コード:

void input_tantou();
void write_file();

プロトタイプ宣言を書くのであれば、引数の型も書く必要があると思います。
今のままでもコンパイルエラーは出ないのでありましょうが、
結局引数のところに何も書いてないのでノーチェック。
ていうか、関数定義の1行目をコピペしてセミコロンを付ければおしまい。
私は楽をしたいので、そういう風にします。

次は、大問題ありの予感。

コード:

  char *tantou;

tantouが指している領域の大きさと

コード:

  fgets(tantou, 20, stdin);

ここの20とが（おそらく）食い違っているので、バッファーオーバーフローしそうです。

コード:

    fflush(stdin);

標準入力に対してfflush()を実行するのは、未定義動作であると思います。
出力ストリームの内容をすべて吐き出す、というのが本来の使い方のはず。

[ただの屍のようだ]

ポインタと配列をごちゃごちゃにしてないかが疑問に思います。
ついでに、ためしに一度21バイト以上入力してみてください

[tomo.xxx]

boxさん
返信ありがとうございます。

・fgetsの改行について
ありがとうございます。

・プロトタイプ宣言について
プロトタイプ宣言は今回はこれでも動くのかなと思って引数を書かずに動かしてみました。
普段は書いているのですが、気を付けます。

・fgets(fp, 20, stdin)について
ここなのですが、例えばchar str[20]と宣言して、fgets(fp, sizeof(str), stdin)ならば20バイト分入力できるのは分かるのですが、ポインタで宣言した場合fgetsのサイズを指定する部分はどのように書けばよいのでしょうか？

・fflushについて
使用しない方がよいということですね。
fgetsで入力した文字列がtantouに残ったままになるのでは？と思いfflushを付けたのですが、これ以外の方法でtantouの中身を消すにはどのような方法があるか教えていただけませんか？

[tomo.xxx]

ポインタと配列をごちゃごちゃにしてないかが疑問に思います。
ついでに、ためしに一度21バイト以上入力してみてください

ただの屍のようださん
返信ありがとうございます。

21バイト以上入力してみたのですが、コンソールには入力した文字とは違う文字が含まれた文字列が表示されました。

しかし、txtファイルの中身は19文字で改行され、自分が打ち込んだものが全て入力されている状態でした。

↓実行結果（適当ですみません）
$ ./a.exe a.txt w
tantou:dsfjaohonfklsdaflwoienofnlfjd
dsfjaohonfklsdaflwotantou:ienofnlfjd
tantou:end

↓a.txtの中身
tantou
------------
dsfjaohonfklsdaflwo
ienofnlfjd

[box]

・fgets(fp, 20, stdin)について
ここなのですが、例えばchar str[20]と宣言して、fgets(fp, sizeof(str), stdin)ならば20バイト分入力できるのは分かるのですが、ポインタで宣言した場合fgetsのサイズを指定する部分はどのように書けばよいのでしょうか？

まずは、mallocか何かで、例えば20バイトの大きさの領域を確保します。
その確保が成功すれば、tantouが指している領域は間違いなく20バイトありますので、
fgets()の第2引数に20を指定できます。

ただし、今のコードのように、tantouを定義している場所（main関数）と
実際の入力箇所（input_tantou関数）が別々である場合は、
input_tantou関数の引数として20を渡す必要があると思います
（tantouを配列で確保しても、mallocなどで動的に確保しても）。というのは、

コード:

void input_tantou(char *tantou)

という風に書いた場合、input_tantou関数が知ることができるのは
tantouの先頭位置だけであって、そこから何バイトの領域を指しているかは
（言語仕様として）知りようがないからです。なので、main関数で例えば

コード:

    char tantou[20];

と書いて（もしくはmallocなどで適切に20バイトを確保して）、

コード:

void input_tantou(char *tantou)
{
    fgets(tantou, sizeof(tantou), stdin);
}

などと書いてしまったら、sizeof(tantou)は20ではなくsizeof(char *)である（たぶん4バイト）から、
目も当たられない結果を招いてしまいます。

・fflushについて
使用しない方がよいということですね。
fgetsで入力した文字列がtantouに残ったままになるのでは？と思いfflushを付けたのですが、これ以外の方法でtantouの中身を消すにはどのような方法があるか教えていただけませんか？

tantouの中身を消すことを特に意識する必要はないと思います。
エンターキーを押すたびに毎回上書きします。それに、何のために消す必要があるのか、よくわかりません。
どうしても消したいと思うなら、例えば

コード:

    rewind(stdin);

のように書く手があるかもしれません（rewindとは、昔の記録メディアである磁気テープを巻き戻して、
読み書き位置を先頭に戻すイメージ。古っ！）が、適切な方法であるかどうかはわかりません。

[ただの屍のようだ]

char tantou[20];とした場合は余った分だけstdinに取り残されますが、
char *tantou;のままではCtrl+Cで強制終了しないといけないほどのエラーになります。（自分のMinGW44では）

[tomo.xxx]

boxさん
返信ありがとうございます。

まずは、mallocか何かで、例えば20バイトの大きさの領域を確保します。
その確保が成功すれば、tantouが指している領域は間違いなく20バイトありますので、
fgets()の第2引数に20を指定できます。

ただし、今のコードのように、tantouを定義している場所（main関数）と
実際の入力箇所（input_tantou関数）が別々である場合は、
input_tantou関数の引数として20を渡す必要があると思います

少し先程のコードを省略して書いてみましたが、main関数でmallocを使って20バイトの領域を確保しても、main関数ではない別の関数で入力する場合は意味がないということでしょうか？（下に書いたコードでは意味がない？）

コード:

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

void input_tantou(char *tantou);

int main(int argc, char *argv[])
{
  char *tantou;

  tantou = (char *)malloc(20);
  if(tantou == NULL){
    printf("Error!!\n");
    exit(1);
  }

  while(1){
    // 担当者名入力
    input_tantou(tantou);
    if(strcmp(tantou, "end\n") == 0){
      break;
    }
  }

  fclose(fp);
  free(tantou);
  
  return 0;
}

void input_tantou(char *tantou)
{
  printf("tantou:");
  fgets(tantou, 20, stdin);
}

tantouの中身を消すことを特に意識する必要はないと思います。
エンターキーを押すたびに毎回上書きします。それに、何のために消す必要があるのか、よくわかりません。
どうしても消したいと思うなら、例えば

コード:

    rewind(stdin);

のように書く手があるかもしれません（rewindとは、昔の記録メディアである磁気テープを巻き戻して、
読み書き位置を先頭に戻すイメージ。古っ！）が、適切な方法であるかどうかはわかりません。

一度fgetsとscanfをひとつのプログラムで同時に使ったときにfgetsが\nを含めるからか、2回あるキーボード入力(fgets:1回、scanf：1回)が思うようにできなかった記憶がありまして・・・

なので、tantouの中身を今回もクリアしないといけないのかと・・・

[tomo.xxx]

ただの屍のようださん
返信ありがとうございます。

char tantou[20];とした場合は余った分だけstdinに取り残されますが、
char *tantou;のままではCtrl+Cで強制終了しないといけないほどのエラーになります。（自分のMinGW44では）

自分はWindows7 64bitにCygwinをインストールしてgccコンパイル、実行しているのですが、やはり環境によっていろいろな操作をするということは、この書き方は不適切ということですね・・・