6
\$\begingroup\$

I want to show how to interact with C from C++ and chose a simple and small library to demonstrate. zlib's gzopen, gzwrite and gzclose looked like a reasonably closed set of funtions.

Does anyone see anything fundamentally wrong with my presentaion? The main code I present here is in class GzWrite.

Intro section:

#include <string>
#include <vector>
#include <fstream> // ifstream
#include <stdexcept> // runtime_error
#include <iostream> // cerr
// C-Headers:
#include <zlib.h> // gzXyz; sudo aptitude install libz-dev
#include <string.h> // strerror
#include <errno.h> // errno
#pragma GCC diagnostic ignored "-Wmissing-field-initializers"
using std::ifstream; using std::ofstream; using std::string; using std::vector;
namespace {

constexpr size_t BLOCK_SZ = 256*1024;
constexpr int LEVEL = Z_DEFAULT_COMPRESSION;

Core class

struct GzWrite { // RAII-Wrapper
    gzFile gz_ ; // C-Struct aus zlib.h
    explicit GzWrite(const string& filename)
        : gz_{gzopen(filename.c_str(),"wb9")} // 'w': write, 'b':binary, '9':level
    {
        if(gz_==NULL) throw std::runtime_error(strerror(errno));
    }
    ~GzWrite() {
        gzclose(gz_);
    }
    int write(const char* data, size_t len) {
        return gzwrite(gz_, data, len);
    }
    GzWrite(const GzWrite&) = delete; // no copy
    GzWrite& operator=(const GzWrite&) = delete; // no assign
};

Helper functions

vector<char> read(const string& infn) {
    ifstream inf{ infn, ifstream::binary };
    if(!inf) throw std::runtime_error("open error");
    inf.seekg(0, inf.end); // go to end of file
    const auto laenge = inf.tellg(); // current pos is file size
    if(laenge > 1024*1024*1024) throw std::runtime_error("<1GB please");
    inf.seekg(0, inf.beg); // back to begin
    vector<char> data(laenge); // make room
    inf.read(data.data(), laenge); // read all at once
    return std::move(data); // make doubly sure data is not copied
}

void pack(const string& infn, const string& outfn) {
    vector<char> indata = read(infn); // read input
    GzWrite gz{outfn}; // init output
    auto res = gz.write(indata.data(), indata.size());
    if(res==0) throw std::runtime_error("write error");
}

} // namespace

Main function:

int main(int argc, const char* argv[]) {
    try {
        const vector<string> fnamen {argv+1, argv+argc};
        for(auto fname : fnamen) {
            std::cout << "packing " << fname << "... ";
            pack(fname, fname+".gz");
            std::cout << fname << ".gz"<< "\n";
        }
    } catch(std::runtime_error exc) {
        std::cerr << "Error: " << exc.what() << "\n";
    }
}
\$\endgroup\$
  • \$\begingroup\$ Could you specify more precisely which kind of interaction of C and C++ you want to show? For example I am missing extern "C" {} which might be necessary for non C++ compatible C headers. \$\endgroup\$ – Nobody May 21 '14 at 11:40
  • \$\begingroup\$ a) In general how functions with pointers are called when you are used to objects and call-by-ref and call-by-value only. b) I assume the the C-header takes care of that (and zlib does), but you are right, I should mention it. \$\endgroup\$ – towi May 21 '14 at 11:43
6
\$\begingroup\$

This example is mostly about making and using a RAII wrapper, not about calling C from C++ (which is trivial).

If you want to show how to call C-like interfaces, then use zlib directly, without wrapping it.

If you want to show the native C++ way to wrap a C interface, then the wrapper should be an iostream subclass, like gzstream.

pack should read blocks of finite size rather than reading the whole file at once. It's a compression utility, so it's likely to be used on files that won't fit in memory!

Some confusing names:

  • GzWrite instances aren't writes; they're streams. How about GzStream or (if it uses the iostreams interface) gzostream?
  • Don't call a function read; it's easy to confuse with Posix read. This is traditionally called slurp or read_file.
  • infn will probably be interpreted as "in-function", not "in-filename". How about infile?
  • laenge (Länge?): length

BLOCK_SZ and LEVEL are unused.

\$\endgroup\$
  • 1
    \$\begingroup\$ gzstream as a lib is out, it does'nt seem to support windows. Yes, GzStream is a better name. You are right, I should use blocks, I chose whole-file for brevity; I will think about it. Actually the read function was called lese, but slurp is even better. infn is not good? I see, my personal convention inf=file, infn=filename; I will be less brief, infilename it is. laenge it will stay, I will have to translate everything back to german :-). Unused? Oh, right. You misread vector{argv+1,argv+argc}: Its pointers to elements initializing with many filenames. Thanks a lot! \$\endgroup\$ – towi May 21 '14 at 20:50
  • \$\begingroup\$ @towi Oops, yes, I mistook the vector initialization for the form with elements instead of iterators. \$\endgroup\$ – Anonymous May 22 '14 at 0:01
  • \$\begingroup\$ I incorporated many of your suggestions. esp. GzWriteStream it is, supporting operator<<. \$\endgroup\$ – towi May 22 '14 at 19:26
5
\$\begingroup\$

Interface

As @Anonymous has said.

Writing this to use the stream interface would be much more civilized. Then reading a file and compressing it would look like this:

gzip::ozipstream    gzipFile("data.gzip");
std::ifstream       normFile("data");

gzipFile << normFile.rdbuf();

That is the interface I would be looking for. Then I could easily change the type of the stream from a file to gzip file and not worry about changing the code.

Code Review

I hope this line is not in a header (and thus polluting my code).

sing std::ifstream; using std::ofstream; using std::string; using std::vector;

Also one per line. So I can quickly see then. Note these are context aware commands and apply to the closest inclosing scope. Which is desirable (ie you don't need to do it for the whole file).

This does not help in a move:

return std::move(data);

Because the return type is not r-value reference.

Best advice is not to do that (sorry don't have a reference handy). But the clang developers hinted that it may hinder at "Going Native 2013". The compiler is great a copy elide in this situation so let it do what it is good at.

Don't like the fact that your helper functions are independant:

vector<char> read(const string& infn);
void pack(const string& infn, const string& outfn);

Why are they not part of the class. You can then take advantage of internal buffers.

\$\endgroup\$
  • \$\begingroup\$ Stream interface, hrmm, I have to ponder that. I want to have a brief example. But I see your point. No the using is not in the header and I also would rather not do that if it were'nt for printed media. I should have pointed that out. I should probably reduce them a bit. Hrm, "why are they not part of the class"? good point. Very good point indeed. \$\endgroup\$ – towi May 22 '14 at 8:57
  • \$\begingroup\$ You are wrong about "This does not help in a move" -- std::move does not have anything to do with returning an &&. It doesn't help here because RVO applies. But if I had two preconstructed potential return-values, it would help. But you are right, I confuse my readers and will get rid of it. \$\endgroup\$ – towi May 22 '14 at 9:01
  • \$\begingroup\$ I support operator<< now. I removed move, reduced the usings and put them inside the namespace, too. But I left read and pack as free functions. When I would read and print using ofstream I would have two separate functions, too. \$\endgroup\$ – towi May 22 '14 at 19:29
3
\$\begingroup\$

This is how I would write it:

Note: This has NOT been tested so it probably will not work first time (add some unit tests). But it should get you close to what you want.

#include <ostream>
#include <vector>
#include <string>

typedef void*   gzFile;
gzFile      gzopen(char const*, char const*)            {return nullptr;}
void        gzclose(gzFile)                             {}
std::size_t gzwrite(gzFile, char const*, std::size_t)   {return 0;}

class OZipStream: public std::ostream
{
    class ZipStreamBuffer: public std::streambuf
    {
        gzFile              gzipFile;
        std::vector<char>   internalBuffer;
        public:
            ZipStreamBuffer(std::string const& fileName)
                : internalBuffer(4 * 1024 * 1024)
            {
                gzipFile    = gzopen(fileName.c_str(), "wb9");
                if (gzipFile == NULL)
                {   throw std::runtime_error(strerror(errno));
                }
                // Leave one byte out of the buffer so we can use it for overflow.
                setp(&internalBuffer[0], &internalBuffer[internalBuffer.size()-1]);
            }
            ~ZipStreamBuffer()
            {
                gzclose(gzipFile);
            }
        protected:
            virtual int overflow (int c = EOF) override
            {
                // Virtual function called by other member functions to put a character into the
                // controlled output sequence without changing the current position.
                //
                // It is called by public member functions such as sputc to write a character
                // when there are no writing positions available at the put pointer (pptr).
                //
                // Its default behavior in streambuf is to always return EOF (indicating
                // failure), but derived classes can override this behavior to attempt to write
                // the character directly and/or to alter pptr and other internal pointers so
                // that more storage is provided, potentially writing unwritten characters to
                // the controlled output sequence. Both filebuf and stringbuf override this
                // virtual member function.
                //
                // Return:      In case of success, the character put is returned, converted to
                //              a value of type int_type using member traits_type::to_int_type.
                //              Otherwise, it returns the end-of-file value (EOF) either if
                //              called with this value as argument c or to signal a failure
                //              (some implementations may throw an exception instead).

                *egptr()    = c;
                std::streamsize     size = egptr() - pbase() + 1;
                if (flush(pbase(), size) != size)
                {   return EOF;
                }
                setp(pbase(), egptr());
                return traits_type::to_int_type(c);
            }
            virtual std::streamsize xsputn (const char* s, std::streamsize n) override
            {
                // Writes characters from the array pointed to by s into the controlled output
                // sequence, until either n characters have been written or the end of the
                // output sequence is reached.
                //
                // Return:      The number of characters written.
                //              streamsize is a signed integral type.

                std::streamsize     size = egptr() - pptr();
                if (size > 0)
                {
                    if (flush(pbase(), size) != size)
                    {   return 0;   // We have not got to the data we are supposed to write.
                    }
                    setp(pbase(), egptr());
                }
                return flush(s, n);
            }
        private:
            std::streamsize flush(char const* begin, std::streamsize n)
            {
                return gzwrite(gzipFile, begin, n);
            }
    };

    std::streambuf*     oldBuf;
    ZipStreamBuffer     buffer;
    public:
        OZipStream(std::string const& fileName)
            : std::ostream(nullptr)
            , buffer(fileName)
        {
            oldBuf  = rdbuf(&buffer);
        }
        ~OZipStream()
        {
            rdbuf(oldBuf);
        }
};
\$\endgroup\$
  • \$\begingroup\$ Good code, since it really uses the whole ostream infrastructure. I will store this for future reference. It goes beyond I want to show with my example, though. \$\endgroup\$ – towi May 23 '14 at 8:53

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.