Re: [Patch] Fix seek to beg after grow bug

[Nathan Myers <ncm-nospam at cantrip dot org>]

On Tue, Feb 18, 2003 at 10:31:42PM +0100, Paolo Carlini wrote:
> Nathan Myers wrote:
> 
> >This doesn't seem quite right.  I think the right code is
> >more like just
> >
> >     __size_type __len = std::max(_M_string.size(),
> >       __size_type(this->_M_out_cur - this->_M_out_beg));
> >
> >In other words, either cur and beg are both 0, or (cur - beg) is 
> >at least as large as the string.
> >
> No, this cannot be ok.
> Look at the testcase: _M_out_cur == _M_out_beg in that case and _M_out_end,
> which points one-past-end, gives the current string length, as _M_out_end -
> _M_out_beg which is _bigger_ than _M_string.size().
> Indeed, the code continues as:
> 
>    return __string_type(this->_M_out_beg, this->_M_out_beg + __len);

Maybe you are working with the code as it was before 169, and I am 
imagining it as it must be after?  As I understand it, the invariants
are 

  _M_out_beg == s.data(), 
  _M_out_end == (s.data()+s.capacity()), and 
  if _M_out_cur > (s.data()+s.length()), then s.length() 
     should be updated to equal _M_out_cur-_M_out_beg at each
     opportunity -- in overflow, seek*, and str.

Then, once invariants are restored in str(), constructing the return
value should be an ordinary string copy.

Look, here's the buffer:
   _______
  |_______|______________________________________________________
  |______________________________________________________________|
  ^beg                               ^len        ^cur           ^end

(The box on top is where _M_len, _M_capacity, and _M_refcount live.)
_M_out_cur can point anywhere in the buffer.  Any time one of the
virtuals finds cur to the right of len, it moves len out to cur 
and pokes a NUL there.  If cur hits end, overflow gets called, and 
reallocates, and then cur and len point somewhere in the middle of 
a bigger buffer.

> >We have to be sure, too, that in seek{pos,off}, the string length gets 
> >updated like the above before _M_out_cur gets moved, because sometimes 
> >_M_out_cur is our only record of how long the string really has become.
> >
> Humm. The seeks do not change any length by themselves, but they change 
> _M_out_cur,
> but _always_ via _M_out_cur_move which is:
> 
>       _M_out_cur_move(off_type __n) // argument needs to be +-
>       {
>         bool __testin = _M_in_cur;
> 
>         _M_out_cur += __n;
>         if (__testin && _M_buf_unified)
>           _M_in_cur += __n;
>         if (_M_out_cur > _M_out_end)
>           {
>             _M_out_end = _M_out_cur;
>             // NB: in | out buffers drag the _M_in_end pointer along...
>             if (__testin)
>               _M_in_end += __n;
>           }
>        }
> 
> Therefore, you can see that _M_out_cur _cannot_ become bigger than 
> _M_out_end.

That seems way too complicated.  I don't know any reason to use
_M_out_cur_move in stringbuf<>::seek*.  There's no such thing as 
a unified stringbuf, or a tied one.  (In fact, all that unified-and-
tied stuff ought to be well-confined in filebuf.  How did it leak out 
to streambuf?)

Also, there must be  a bug in _M_out_cur_move; you can't just add a 
random value to a pointer without first checking whether the sum 
will still point to valid storage; if not, the result is undefined.  
Furthermore, it can't be right to simply drag along the _M_out_end 
pointer, because that should identify the really-end-of-buffer, so if 
you need to pass it, you have to call a virtual.  Sigh.

What a mess.

> Really, it seems to me that _M_out_end is _always_ equal to the string end,
> and in fact, as such is used (under the name epptr, but this is another 
> story :)
> in the seek operation which we already privately discussed about...

I think that used to be true, before 169.  Now, we want epptr() to
point to the end of the string buffer (less one, to allow space for 
the NUL).  That way you only call overflow when you need to grow the
string buffer.

> >Likewise, whenever the buffer gets lengthened, the string capacity 
> >needs to be updated along with _M_out_end.
> >
> This is done correctly, if not with optimal efficiency by str(), it seems.
> 
> Do you agree??

It has to happen in overflow().  It seems to me that in str() it's 
too late.

Or maybe I'm engaging in wishful thinking, and stringbuf is way, way
more complicated than I imagine.

Nathan Myers
ncm-nospam@cantrip.org