Proper subclassing of streambuf

[mathieu]

Dear all,

I am trying -again- to understand how to properly implement a
subclass of streambuf. In the following example I am trying to
reproduce a case where sync() should refuse to write any more
characters. For the purpose of the exercise I used a fixed size buffer
to quickly get a segfault. Could anyone of you comment on the code and
let me know how they would handle case where sync() should not write
anymore (obviously you do not have access to the size of the buffer as
it just an example).

Thanks !

#include <iostream>
#include <string>
#include <cassert>
#include <cstring>

class windowbuf : public std::streambuf {
char *buffer;
public:
windowbuf(char *b, size_t len):buffer(b) { setp(b, b+len); }
int_type sync ();
int_type write( const char *buffer, size_t len );
int_type overflow (int ch);
};

windowbuf::int_type windowbuf::write( const char *buf, size_t len )
{
memcpy(buffer, buf, len );
buffer += len;
return len;
}

windowbuf::int_type windowbuf::sync ()
{
if (pptr () && pbase () < pptr () && pptr () <= epptr ())
{
int n = write( pbase(), pptr() - pbase() );
setp (pbase (), pbase() + n);
}

return 0;
}

int windowbuf:verflow (int ch)
{
if( pbase() == 0 ) return traits_type::eof();
if( ch == traits_type::eof() )
return sync();
if( pptr() == epptr() )
sync();

*pptr() = (char_type)ch;
pbump(1);
return ch;
}

int main (int argc, char*argv[])
{
static const int n = 5;
char fixedbuffer[n];
windowbuf wbuf( fixedbuffer, n);
std:stream wstr(&wbuf);
wstr << "12" << std::flush;
wstr << "Hello" << std::flush;
wstr << "world!" << std::flush;
wstr << "Very long string" << std::flush;

std::string s( fixedbuffer, fixedbuffer + n );
std::cout << s << std::endl;

return 0;
}

 

[James Kanze]

I am trying -again- to understand how to properly implement a
subclass of streambuf. In the following example I am trying to
reproduce a case where sync() should refuse to write any more
characters.

It's not too clear what you mean by that, but the default
behavior of sync() (if you don't override it) is to succeed
withouth doing anything. This is appropriate in cases where the
managed subsequence is identical with the actual underlying
controlled sequence. Otherwise, sync() must be overridden to
"synchronize" the controlled sequence with the managed
subsequence in the character array, by writing the characters in
the array to the controlled sequence. I'm not sure what you
mean by "refuse to write any more characters": if the controlled
sequence has a maximum fixed length, then overflow() (not
sync()) should ensure that the available buffer never allows
more characters to be written. (In most cases, sync() will not
be called until the file is closed.)

For the purpose of the exercise I used a fixed size buffer to
quickly get a segfault.

How does using a fixed size buffer quickly cause a segfault?

Could anyone of you comment on the
code and let me know how they would handle case where sync()
should not write anymore (obviously you do not have access to
the size of the buffer as it just an example).

If you're writing to a memory buffer, you don't have to do
anything in sync(); the default does exactly what you want.

#include <iostream>
#include <string>
#include <cassert>
#include <cstring>

class windowbuf : public std::streambuf {
char *buffer;
public:
windowbuf(char *b, size_t len):buffer(b) { setp(b, b+len); }

Here, you've defined the character array passed as an argument
to be the managed character array (sub)sequence. If this is the
also the underlying controlled sequence (and I don't see
anything else that could be the underlying controlled sequence),
then that's all you have to do. Period: all of the other
functions are invoked to synchronize the character array with
the controlled sequence, or move the subset of the controlled
sequence represented by the character array. If the character
array is the controlled sequence, then they don't have to do
anything, and the implementations in the base class are fine.

int_type sync ();
int_type write( const char *buffer, size_t len );
int_type overflow (int ch);

The functions Sync and overflow should not be public, but
protected. And I'm very suspicious of a public write function:
if you're trying to provide two different ways for client code
to write to the buffer, it's going to cause problems.

And by the way, sync returns an int, and not an int_type. (Of
course, for the specialization you're dealing with, int_type is
an int. For a non-template implementation, like this, I'd
normally declare both sync and overflow to return int. But
int_type is only correct for overflow.)

};

windowbuf::int_type windowbuf::write( const char *buf, size_t len )
{
memcpy(buffer, buf, len );
buffer += len;
return len;
}

When would you call this function, and what is it supposed to
do. It looks sort of like xsputn, except that it doesn't take
into account anything which has already been written to the
buffer, nor the remaining length in the buffer. A correct
implementation of xsputn would be more along the lines of:

windowbuf::streamsize windowbuf::xsputn( char const* source,
streamsize len )
{
streamsize toCopy = std::min( len, epptr() - pptr() );
memcpy( pptr(), source, toCopy );
return toCopy;
}

(This is not actually sufficient. If toCopy is less than len,
it should call overflow with the next character, or call sync,
if that's what overflow does, and try to write the remaining
characters.)

It both takes and returns a streamsize. An assert that the
argument isn't negative would probably be in order.

The implementation of this function is purely an optimization,
however. The implementation in the base class simply calls
sputc len times, which will ultimately have the same effect.

windowbuf::int_type windowbuf::sync ()
{
if (pptr () && pbase () < pptr () && pptr () <= epptr ())
{
int n = write( pbase(), pptr() - pbase() );
setp (pbase (), pbase() + n);
}
return 0;
}

And what on earth is this supposed to do? Your sequences are
already sync'ed. If they weren't (and the stream is pure
output, without support for seek), then sync() should copy the
range [pbase(),pptr()) into the underlying sequence, and then
call setp to reinitialize the buffer pointer. If the original
buffer size is to represent the maximum number of characters
which can be written, this would be:
setp( pptr(), epptr() );
..

int windowbuf:verflow (int ch)
{
if( pbase() == 0 ) return traits_type::eof();

How could this ever happen, given your code. You set pbase() to
non-null in the constructor, and you never set it to null later.

if( ch == traits_type::eof() )
return sync();

Overflow should not return what sync returns, since the
semantic is potentially different. (sync returns -1 in case of
error, overflow returns EOF. On all systems I've worked on, EOF
has been -1, but all that is formally required is that it be
negative.)

if( pptr() == epptr() )
sync();

*pptr() = (char_type)ch;

If pptr() == epptr(), this would cause undefined behavior. Or
if (as would happen in your sync), you've set epptr() beyond the
actual end of your buffer, and pptr() is beyond the end of your
buffer, you'll also get undefined behavior.

pbump(1);
return ch;
}

Note that one frequent trick is to pass an end pointer to setp
that is one less than the actual buffer size. That way,
overflow can still put its argument into the buffer and output
it with the rest.