5

I recently wrote some code that I thought was okay, but a colleague says it was causing random crashes in our application. The offending code is writing to an unopened ofstream. My question is: should it be okay to write to an ofstream that has not been opened? This came about when the initialization of a class would not open its ofstream for logging debug information. But subsequent methods would still use the unopened ofstream. Here is an example:

class A {
  public:
    A(const std::string& fname) {
        if (!fname.empty()) {
            m_debug_log.open(fname.c_str());
        }
    }

    void DoSomething() {
        m_debug_log << "doing something useful now" << std::endl;
    }

  private:
    std::ofstream m_debug_log;
};

My colleague says that he stopped the random crashes by wrapping all output operations to m_debug_log with a validity check on the ofstream. So the output operations are performed only when m_debug_log is a valid output stream. 

    void DoSomething() {
        if (m_debug_log)
            m_debug_log << "doing something useful now" << std::endl;
    }

Of course writing to the stream only when it is valid makes the most sense. But I never expected that writing to the unopened ofstream would cause correctness problems. (Yes, it is inefficient, but coding speed was my highest priority at the time.)

I quickly searched but found nothing definitive about this. In particular I did not see anything explicit about undefined behavior when writing to an uninitialized ofstream. Should the initial implementation have been correct? My question is a general one rather than about a particular implementation. For what it is worth I routinely use VS 2010, VS 2013, Ubuntu 12.04, and Centos 6.3, and found no problems with initial testing. It was only when running for longer periods of time when the crashes occurred.

Phil
  • 5,822
  • 2
  • 31
  • 60
  • 6
    The lesson here is that if an operation can fail, you should always check for failure. – Some programmer dude Mar 05 '15 at 06:05
  • 2
    And if an operation doesn't make sense, such as writing to a closed stream, you shouldn't do it in the first place. – user207421 Mar 05 '15 at 06:05
  • 1
    I have used similar techniques for logging in the past and writing to a closed stream in and of itself can't lead to problems, because the streamstate is verified before any operation. If wasting a few cycles for that is too much, or if the debug output performs some expensive calculations, checking the streamstate is a good advice. I suggest you look for the error elsewhere. – Ulrich Eckhardt Mar 05 '15 at 06:10
  • Check this answer http://stackoverflow.com/questions/28499630/is-it-allowed-to-write-to-a-ofstream-when-it-is-not-opened-in-c – Tasos Vogiatzoglou Mar 05 '15 at 06:11
  • I honestly think that a properly initialized object should not crash when its functions are called with legal arguments. So a fully constructed (but not opened) `std::fstream` should still be stable. It would be interesting to find something explicitly mentioned in the stadard however. – Galik Mar 05 '15 at 06:13
  • That sounds more like undefined behaviour getting masked by the inserted checks, which is a terrible thing. I would revert the code and search for the actual cause, which unfortunately could be anywhere. – molbdnilo Mar 05 '15 at 07:17

1 Answers1

2

A default-constructed (unopened) std::ofstream becomes bad as soon as anything is written to it. Subsequent writes should silently (and safely!) fail.

Unless Microsoft's implementation is peculiar, I suspect the random crashes are coming from elsewhere. Make sure that the m_debug_log is fully constructed, even if only with the default constructor. Are you sure the real DoSomething is not a static method?

Bulletmagnet
  • 5,665
  • 2
  • 26
  • 56
  • What you said about silently and safely failing is what I expect should be true. It is not, however, what we experienced. The platform that exhibited the problem was not Microsoft but a special-purpose android-like OS, and the problem did not occur immediately. It took some time before crashing. But removing all writes to debug log made the crashes go away. Also note the problem is not in the code being written either. When we initialize it with a non-empty file name, the crashes go away too. – Phil Mar 06 '15 at 02:09