hnwの日記

feof関数でwhileループを回す奴はド素人

（2013-04-07 01:30追記）補足のコードに恥ずかしい間違いがあったのを修正しました。@nonakapさん、id:s-tomoさん、ご指摘ありがとうございます。
（2013-04-07 10:00追記）「補足その2」を書き足しました。
（2017-04-23追記）論旨がわかりにくい部分があったので、整理しました


stackoverflowの記事「“while( !feof( file ) )” is always wrong」をざっくり翻訳してみます。これはWilliam Pursellさんによる自作自演スレ（回答者も本人）で、Cでwhile( !feof( file ) )というループを作るのが悪い理由を説明するものです。

ちなみにPHPについてもほぼ同じことが言えますので、PHPプログラマの方にも一読をお勧めします（PHPの主要なファイル操作関数はCとほぼ同じインターフェースです）。もっとも、PHPはファイル操作が得意な言語だとは思えないので、他の言語で実現したり、そもそもファイル操作を使わずに済まないかを検討すべきだとも思います。

while( !feof( file ) )は常に悪い

最近 while( !feof( f )) に関する記事を見てるんだけど、なぜこれが悪いかを説明している良い文章へのリンクが見つけられない。だから、ここで説明しようと思う。

なぜこれが悪いかというと、（リードエラーがなかった場合に）コード作者の期待より1回多くループしてしまうからだ。また、リードエラーがあった場合には無限ループになってしまう。

この形のループを使うのが正しい状況もあるのかもしれないけど、僕には想像ができないし、僕が過去に見たこの形のループは100%間違っていた。さらに、文法的に正しかったとしても不愉快なほどに醜くなってしまい、非常に悪いコードになるので避けるべきだ。

次のようなコードを考えてみよう：

/* 警告: ダメなコードのデモ */

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

FILE *Fopen( const char *path, const char *mode );
int
main( int argc, char **argv )
{
    FILE *in;
    unsigned count;

    in = argc > 1 ? Fopen( argv[ 1 ], "r" ) : stdin;
    count = 0;

    /* 警告: これはバグ */
    while( !feof( in )) {  /* ここがダメ! */
        (void) fgetc( in );
        count++;
    }
    printf( "Number of characters read: %u\n", count );
    return EXIT_SUCCESS;
}

FILE *
Fopen( const char *path, const char *mode )
{
    FILE *f = fopen( path, mode );
    if( f == NULL ) {
        perror( path );
        exit( EXIT_FAILURE );
    }
    return f;
}

このプログラムはいつでも入力ストリームの文字数よりも1大きい数を表示する（リードエラーが無いとして）。入力ストリームが空だった場合について考えてみよう。

$ ./a.out < /dev/null
Number of characters read: 1

この場合、feof()は何のデータも読んでいないうちに呼ばれるので、falseを返す。ループに入ると、fgetc()が呼ばれてEOFを返す。そしてcountがインクリメントされる。次にfeof()が呼ばれてtrueを返し、ループが終了する。

この現象は常に起きる。feof()はストリームの読み込みがファイル末尾に出くわした後でないとtrueを返さない。feof()は次の読み込みでファイル末尾に到達するかどうかを調べる関数ではなく、リードエラーがあったのかファイル末尾に到達したのかを区別する関数なのだ。もしfread()が0を返したら、feofかferrorを使ってどちらの状況か判断をしなくてはならない。fgetcがEOFを返した場合も同様だ。feof()が有用なのは、freadが0を返した後かfgetcがEOFを返した後だけだ*1。こうした値を返していないうちはfeof()は必ずfalseを返す。

feof()を呼ぶ前に、読み込み（fread()でもfscanf()でもfgetc()でも）の返り値を常に検査する必要がある。

もっとひどい状況として、リードエラーが起きた場合についても考えてみよう。この場合、fgetc()はEOFを返し、feof()がfalseを返すので、無限ループになってしまう。while(!feof(p))を使う場合、少なくともループ内でferror()によるチェックが必要になる。もしくはループ条件をwhile(!feof(p) && !ferror(p))に置き換える必要がある。さもないと無限ループしてしまい、吐き出された様々なゴミデータで何らかの処理を行ってしまうかもしれない。

まとめると、while(!feof(f))が正解の状況がありえない、と確信を持って言うことはできないけど（その場合でも無限ループを防ぐためにリードエラーが起きていないかループ内でチェックする必要があるけど）、それでもほぼ間違いなく悪いコードだと言える。この形のループが間違いでない場合があったとしても、イディオムとして悪いので、そんなコードを書くべきではないだろう。こうしたコードを見た人は即座に「これはバグだ」と言うべきだ。もしかするとコードの作者をひっぱたくかもしれない（ただし、コード作者があなたの上司だった場合は慎重になった方が良いだろうけど）。

補足

元のページには「絶対ダメということはないはずだ、EOFに出くわすまでループさせたい可能性もあるんじゃないか」という意見も出ているんですが、具体的にそんな状況がありうるのかは疑問です。

ところで、上の文章はダメな理由の説明としては十分だと思うんですが、このWilliam Pursellさんが良いと思うコードが書いてないんですよね。そこで、僕ならこう書くかな？というコードを示しておきます。

    while (1) {
        int c;
        c = fgetc(in);
	if (c == EOF) {
            if (!feof(in)) {
                // エラー表示
            }
            break;
        }
        count++;
    }

ループ自体は無限ループにした上で、読み込み関数がエラーを返していたら直後でfeof()を使ってファイル終端かリードエラーかを区別し、breakでループを脱出します。両者を区別する必要がなければ次のようにシンプルに書けます。

    int c;
    while ((c = fgetc(in)) != EOF) {
        count++;
    }

補足その2

ブクマコメントを見ていると、「whileループの前でもfgetc()すればいいのではないか」という意見もあるように思いましたので、補足します。

whileループの前でもfgetc()するとしたら、下記のようなコードになるかと思います。

    int c;
    c = fgetc(in);
    while (!feof(in)) { /* 警告：リードエラーの対策がありません */
        count++;　/* 通常であれば、ここでもっと意味のある処理をするはず */
        c = fgetc(in);
    }

しかし、元記事でも指摘されている通り、これだとリードエラーがあった場合に無限ループしてしまいますので、ferror()の判定も追加する必要があります。

    int c;
    c = fgetc(in);
    while (!feof(in) && !ferror(in)) {
        count++;　/* 通常であれば、ここでもっと意味のある処理をするはず */
        c = fgetc(in);
    }

ところで、「既にファイル終端に到達しているわけでもないし、ファイル読み込みでエラーが出ているわけでもない」というのは回りくどい条件のように思います。「直前のファイル読み込みが成功したか」の方が直接的で書き手の意図が伝わりやすいコードなのではないでしょうか。

そう考えると、「補足」で紹介したファイル読み込み関数の返り値をループ継続の判定に使うコードに落ち着くように思います。

ファイルに関するループ処理は読み込み成功ごとに何かしたいことが多いはずなので、feof()だけをループの条件文に使うのはバグの元だという主張は一定の説得力があるように思います。

*1:訳注：fgets()など、他の関数についても同様です