std::ifstream setting fail() even when no error

Question

Using GCC 4.7.3 on Cygwin 1.7.24. Compiler options include: -std=gnu++11 -Wall -Wextra

I am working on a command line application and I needed to be able to load and save a set of strings so I wrote a quick wrapper class around std::set to add load and save methods.

// KeySet.h

#ifndef KEYSET_H
#define KEYSET_H

#include <cstdlib>
#include <sys/stat.h>
#include <cerrno>
#include <cstring>

#include <string>
#include <set>
#include <iostream>
#include <fstream>

inline bool file_exists (const std::string& filename)
{
/*
    Utility routine to check existance of a file.  Returns true or false,
    prints an error and exits with status 2 on an error.
*/
    struct  stat buffer;
    int     error = stat(filename.c_str(), &buffer);
    if (error == 0) return true;
    if (errno == ENOENT) return false;
    std::cerr << "Error while checking for '" << filename << "': " << strerror(errno) << std::endl;
    exit (2);
}

class KeySet
{
private:
    std::string             filename;
    std::set<std::string>   keys;

public:
    KeySet() {}
    KeySet(const std::string Pfilename) : filename(Pfilename) {}

    void set_filename (const std::string Pfilename) {filename = Pfilename;}
    std::string get_filename () {return filename;}
    auto size () -> decltype(keys.size()) {return keys.size();}
    auto cbegin() -> decltype(keys.cbegin()) {return keys.cbegin();}
    auto cend() -> decltype(keys.cend()) {return keys.cend();}
    auto insert(const std::string key) -> decltype(keys.insert(key)) {return keys.insert(key);}
    void load ();
    void save ();
};

void KeySet::load ()
{
    if (file_exists(filename)) {
        errno = 0;
        std::ifstream   in (filename, std::ios_base::in);

        if (in.fail()) {
            std::cerr << "Error opening '" << filename << "' for reading: " << strerror(errno) << std::endl;
            exit (2);
        }

        std::string     token;
        if (token.capacity() < 32) token.reserve(32);

        while (in >> token) keys.insert(token);

        if (!in.eof()) {
            std::cerr << "Error reading '" << filename << "': " << strerror(errno) << std::endl;
            exit (2);
        }

        in.clear(); // need to clear flags before calling close
        in.close();
        if (in.fail()) {
            std::cerr << "Error closing '" << filename << "': " << strerror(errno) << std::endl;

            exit (2);
        }
    }
}

void KeySet::save ()
{
    errno = 0;
    std::ofstream   out (filename, std::ios_base::out);

    if (out.fail()) {
        std::cerr << "Error opening '" << filename << "' for writing: " << strerror(errno) << std::endl;
        exit (2);
    }

    for (auto key = keys.cbegin(), end = keys.cend(); key != end; ++key) {
        out << *key << std::endl;
    }

    out.close();
    if (out.fail()) {
        std::cerr << "Error writing '" << filename << "': " << strerror(errno) << std::endl;
        exit (2);
    }
}

#endif

//

Here's a quick program to test the load method.

// ks_test.cpp

#include "KeySet.h"

int main()
{
    KeySet          test;
    std::string     filename = "foo.keys.txt";

    test.set_filename(filename);

    test.load();

    for (auto key = test.cbegin(), end = test.cend(); key != end; ++key) {
        std::cout << *key << std::endl;
    }
}

The data file just has "one two three" in it.

When I go to run the test program, I get the following error from my test program:

$ ./ks_test
Error closing 'foo.keys.txt': No error

Both cppreference.com and cplusplus.com say that the close method should set the fail bit on error. The save method works fine, and the load method works correctly if I comment out the error check after the close. Should this really work or have I misunderstood how close is supposed to work? Thanks in advance.

Edited to clarify, fix typo's and adjust code per Joachim Pileborg's and Konrad Rudolph's comments.

Edited to add solution to the code.

Answer 1

As it turns out, the close method for ifstream (and I assume all other IO objects) DOES NOT clear the error flags before closing the file. This means you need to add an explicit clear() call before you close the stream after end of file if you are checking for errors during the close. In my case, I added in.clear(); just before the in.close(); call and it is working as I expect.

Answer 2

You have two errors here: The first is about how you do your reading, more specifically the loop for reading. The eof flag will not be set until after you tried to read and the read failed. Instead you should do like this:

while (in >> token) { ... }

Otherwise you will loop one time to many and try to read beyond the end of the file.

The second problem is the one you notice, and it depends on the the first problem. Since you try to read beyond the end of the file, the stream will set failbit causing in.fail() to return true even though there is no real error.