What's the correct way to do a 'catch all' error check on an fstream output operation?

Question

What's the correct way to check for a general error when sending data to an fstream?

UPDATE: My main concern regards some things I've been hearing about a delay between output and any data being physically written to the hard disk. My assumption was that the command "save_file_obj << save_str" would only send data to some kind of buffer and that the following check "if (save_file_obj.bad())" would not be any use in determining if there was an OS or hardware problem. I just wanted to know what was the definitive "catch all" way to send a string to a file and check to make certain that it was written to the disk, before carrying out any following actions such as closing the program.

I have the following code...

int Saver::output()
{
    save_file_handle.open(file_name.c_str());
    if (save_file_handle.is_open())
    {
        save_file_handle << save_str.c_str();

        if (save_file_handle.bad())
        {
            x_message("Error - failed to save file");
            return 0;
        }

        save_file_handle.close();

        if (save_file_handle.bad())
        {
            x_message("Error - failed to save file");
            return 0;
        }

        return 1;
    }
    else
    {
        x_message("Error - couldn't open save file");
        return 0;
    }
} 

Answer 1

A few points. Firstly:

save_file_handle

is a poor name for an instance of a C++ fstream. fstreams are not file handles and all this can do is confuse the reader.

Secondly, as Michael pints out, there is no need to convert a C++ string to a C-string. The only time you should really find yourself doing this is when interfacing with C-style APIS, and when using a few poorly designed C++ APIs, such as (unfortunately) fstream::open().

Thirdly, the canonical way to test if a stream operation worked is to test the operation itself. Streams have a conversion to void * which means you can write stuff like this:

if ( save_file_handle << save_str ) {
   // operation worked
}
else {
   // failed for some reason
}

Your code should always testv stream operations, whether for input or output.

Answer 2

Everything except for the check after the close seems reasonable. That said, I would restructure things slightly differently and throw an exception or use a bool, but that is simply a matter of preference:

bool Saver::output()
{
    std::fstream out(_filename.c_str(),std::ios::out);
    if ( ! out.is_open() ){
         LOG4CXX_ERROR(_logger,"Could not open \""<<filename<<"\"");
         return false;
    }

    out << _savestr << std::endl;
    if ( out.bad() ){
         LOG4CXX_ERROR(_logger,"Could not save to \""<<filename<<"\"");
         out.close();
         return false;
    }

    out.close();
    return true;
}

I should also point out that you don't need to use save_str.c_str(), since C++ iostreams (including fstream, ofstream, etc.) are all capable of outputting std::string objects. Also, if you construct the file stream object in the scope of the function, it will automatically be closed when it goes out of scope.