This is the mail archive of the libstdc++@gcc.gnu.org mailing list for the libstdc++ project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[Patch] Change _M_overflow to call _M_convert_to_external once


Hi,

this is my current best proposal, taking into account
Nathan's and Benjamin's suggestions.

A few sparse observations:
1- setbuf(s(!=null), n(==1)) is simply ignored. This is allowed
by the standard and not a big loss, probably, but still a change
of behavior.
2- setbuf(0,0) leads internally to _M_buf_size == 1, that is,
actually, zero-sized get and put areas (null pointers). Using
this value instead of _M_buf_size == 0 simplifies _M_set_buffer.
3- I have renamed _M_set_determinate as _M_set_buffer, and
removed _M_set_indeterminate which was trivially calling
_M_set_determinate.
4- Two tests had to be tweaked: one relied on a BUFSIZ sized put
area, the other on a single-char stdio_filebuf.
5- Before and after this patch, unbuffered basic_filebuf::_M_overflow
and _M_underflow (a.k.a., libstdc++/9404) are still missing.
Benjamin, which strategy do you prefer? Adding a special case to
the current versions would now be easy.

Tested x86-linux.

Thanks,
Paolo.

//////////

2003-05-14  Paolo Carlini  <pcarlini@unitus.it>
	    Nathan Myers  <ncm@cantrip.org>

	* include/bits/fstream.tcc (_M_overflow): Rewrote to call
	_M_convert_to_external only once (_M_buf_size is now the size of the put
	area + 1 for the overflow char of a full area); call _M_set_buffer instead
	of _M_set_indeterminate.
	(_M_allocate_internal_buffer): Don't allocate a buffer smaller than 2 chars.
	(setbuf): Don't accept a buffer smaller than 2 chars.
	(_M_underflow): Refill _M_buf_size - 1 chars; call _M_set_buffer, instead of
	_M_set_determinate.
	(open): Call _M_set_buffer, instead of _M_set_indeterminate.
	(seekoff): Likewise.
	* include/ext/stdio_filebuf.h (stdio_filebuf(int, std::ios_base::openmode,
	bool, size_t), stdio_filebuf(std::__c_file*, std::ios_base::openmode, size_t):
	Likewise.
	* include/std/std_fstream.h (_M_set_indeterminate): Remove.
	(_M_set_determinate): Rename as _M_set_buffer, _M_buf_size -> _M_buf_size - 1.
	* include/std/std_streambuf.h: Tweak _M_out_lim comment.
	* testsuite/27_io/basic_filebuf/sgetn/char/1.cc: Tweak, taking into account
	that, for _M_buf_size == BUFSIZ == 8192, the size of the put area is now
	BUFSIZ - 1.
	* testsuite/ext/stdio_filebuf_2.cc: Tweak, taking into account that now
	the smallest _M_buf_size is 2 (still fails, for the same reason, with 3.2.3)
diff -prN libstdc++-v3-orig/include/bits/fstream.tcc libstdc++-v3/include/bits/fstream.tcc
*** libstdc++-v3-orig/include/bits/fstream.tcc	Tue May 13 22:13:14 2003
--- libstdc++-v3/include/bits/fstream.tcc	Wed May 14 18:17:19 2003
*************** namespace std
*** 48,54 ****
      basic_filebuf<_CharT, _Traits>::
      _M_allocate_internal_buffer()
      {
!       if (!this->_M_buf && this->_M_buf_size)
  	{
  	  // Allocate internal buffer.
  	  this->_M_buf = new char_type[this->_M_buf_size];
--- 48,54 ----
      basic_filebuf<_CharT, _Traits>::
      _M_allocate_internal_buffer()
      {
!       if (!_M_buf_allocated && this->_M_buf_size > 1)
  	{
  	  // Allocate internal buffer.
  	  this->_M_buf = new char_type[this->_M_buf_size];
*************** namespace std
*** 101,107 ****
  	      this->_M_mode = __mode;
  
  	      // Setup initial position of buffer.
! 	      _M_set_indeterminate();
  
  	      if ((__mode & ios_base::ate)
  		  && this->seekoff(0, ios_base::end, __mode) < 0)
--- 101,107 ----
  	      this->_M_mode = __mode;
  
  	      // Setup initial position of buffer.
! 	      _M_set_buffer(0);
  
  	      if ((__mode & ios_base::ate)
  		  && this->seekoff(0, ios_base::end, __mode) < 0)
*************** namespace std
*** 218,244 ****
  				ios_base::in);
  	    }
  
! 	  if (_M_buf_size)
  	    {
  	      streamsize __elen = 0;
  	      streamsize __ilen = 0;
  
  	      if (__check_facet(_M_codecvt).always_noconv())
  		{
! 		  __elen = _M_file.xsgetn(reinterpret_cast<char*>(this->_M_in_beg), _M_buf_size);
  		  __ilen = __elen;
  		}
  	      else
  		{
! 		  char* __buf = static_cast<char*>(__builtin_alloca(_M_buf_size));
! 		  __elen = _M_file.xsgetn(__buf, _M_buf_size);
  		  
  		  const char* __eend;
  		  char_type* __iend;
  		  codecvt_base::result __r;
  		  __r = _M_codecvt->in(_M_state_cur, __buf, __buf + __elen, 
  				       __eend, this->_M_in_beg, 
! 				       this->_M_in_beg + _M_buf_size, __iend);
  		  if (__r == codecvt_base::ok)
  		    __ilen = __iend - this->_M_in_beg;
  		  else if (__r == codecvt_base::noconv)
--- 218,244 ----
  				ios_base::in);
  	    }
  
! 	  if (_M_buf_size > 1)
  	    {
  	      streamsize __elen = 0;
  	      streamsize __ilen = 0;
  
  	      if (__check_facet(_M_codecvt).always_noconv())
  		{
! 		  __elen = _M_file.xsgetn(reinterpret_cast<char*>(this->_M_in_beg), _M_buf_size - 1);
  		  __ilen = __elen;
  		}
  	      else
  		{
! 		  char* __buf = static_cast<char*>(__builtin_alloca(_M_buf_size - 1));
! 		  __elen = _M_file.xsgetn(__buf, _M_buf_size - 1);
  		  
  		  const char* __eend;
  		  char_type* __iend;
  		  codecvt_base::result __r;
  		  __r = _M_codecvt->in(_M_state_cur, __buf, __buf + __elen, 
  				       __eend, this->_M_in_beg, 
! 				       this->_M_in_beg + _M_buf_size - 1, __iend);
  		  if (__r == codecvt_base::ok)
  		    __ilen = __iend - this->_M_in_beg;
  		  else if (__r == codecvt_base::noconv)
*************** namespace std
*** 258,264 ****
  
  	      if (0 < __ilen)
  		{
! 		  _M_set_determinate(__ilen);
  		  __ret = traits_type::to_int_type(*this->_M_in_cur);
  		  if (__bump)
  		    _M_move_in_cur(1);
--- 258,264 ----
  
  	      if (0 < __ilen)
  		{
! 		  _M_set_buffer(__ilen);
  		  __ret = traits_type::to_int_type(*this->_M_in_cur);
  		  if (__bump)
  		    _M_move_in_cur(1);
*************** namespace std
*** 338,380 ****
      _M_overflow(int_type __c)
      {
        int_type __ret = traits_type::eof();
        const bool __testput = this->_M_out_beg < this->_M_out_lim;
  
        if (__testput)
! 	{
! 	  // Need to restore current position. The position of the external
! 	  // byte sequence (_M_file) corresponds to _M_filepos, and we need
! 	  // to move it to _M_out_beg for the write.
! 	  if (_M_filepos && _M_filepos != this->_M_out_beg)
! 	    {
! 	      off_type __off = this->_M_out_beg - _M_filepos;
! 	      _M_file.seekoff(__off, ios_base::cur);
! 	    }
! 
! 	  // Convert internal buffer to external representation, output.
! 	  if (_M_convert_to_external(this->_M_out_beg, 
! 				     this->_M_out_lim - this->_M_out_beg))
! 	    {
! 	      // Convert pending sequence to external representation, output.
! 	      // If eof, then just attempt sync.
! 	      if (!traits_type::eq_int_type(__c, traits_type::eof()))
! 		{
! 		  // User code must flush when switching modes (thus
! 		  // don't sync).
! 		  char_type __pending = traits_type::to_char_type(__c);
! 		  if (_M_convert_to_external(&__pending, 1))
! 		    {
! 		      _M_set_indeterminate();
! 		      __ret = traits_type::not_eof(__c);
! 		    }
! 		}
! 	      else if (!_M_file.sync())
! 		{
! 		  _M_set_indeterminate();
! 		  __ret = traits_type::not_eof(__c);
! 		}
! 	    }
  	}
        _M_last_overflowed = true;	
        return __ret;
      }
--- 338,371 ----
      _M_overflow(int_type __c)
      {
        int_type __ret = traits_type::eof();
+       const bool __testeof = traits_type::eq_int_type(__c, __ret);
        const bool __testput = this->_M_out_beg < this->_M_out_lim;
+       bool __flushok = false;
  
        if (__testput)
!  	{
!  	  // Need to restore current position. The position of the external
!  	  // byte sequence (_M_file) corresponds to _M_filepos, and we need
!  	  // to move it to _M_out_beg for the write.
! 	  if (_M_filepos != this->_M_out_beg)
! 	    _M_file.seekoff(this->_M_out_beg - _M_filepos, ios_base::cur);
! 
! 	  // If appropriate, append the overflow char.
! 	  if (!__testeof)
! 	    *this->_M_out_lim++ = traits_type::to_char_type(__c);
! 	  // Convert pending sequence to external representation, output.
! 	  __flushok = _M_convert_to_external(this->_M_out_beg,
! 					     this->_M_out_lim - this->_M_out_beg);
! 	  if (__testeof && __flushok)
! 	    __flushok = !_M_file.sync();
! 
! 	  if(__flushok)
!  	    {
! 	      _M_set_buffer(0);
! 	      __ret = traits_type::not_eof(__c);
!  	    }
  	}
+ 
        _M_last_overflowed = true;	
        return __ret;
      }
*************** namespace std
*** 478,490 ****
      setbuf(char_type* __s, streamsize __n)
      {
        if (!this->is_open() && __s == 0 && __n == 0)
! 	this->_M_buf_size = 0;
!       else if (__s && __n)
  	{
  	  // This is implementation-defined behavior, and assumes
  	  // that an external char_type array of length (__s + __n)
  	  // exists and has been pre-allocated. If this is not the
! 	  // case, things will quickly blow up.
  
  	  // Step 1: Destroy the current internal array.
  	  _M_destroy_internal_buffer();
--- 469,483 ----
      setbuf(char_type* __s, streamsize __n)
      {
        if (!this->is_open() && __s == 0 && __n == 0)
! 	this->_M_buf_size = 1;
!       else if (__s && __n > 1)
  	{
  	  // This is implementation-defined behavior, and assumes
  	  // that an external char_type array of length (__s + __n)
  	  // exists and has been pre-allocated. If this is not the
! 	  // case, things will quickly blow up. __n - 1 positions
! 	  // will be used for the get and put areas, 1 to host the
! 	  // overflow char of a full put area. Therefore __n > 1.
  
  	  // Step 1: Destroy the current internal array.
  	  _M_destroy_internal_buffer();
*************** namespace std
*** 492,498 ****
  	  // Step 2: Use the external array.
  	  this->_M_buf = __s;
  	  this->_M_buf_size = __n;
! 	  _M_set_indeterminate();
  	}
        _M_last_overflowed = false;	
        return this; 
--- 485,491 ----
  	  // Step 2: Use the external array.
  	  this->_M_buf = __s;
  	  this->_M_buf_size = __n;
! 	  _M_set_buffer(0);
  	}
        _M_last_overflowed = false;	
        return this; 
*************** namespace std
*** 539,545 ****
  
  	      // Return pos_type(off_type(-1)) in case of failure.
  	      __ret = _M_file.seekoff(__computed_off, __way, __mode);
! 	      _M_set_indeterminate();
  	    }
  	  else
  	    {
--- 532,538 ----
  
  	      // Return pos_type(off_type(-1)) in case of failure.
  	      __ret = _M_file.seekoff(__computed_off, __way, __mode);
! 	      _M_set_buffer(0);
  	    }
  	  else
  	    {
diff -prN libstdc++-v3-orig/include/ext/stdio_filebuf.h libstdc++-v3/include/ext/stdio_filebuf.h
*** libstdc++-v3-orig/include/ext/stdio_filebuf.h	Sun May 11 06:20:55 2003
--- libstdc++-v3/include/ext/stdio_filebuf.h	Wed May 14 13:19:55 2003
*************** namespace __gnu_cxx
*** 67,72 ****
--- 67,74 ----
         *  @param  mode  Same meaning as in a standard filebuf.
         *  @param  del  Whether to close the file on destruction.
         *  @param  size  Optimal or preferred size of internal buffer, in bytes.
+        *                Note that it includes a position for the overflow char,
+        *                therefore, can't be smaller than 2.
         *
         *  This constructor associates a file stream buffer with an open
         *  POSIX file descriptor.  Iff @a del is true, then the associated
*************** namespace __gnu_cxx
*** 79,85 ****
         *  @param  f  An open @c FILE*.
         *  @param  mode  Same meaning as in a standard filebuf.
         *  @param  size  Optimal or preferred size of internal buffer, in bytes.
!        *                Defaults to system's @c BUFSIZ.
         *
         *  This constructor associates a file stream buffer with an open
         *  C @c FILE*.  The @c FILE* will not be automatically closed when the
--- 81,89 ----
         *  @param  f  An open @c FILE*.
         *  @param  mode  Same meaning as in a standard filebuf.
         *  @param  size  Optimal or preferred size of internal buffer, in bytes.
!        *                Defaults to system's @c BUFSIZ. Note that it includes
!        *                a position for the overflow char, therefore, can't be
!        *                smaller than 2.
         *
         *  This constructor associates a file stream buffer with an open
         *  C @c FILE*.  The @c FILE* will not be automatically closed when the
*************** namespace __gnu_cxx
*** 121,129 ****
        if (this->is_open())
  	{
  	  this->_M_mode = __mode;
! 	  this->_M_buf_size = __size;
  	  _M_allocate_internal_buffer();
! 	  _M_set_indeterminate();
  	}
      }
  
--- 125,133 ----
        if (this->is_open())
  	{
  	  this->_M_mode = __mode;
! 	  this->_M_buf_size = __size; // > 1
  	  _M_allocate_internal_buffer();
! 	  _M_set_buffer(0);
  	}
      }
  
*************** namespace __gnu_cxx
*** 136,144 ****
        if (this->is_open())
  	{
  	  this->_M_mode = __mode;
! 	  this->_M_buf_size = __size;
  	  _M_allocate_internal_buffer();
! 	  _M_set_indeterminate();
  	}
      }
  } // namespace __gnu_cxx
--- 140,148 ----
        if (this->is_open())
  	{
  	  this->_M_mode = __mode;
! 	  this->_M_buf_size = __size; // > 1
  	  _M_allocate_internal_buffer();
! 	  _M_set_buffer(0);
  	}
      }
  } // namespace __gnu_cxx
diff -prN libstdc++-v3-orig/include/std/std_fstream.h libstdc++-v3/include/std/std_fstream.h
*** libstdc++-v3-orig/include/std/std_fstream.h	Tue May 13 22:13:15 2003
--- libstdc++-v3/include/std/std_fstream.h	Wed May 14 18:48:37 2003
*************** namespace std
*** 123,129 ****
  
        /**
         *  @if maint
!        *  Actual size of internal buffer.
         *  @endif
        */
        size_t			_M_buf_size;
--- 123,131 ----
  
        /**
         *  @if maint
!        *  Actual size of internal buffer. This number is equal to the size
!        *  of the put area + 1 position, reserved for the overflow char of
!        *  a full area.
         *  @endif
        */
        size_t			_M_buf_size;
*************** namespace std
*** 452,492 ****
        void
        _M_output_unshift();
  
!       // These two functions are used to clarify internal buffer
!       // maintenance. After an overflow, or after a seekoff call that
!       // started at beg or end, or possibly when the stream becomes
!       // unbuffered, and a myrid other obscure corner cases, the
!       // internal buffer does not truly reflect the contents of the
!       // external buffer. At this point, for whatever reason, it is in
!       // an indeterminate state.
!       /**
!        *  @if maint
!        *  @doctodo
!        *  @endif
!       */
!       void
!       _M_set_indeterminate(void)
!       { _M_set_determinate(off_type(0)); }
! 
!       /**
!        *  @if maint
!        *  @doctodo
!        *  @endif
!       */
!       void
!       _M_set_determinate(off_type __off)
!       {
! 	const bool __testin = this->_M_mode & ios_base::in;
! 	const bool __testout = this->_M_mode & ios_base::out;
  	if (__testin)
  	  this->setg(this->_M_buf, this->_M_buf, this->_M_buf + __off);
! 	if (__testout)
! 	  {
! 	    this->setp(this->_M_buf, this->_M_buf + this->_M_buf_size);
! 	    this->_M_out_lim += __off;
! 	  }
! 	_M_filepos = this->_M_buf + __off;
!       }
      };
  
    // [27.8.1.5] Template class basic_ifstream
--- 454,480 ----
        void
        _M_output_unshift();
  
!        // This function sets the pointers of the internal buffer, both get
!        // and put areas. Typically, __off == _M_in_end - _M_in_beg upon
!        // _M_underflow; __off == 0 upon _M_overflow, seekoff, open, setbuf.
!        // 
!        // NB: _M_out_end - _M_out_beg == _M_buf_size - 1, since _M_buf_size
!        // reflects the actual allocated memory and the last cell is reserved
!        // for the overflow char of a full put area.
!        void
!        _M_set_buffer(streamsize __off)
!        {
!  	const bool __testin = this->_M_mode & ios_base::in;
!  	const bool __testout = this->_M_mode & ios_base::out;
  	if (__testin)
  	  this->setg(this->_M_buf, this->_M_buf, this->_M_buf + __off);
!  	if (__testout)
!  	  {
! 	    this->setp(this->_M_buf, this->_M_buf + this->_M_buf_size - 1);
!  	    this->_M_out_lim += __off;
!  	  }
!  	_M_filepos = this->_M_buf + __off;
!        }
      };
  
    // [27.8.1.5] Template class basic_ifstream
diff -prN libstdc++-v3-orig/include/std/std_streambuf.h libstdc++-v3/include/std/std_streambuf.h
*** libstdc++-v3-orig/include/std/std_streambuf.h	Tue May 13 22:13:15 2003
--- libstdc++-v3/include/std/std_streambuf.h	Wed May 14 13:10:34 2003
*************** namespace std
*** 187,194 ****
        //@{
        /**
         *  @if maint
!        *  _M_set_indeterminate and setp set it equal to _M_out_beg, then
!        *  at each put operation it may be moved forward (toward _M_out_end)
         *  by _M_out_cur_move.
         *  @endif
        */      
--- 187,194 ----
        //@{
        /**
         *  @if maint
!        *  setp (and _M_set_buffer(0) in basic_filebuf) set it equal to _M_out_beg,
!        *  then at each put operation it may be moved forward (toward _M_out_end)
         *  by _M_out_cur_move.
         *  @endif
        */      
diff -prN libstdc++-v3-orig/testsuite/27_io/basic_filebuf/sgetn/char/1.cc libstdc++-v3/testsuite/27_io/basic_filebuf/sgetn/char/1.cc
*** libstdc++-v3-orig/testsuite/27_io/basic_filebuf/sgetn/char/1.cc	Sun May 11 06:20:56 2003
--- libstdc++-v3/testsuite/27_io/basic_filebuf/sgetn/char/1.cc	Wed May 14 13:14:19 2003
*************** void test05() 
*** 123,133 ****
    VERIFY( c7 == c2 ); // n != i
    strmsz_1 = fb_03.sgetn(carray1, 10);
    VERIFY( !strmsz_1 ); //zero
!   strmsz_1 = fb_01.in_avail();
    strmsz_2 = fb_01.sgetn(carray2, strmsz_1 + 5);
    VERIFY( strmsz_1 == strmsz_2 - 5 ); 
    c4 = fb_01.sgetc(); // buffer should have underflowed from above.
!   VERIFY( c4 == 'i' );
    strmsz_1 = fb_01.in_avail();
    VERIFY( strmsz_1 > 0 );
    strmsz_2 = fb_01.sgetn(carray2, strmsz_1 + 5);
--- 123,133 ----
    VERIFY( c7 == c2 ); // n != i
    strmsz_1 = fb_03.sgetn(carray1, 10);
    VERIFY( !strmsz_1 ); //zero
!   strmsz_1 = fb_01.in_avail(); // N.B.: _M_in_end - _M_in_beg == BUFSIZ - 1
    strmsz_2 = fb_01.sgetn(carray2, strmsz_1 + 5);
    VERIFY( strmsz_1 == strmsz_2 - 5 ); 
    c4 = fb_01.sgetc(); // buffer should have underflowed from above.
!   VERIFY( c4 == 'h' );
    strmsz_1 = fb_01.in_avail();
    VERIFY( strmsz_1 > 0 );
    strmsz_2 = fb_01.sgetn(carray2, strmsz_1 + 5);
diff -prN libstdc++-v3-orig/testsuite/ext/stdio_filebuf_2.cc libstdc++-v3/testsuite/ext/stdio_filebuf_2.cc
*** libstdc++-v3-orig/testsuite/ext/stdio_filebuf_2.cc	Sat Apr 12 18:07:51 2003
--- libstdc++-v3/testsuite/ext/stdio_filebuf_2.cc	Wed May 14 13:15:21 2003
*************** void test01()
*** 39,47 ****
      using namespace __gnu_cxx;
      
      // One char big stack-based buffer.
!     stdio_filebuf<char> sbuf(file, ios_base::out, 1); 
      sbuf.sputc('T');
      sbuf.sputc('S');
    }
    fclose(file);
  
--- 39,48 ----
      using namespace __gnu_cxx;
      
      // One char big stack-based buffer.
!     stdio_filebuf<char> sbuf(file, ios_base::out, 2); 
      sbuf.sputc('T');
      sbuf.sputc('S');
+     sbuf.sputc('P');
    }
    fclose(file);
  
*************** void test01()
*** 51,58 ****
    streamsize n = fbuf.sgetn(buf, sizeof(buf));	
    fbuf.close();
    
!   VERIFY( n == 2 );
!   VERIFY( !memcmp(buf, "TS", 2) );
  }
  
  int main()
--- 52,59 ----
    streamsize n = fbuf.sgetn(buf, sizeof(buf));	
    fbuf.close();
    
!   VERIFY( n == 3 );
!   VERIFY( !memcmp(buf, "TSP", 3) );
  }
  
  int main()

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]