12
\$\begingroup\$

I have the following method for opening a file:

void TankFile::OpenForReading(const std::string & filename)
{
    assert(!filename.empty());

    errno = 0;
    file.exceptions(0); // Don't throw
    file.open(filename, (std::fstream::in | std::fstream::binary));

    if (!file.is_open() || !file.good())
    {
        const char * errorStr = strerror(errno);
        throw TankFileError(Format("Failed to open Tank file \"%s\": '%s'", filename.c_str(), errorStr));
    }
}

The objective here is to attempt to open a file and throw TankFileError with a proper error description on failure. The caller will be expecting this exception type.

Everything works fine and I get a nice error message like this if the exception is thrown:

Failed to open Tank file "unexistent_file": 'No such file or directory'

But what I don't like in that block is having to use the errno global and strerror().

A way around it would be to let the stream throw an exception, then catch it, get the error message from the what() member and re-throw with TankFileError, but I find this solution also a bit hackish, plus, in the tests I did, the resulting error message from std::fstream::failure was pretty cryptic:

void TankFile::OpenForReading(const std::string & filename)
{
    assert(!filename.empty());

    try
    {
        file.exceptions(std::fstream::failbit);
        file.open(filename, (std::fstream::in | std::fstream::binary));
    }
    catch (std::fstream::failure & err)
    {
        throw TankFileError(Format("Failed to open Tank file \"%s\": '%s'", filename.c_str(), err.what()));
    }
}

Produced the error message:

Failed to open Tank file "unexistent_file": 'ios_base::clear: unspecified iostream_category error'.

Is there a better way to implement this? I was hoping that the new C++11 system_error library would provide a way to query this kind of error messages, but from what I've seen, you still have to pass errno around to get an error string.

\$\endgroup\$
  • 7
    \$\begingroup\$ +1 This is a great question. I also noticed that the error messages returned by std::ios::failure are pretty useless. \$\endgroup\$ – jliv902 Jul 24 '14 at 19:00
4
\$\begingroup\$

I currently don't see any seamless alternative to using errno and ::strerror;.

#include <stdexcept>
#include <system_error>
#include <string>
#include <iostream>

#include <cstring> // strerror

#include <fstream>

#if defined(_LIBCPP_VERSION) && (_LIBCPP_VERSION >= 1000)
#define HAS_IOS_BASE_FAILURE_DERIVED_FROM_SYSTEM_ERROR 1
#else
#define HAS_IOS_BASE_FAILURE_DERIVED_FROM_SYSTEM_ERROR 0
#endif

using std::cerr;
using std::cout;
using std::endl;


int main(/*int argc, char** argv*/) {
    int rv = EXIT_SUCCESS;
    errno = 0;

    try {
        std::ifstream ifs;
        ifs.exceptions(std::ios::badbit | std::ios::failbit);
        ifs.open("DOESN'T EXIST");

    } catch (const std::ios_base::failure& e) {

#if (HAS_IOS_BASE_FAILURE_DERIVED_FROM_SYSTEM_ERROR)
        //
        // e.code() is only available if the lib actually follows iso §27.5.3.1.1
        // and derives ios_base::failure from system_error
        // like e.g. libc++
        // http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/ios?revision=193085&view=markup
        // (line 415)
        //
        // and not keeps on deriving it directly from runtime_error
        // like libstdc++
        // https://github.com/mirrors/gcc/blob/master/libstdc%2B%2B-v3/include/bits/ios_base.h#L209
        //
        cout << "libc++ error #"    << e.code().value()  
                << ' '              << e.code().message()
                << ',' << endl << "    ";
#endif
        cout << "libc error #"      << (rv = errno)
                << ": "             << ::strerror(errno)
                << endl;

        cout << "handled" << endl;
    } 

    return rv;
}

using the the new e.code() semantics yields (with clang 3.4 and libc++ 1101) only the first line of

libc++ error #1 unspecified iostream_category error,
    libc error #2: No such file or directory
handled

Live

So even if one's lucky enough to have a std:: implementation that cares about iso §27.5.3.1.1 and actually derives ios_base::failure from system_error, the msg generated is too poor to be presented to users.

The only thing to be discussed is how the libc messages are best incorporated into wrapper classes.

\$\endgroup\$
  • 3
    \$\begingroup\$ Errors should be written to an error stream (such as stderr) with either cerr or clog. \$\endgroup\$ – edmz Jul 26 '14 at 15:30
  • \$\begingroup\$ Unless it is the regular output of the program, as it is the case here. \$\endgroup\$ – Solkar Jul 26 '14 at 15:51
  • 1
    \$\begingroup\$ Indeed that is not regular output. Therefore, you should write it to a separated stream (I might redirect stdout to focus only on stderr to cleanly see whether the program succeded or not). \$\endgroup\$ – edmz Jul 26 '14 at 16:23
  • \$\begingroup\$ And indeed that is regular output of this program that is meant to test exceptions. \$\endgroup\$ – Solkar Jul 26 '14 at 16:28
  • 3
    \$\begingroup\$ Your answer/program is a test program, but @black is pointing out that the OP's code (which is what you should be reviewing - and that is what I meant when I said why do you think this is a test program) should write to stderr. \$\endgroup\$ – rolfl Jul 26 '14 at 17:24
2
\$\begingroup\$

I am afraid, no. iostreams are not designed for exceptions, which were added later, so you can easily understand why error messages are not helpful.
Moreover, you must explicitly ask for them in order to be thrown; otherwise, the default behavior is kept, that is querying the state of the stream.

The "modern" way is using <system_error> (as std::thread does, for example): as you have correctly pointed out, though, it isn't that higher level and modern.

The best solution is to use boost::filesystem which does throw exceptions, uses iterators and so on. Another solution might be wrapping it on your own, but it'd be worthless and I wouldn't suggest that unless required.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.