0

I have some data that I want to format and output, either in raw text file or .gz compressed text file.

Thus, I wanted to do something like this :

shared_ptr<ofstream> file = make_shared<ofstream>(filename.c_str());
shared_ptr<ostream> stream;
if (compressed) {
    stream = getCompressedStream(*file);
} else {
    stream = std::move(file);
}

for (auto c = data.begin(); c != data.end(); ++c) {
    *stream << **c << "\t" << (*c)->getThing() << endl;
}

With getCompressedStream a function that decrypt the stream and return a new non encrypted stream :

std::unique_ptr<std::ostream> getCompressedStream(std::ofstream& file)
{
    typedef io::filtering_ostream filter;
    std::unique_ptr<filter> out = std::unique_ptr<filter> (new filter());
    out->push(io::gzip_compressor());
    out->push(file);
    return std::move(out);
}

So I wanted getCompressedStream to abstract calls to boost lib, so that I only use std streams in my main program.

It's not working : the .gz file is damaged/not readable.

According to this thread, the output of a filtering_stream is done in the destruction of the object. Thus, I don't know if it can be done cleanly with shared_ptr. I don't know whether file or stream is destroyed first, and I guess this leads to issues.

Do you think implementing getCompressedStream this way is possible? What would you change?

Thanks

Edit : It works if I switch to regular pointers instead of shared pointers, and delete the stream explicitely before the file.

bomzh
  • 101
  • 2
  • c++11 ofstream works with std::string, so `make_shared(filename)` should work. also no need to `std::move(out)`, just `return out` – billz Aug 13 '13 at 11:12

1 Answers1

0

I'm not sure. The real problem is that you never explicitly close the ofstream, so you have no idea as to what is going on. (Except in the case of exceptions, where you're going to delete the new file anyway, you must check the status of the ofstream object after the close, to know whether everything succeeded or not.) More generally, your solution seems overly complicated to me; you're trying to do too much in a single function. The way I usually do this (with my own filtering streambuf, which predate the boost versions by some years) is to do something like:

std::ofstream file( filename.c_str() );
if ( compressed ) {
    CompressedStreambuf c( file );
    outputData( file );
} else {
    outputDate( file );
}
file.close();
if ( !file ) {
    remove( filename.c_str() );   //  Since it isn't complete
    //  Ensure that program return code is failure.
}

Of course, my own filtering streambuf work slightly differently than those of boost: the constructor taking an ostream inserts the filter in front of the ostream, and the destructor flushes and removes the filter (a variant of RAII). But it shouldn't be too difficult to make a wrapper of the Boost classes to support this.

The advantage of this solution is that everything is guaranteed to be constructed and destructed in the correct, nested order, because there is no dynamic allocation which would allow an arbitrary order.

James Kanze
  • 150,581
  • 18
  • 184
  • 329
  • Basically, I can do the same as your snippet : I replace CompressedStreambuf by my function which will return a unique_ptr... Which is guaranteed to be destroyed at the end of the if (Compress). Right ? – bomzh Aug 13 '13 at 12:27
  • @tom_b Yes. But I'm less sure of the ordering here. Rather than use a function which returns a new stream, as you do, I'd create an object, whose constructor creates the filter objects and the stream as desired, and whose destructor cleans them up. (One important issue I've found with my own filtering streambuf is the necessity of ensuring that they are correctly flushed _before_ the final destination is destructed or closed. I don't know how the Boost streambuf's handle this.) – James Kanze Aug 13 '13 at 12:59