C言語 関数 文字列の反転について

Question

現在C言語の関数を学んでいる学生です。
ユーザーが入力した文字列の初めから４文字を反転して返す関数を作ろうと下のコードを書きました。

#include <stdio.h>

void hanten();

int main(void) {
	char s[80];
	gets(s);
	hanten(s);
	return 0;
}
void hanten() {
	int i, k;
	char s[80];

	k = 0;
	while (1) {
		if (s[k] == '\0')break;
		k++;
	}

	for (i = (k >= 3 ? 3 : k); i >= 0; i--) {
		printf("%c", s[i]);
	}
}

実行したところ

abcdef
ﾌﾌﾌﾌ続行するには何かキーを押してください . . .

といった具合に、どんな文字列を入力しても
フフフフとコンピュータに笑われてしまうのですが、これは何が原因なのでしょうか？
初歩的な質問だと思うのですが、解説頂けると嬉しいです。

Answer 1

main関数の中のsとhanten関数のなかのsはそれぞれ独立しており、別物だからです。hanten関数でのsには入力された文字列が入っているわけでは無く、char s[80];で初期化されたときの文字列(何が入るかはコンパイラや環境によって異なります。今回は毎回"ﾌﾌﾌﾌ"から始まる文字列だったというわけです。)が入っています。単純にそれが出力されただけです。

詳細な解説など

修正してみたコードを下記に示します。
http://ideone.com/f08zmt
このコードで修正した問題点をあげます。

1. マジックナンバー(80の部分)を使用すべきではない。

char配列のサイズを80と直接書いていますが、この書き方はマジックナンバーと言われ、どのようなプログラミング言語であっても推奨されません。マクロを使って定義することを推奨します。

#define BUFF_SIZE 80
...
  char s[BUFF_SIZE];

1. 関数宣言や定義で引数部分を省略(())を使用すべきではない。

C言語において、関数宣言・定義における仮引数部分が無い、つまり()と書くことは、「引数は何でもいい」といいという意味です。これはどのような引数をつけて関数を呼び出してもコンパイルエラーにはなりません。また、別のところで仮引数を定義しないと、その引数を使うこともできません。文字列を渡したいのなら、きちんと宣言・定義両方に書くべきです。逆に、引数を何も渡さないとするなら、(void)と明記してください。

void hanten(char *s);
...
void hanten(char *s) {
    // sは仮引数として使うため、char s[80];をいれてはいけません。
    ...
}

1. getsを使用してはいけない。

C言語のgetsはセキュリティ上の問題があるため使用してはいけません。代わりにfgets等を使用してください。また、getsは最新のC言語仕様(C11)では廃止されています。
参照: Wikipedia: gets

    fgets(s, BUFF_SIZE, stdin);

1. 文字列の長さを知りたいならstrlenを使おう。

while部分でどこまで文字列があるかを確認しているようですが、標準関数に文字列の長さを調べるstrlenがあります。車輪の再開発はせずに、なるべく標準関数を使いましょう。

#include <string.h>
...
    k = strlen(s);
    for (i = (k >= 4 ? 3 : k - 1); i >= 0; i--) {
        ...

kには長さが入りますので、k - 1が最後の文字になることだけ注意してください。

Comments

[Takumi0106]

遅くなりましたが、わかりやすくまた、深い説明ほんとうにありがとうございます。
C言語の入門書を順にこなしているだけではわからないことが多くあることに気づかされました。以後、励みます。

Answer 2

まず，gets関数を使われていますが，これはバグを引き起こしやすく，現在の新しい規格では削除されているはずです．私の実行環境でも
”warning: this program uses gets(), which is unsafe.”と怒られてしまいます．（下のリンクにも例としてgets関数が紹介されています．）
なのでscanfを使いましょう．
次に，hanten関数ですが，引数を指定するのを忘れてしまっています！hanten関数の中でchar s[80]とまた定義されていますが，これも消しましょう．
ので以下のようにすると良いでしょう．

#include <stdio.h>

void hanten(char[]);

int main(void) {
  char s[80];
  scanf("%s", s);
  hanten(s);
  return 0;
}
void hanten(char s[]) {
  int i, k;

  k = 0;
  while (1) {
    if (s[k] == '\0')break;
    k++;
  }

  for (i = (k >= 3 ? 3 : k); i >= 0; i--) {
    printf("%c", s[i]);
  }
  printf("\n");
}

バッファオーバーフロー

Comments

[Takumi0106]

丁寧な説明、ありがとうございます。
gets関数はバグを引き起こしやすいというのは知らなかったので勉強になりました。

[raccy]

scanfで単純に"%s"とするのはgetsと同じ問題がおきます。単純な"%s"だけは絶対に使用しないでください。scanfを使用する場合は"%79s"など文字列に入るサイズを必ず指定してください。
参考: https://ja.wikipedia.org/wiki/Scanf#.E3.83.90.E3.83.83.E3.83.95.E3.82.A1.E3.82.AA.E3.83.BC.E3.83.90.E3.83.BC.E3.83.A9.E3.83.B3

[H_I_D]

すみません．ご指摘ありがとうございます．

Answer 3

main関数でsを渡そうとしているのに、hanten関数の宣言に、引数がありません。

文字列を渡せるようにするには、こうします。

void hanten(char *s) {
    int i, k;
    /* char s[80]; */ /* これは要らない */
    /* 以下略 */
}

ﾌﾌﾌﾌとなるのは、何も入っていない＋未初期化の文字列sを処理しようとしているからです。
この文字は開発環境によって異なります。

これだけではまだ完成ではありませんが、残りは質問者さんがご自分でお試しになった方が良いでしょう。

Comments

[Takumi0106]

指摘頂いたところを直したところ、目的の出力を得ることができ、理解を深めることができました。有難うございました。