Should ifstream be opened in binary mode?

[lightvector]

It appears that zstr does not open input streams in binary mode (std::ios_base::binary):

explicit ifstream(const std::string& filename, std::ios_base::openmode mode = std::ios_base::in)
    : detail::strict_fstream_holder< strict_fstream::ifstream >(filename, mode),
      std::istream(new istreambuf(_fs.rdbuf()))

Although it does for the output stream:

explicit ofstream(const std::string& filename, std::ios_base::openmode mode = std::ios_base::out)
    : detail::strict_fstream_holder< strict_fstream::ofstream >(filename, mode | std::ios_base::binary),
      std::ostream(new ostreambuf(_fs.rdbuf()))

This might be a bug? When running on Linux, I haven't found any issues, but when compiling and running on Windows, non-binary mode appears to corrupt the bytes read in, presumably because in text mode it attempts to do some Windows-specific end-of-line character conversions. Adding std::ios_base::binary to the ifstream mode fixes this.

[lczech]

Hm, the issue that I see there is that if the input data is not in fact compressed, the stream might not work as expected, because it then is supposed to act like a normal istream, with the end-of-line conversions.

[lightvector]

Ah, glad to see a comment/reply. :)

Is there any way to get the best of both worlds? If not, then it seems like having it fail to do end-of-line conversions when reading a file that happens to not be compressed seems like the lesser issue. Because as it stands, on Windows the library typically fails to do it's job at all - be able to read compressed files, which is the whole point of using it.

[lczech]

Makes sense. In that case, a simple workaround would be to have a little helper function that first opens the file and tries to find the magic bytes. If found, continue to open the file with zstr; if not, open it with a normal ifstream.

bool is_gzip_compressed_file( std::string const& file_name )
{
    // Open the file in binary mode.
    std::ifstream infile;
    infile.open( file_name, std::ifstream::in | std::ifstream::binary );
    if( !infile.good() ) {
        return false;
    }

    // Get the first two characters. If this fails, the file is too short, so it is not a gzip file.
    unsigned char buffer[2];
    infile.read( reinterpret_cast<char*>( &buffer ), 2 );
    if( !infile.good() ) {
        return false;
    }
    infile.close();

    // Check if the file starts with the magic number of gz files.
    return magic = ( buffer[0] == 0x1f ) && ( buffer[1] == 0x8b );
}

That currently only does gzip, but can easily be extended to check the zlib magic bytes as well.

[sandsmark]

I'd say binary mode is the best default, I don't know in what case you would want something that probably is compressed to automatically get mangled line endings?

I'd say it's more surprising that the data is suddenly mangled, people that want the line ending stuff usually know about it and can pass the required flags.

[ferdymercury]

Could someone of you review #71 ?
Thanks!

[ferdymercury]

Default open binary mode lead to backward compatibility problems on Mac (ERGO-Code/HiGHS#793)

So I guess the only solution would be as mentioned below:

In that case, a simple workaround would be to have a little helper function that first opens the file and tries to find the magic bytes. If found, continue to open the file with zstr; if not, open it with a normal ifstream.

[LuNoX]

@ferdymercury I added proper header detection in #74, so a peek function should be very easy to implement