You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@stdcxx.apache.org by Martin Sebor <se...@roguewave.com> on 2008/03/25 16:11:14 UTC

Re: svn commit: r640831 - /stdcxx/trunk/include/sstream.cc

Any idea what caused it? I've gone through the recent history
of the function and this looks like the likely change might
be this one:

   http://svn.apache.org/viewvc?view=rev&revision=442675

but the test wasn't failing. Maybe because the test doesn't
cause the buffer to reallocate?

Martin

faridz@apache.org wrote:
> Author: faridz
> Date: Tue Mar 25 07:33:02 2008
> New Revision: 640831
> 
> URL: http://svn.apache.org/viewvc?rev=640831&view=rev
> Log:
> 2008-03-25  Farid Zaripov  <fa...@epam.com>
> 
> 	STDCXX-792
> 	* include/sstream.cc (str): Save pptr and restore is at the end if
> 	str() is called with internal buffer to increase the capacity of
> 	buffer (i.e. from xsputn() or overflow()).
> 	(xsputn): Remove fix for STDCXX-515 because the pptr now is
> 	preserved in str().
> 
> Modified:
>     stdcxx/trunk/include/sstream.cc
> 
> Modified: stdcxx/trunk/include/sstream.cc
> URL: http://svn.apache.org/viewvc/stdcxx/trunk/include/sstream.cc?rev=640831&r1=640830&r2=640831&view=diff
> ==============================================================================
> --- stdcxx/trunk/include/sstream.cc (original)
> +++ stdcxx/trunk/include/sstream.cc Tue Mar 25 07:33:02 2008
> @@ -82,8 +82,10 @@
>      _ValueAlloc __alloc;
>  
>      // new buffer and size
> -    char_type     *__buf;
> -    _RWSTD_SIZE_T  __bufsize = __slen;
> +    char_type        *__buf;
> +    _RWSTD_SIZE_T     __bufsize = __slen;
> +    // saved offset of pptr 
> +    _RWSTD_STREAMSIZE __off = -1;
>  
>      if (__s == this->_C_buffer) {
>          // special case: str(_C_buffer, _C_bufsize + N) called
> @@ -94,6 +96,8 @@
>          // set `slen' to the number of initialized characters
>          // in the buffer
>          __slen = this->egptr () - this->pbase ();
> +        // save the offset of pptr
> +        __off = this->pptr () - this->pbase ();
>      }
>  
>      if (this->_C_bufsize < __bufsize) {
> @@ -166,8 +170,12 @@
>      if (this->_C_is_out ()) {
>          this->setp (this->_C_buffer, this->_C_buffer + this->_C_bufsize);
>  
> -        if (   __s != __buf && this->_C_state & ios_base::in
> -            || this->_C_state & (ios_base::app | ios_base::ate)) {
> +        if (0 <= __off) {
> +            // restore the pptr
> +            this->pbump (__off);
> +        }
> +        else if (   this->_C_state & ios_base::in
> +                 || this->_C_state & (ios_base::app | ios_base::ate)) {
>              // in input or append/ate modes seek to end
>              // (see also lwg issue 562 for clarification)
>              this->pbump (__slen);
> @@ -204,15 +212,9 @@
>              __off = this->pbase () - __s;
>          }
>  
> -        // preserve current pptr() since str() would seek to end
> -        const streamsize __cur = this->pptr () - this->pbase ();
> -
>          // grow the buffer if necessary to accommodate the whole
>          // string plus the contents of the buffer up to pptr()
>          str (this->_C_buffer, __bufsize);
> -
> -        // restore pptr()
> -        this->pbump (__cur - (this->pptr () - this->pbase ()));
>  
>          _RWSTD_ASSERT (__n <= this->epptr () - this->pptr ());
>  
> 
> 
> 


Re: svn commit: r640831 - /stdcxx/trunk/include/sstream.cc

Posted by Martin Sebor <se...@roguewave.com>.
Farid Zaripov wrote:
>> -----Original Message-----
>> From: Martin Sebor [mailto:msebor@gmail.com] On Behalf Of Martin Sebor
>> Sent: Tuesday, March 25, 2008 5:11 PM
>> To: dev@stdcxx.apache.org
>> Subject: Re: svn commit: r640831 - /stdcxx/trunk/include/sstream.cc
>>
>> Any idea what caused it? I've gone through the recent history 
>> of the function and this looks like the likely change might 
>> be this one:
>>
>>    http://svn.apache.org/viewvc?view=rev&revision=442675
>>
>> but the test wasn't failing. Maybe because the test doesn't 
>> cause the buffer to reallocate?
> 
>   I think the bug introduced in
> http://svn.apache.org/viewvc?view=rev&revision=380995

Okay, thanks for the detective work! Looking at the test,
it does appear to exercise very long sequences (just like
in Mark's test case) so I should probably try to figure
why it didn't reveal such a blatant bug.

Martin

> since the overflow() and xsputn() started calling the str() for
> reallocate the buffer.
> 
> rev 442675:
> ---------------
> +    if (this->_C_is_out ()) {
> +        this->setp (this->_C_buffer, this->_C_buffer +
> this->_C_bufsize);
> +
> +        if (__s != __buf || this->_C_state & (ios_base::app |
> ios_base::ate))
> +            this->pbump (__slen);   // seek to end
>      }
> ---------------
> 
>  After that change setp() resets pptr to pbase, then pbump() seeks pptr
> to end always,
> when str() is called from xsputn() or overflow() (the str() is called to
> reallocate buffer,
> so __s != __buf). The original position of pptr is lost.
> 
> rev 442675:
> ---------------
>      if (this->_C_is_out ()) {
>          this->setp (this->_C_buffer, this->_C_buffer +
> this->_C_bufsize);
>  
> -        if (__s != __buf || this->_C_state & (ios_base::app |
> ios_base::ate))
> -            this->pbump (__slen);   // seek to end
> +        if (   __s != __buf && this->_C_state & ios_base::in
> +            || this->_C_state & (ios_base::app | ios_base::ate)) {
> +            // in input or append/ate modes seek to end
> +            // (see also lwg issue 562 for clarification)
> +            this->pbump (__slen);
> +        }
>      }
>  ---------------
> 
>   After that change setp() resets the pptr to pbase, then if _C_state &
> ios_base::in != 0
> then pbump() seeks pptr to end. In both cases the original position of
> pptr is lost.
> 
>   In STDCXX-515 the stringstream is used (_C_state & ios_base::in != 0)
> and subsequent
> output to stream is appended to end of the buffer despite of that pptr
> is not points to end
> due to seek(-1), ios_base::cur).
> 
>   In STDCXX-795 the ostringstream is used (_C_state & ios_base::in == 0)
> and subsequent
> output to stream is inserted from the begin of the buffer.
> 
> Farid.


RE: svn commit: r640831 - /stdcxx/trunk/include/sstream.cc

Posted by Farid Zaripov <Fa...@epam.com>.
> -----Original Message-----
> From: Martin Sebor [mailto:msebor@gmail.com] On Behalf Of Martin Sebor
> Sent: Tuesday, March 25, 2008 5:11 PM
> To: dev@stdcxx.apache.org
> Subject: Re: svn commit: r640831 - /stdcxx/trunk/include/sstream.cc
> 
> Any idea what caused it? I've gone through the recent history 
> of the function and this looks like the likely change might 
> be this one:
> 
>    http://svn.apache.org/viewvc?view=rev&revision=442675
> 
> but the test wasn't failing. Maybe because the test doesn't 
> cause the buffer to reallocate?

  I think the bug introduced in
http://svn.apache.org/viewvc?view=rev&revision=380995
since the overflow() and xsputn() started calling the str() for
reallocate the buffer.

rev 442675:
---------------
+    if (this->_C_is_out ()) {
+        this->setp (this->_C_buffer, this->_C_buffer +
this->_C_bufsize);
+
+        if (__s != __buf || this->_C_state & (ios_base::app |
ios_base::ate))
+            this->pbump (__slen);   // seek to end
     }
---------------

 After that change setp() resets pptr to pbase, then pbump() seeks pptr
to end always,
when str() is called from xsputn() or overflow() (the str() is called to
reallocate buffer,
so __s != __buf). The original position of pptr is lost.

rev 442675:
---------------
     if (this->_C_is_out ()) {
         this->setp (this->_C_buffer, this->_C_buffer +
this->_C_bufsize);
 
-        if (__s != __buf || this->_C_state & (ios_base::app |
ios_base::ate))
-            this->pbump (__slen);   // seek to end
+        if (   __s != __buf && this->_C_state & ios_base::in
+            || this->_C_state & (ios_base::app | ios_base::ate)) {
+            // in input or append/ate modes seek to end
+            // (see also lwg issue 562 for clarification)
+            this->pbump (__slen);
+        }
     }
 ---------------

  After that change setp() resets the pptr to pbase, then if _C_state &
ios_base::in != 0
then pbump() seeks pptr to end. In both cases the original position of
pptr is lost.

  In STDCXX-515 the stringstream is used (_C_state & ios_base::in != 0)
and subsequent
output to stream is appended to end of the buffer despite of that pptr
is not points to end
due to seek(-1), ios_base::cur).

  In STDCXX-795 the ostringstream is used (_C_state & ios_base::in == 0)
and subsequent
output to stream is inserted from the begin of the buffer.

Farid.