Re: [RFC] A QoI problem with filebuf and POD charT != char, wchar_t

[Paolo Carlini <pcarlini at suse dot de>]

Paolo Carlini wrote:

> ... well, /almost/ identically, of course: a few sizeof(_CharT) would 
> have
> to be sprinkled here and there.

Hi again.

For concreteness, I'm attaching below a draft patch, already regtested and
lightly tested on a few testcases similar to the posted one.

I briefly recall that the general philosophy is as follows: a codecvt 
facet not
available is basically equivalent to always_noconv. In order to deal 
with both
in the same way we only need to take into account sizeof(char_type). 
When (the
most common case) char_type == char, no computational price is paid since
sizeof(char_type) == 1 and no actual divisions or any other additional code
is really involved.

If no one finds something basically flawed with this approach, I'll 
refine the
patch further, prepare a basic set of new testcases (probably using a 
short type
as char_type), and post the whole in a few days.

Thanks,
Paolo.

/////////////
diff -urN libstdc++-v3-orig/include/bits/fstream.tcc libstdc++-v3/include/bits/fstream.tcc
--- libstdc++-v3-orig/include/bits/fstream.tcc	2004-04-16 18:06:18.000000000 +0200
+++ libstdc++-v3/include/bits/fstream.tcc	2004-05-19 13:43:44.000000000 +0200
@@ -166,8 +166,10 @@
 	  // For a stateful encoding (-1) the pending sequence might be just
 	  // shift and unshift prefixes with no actual character.
 	  __ret = this->egptr() - this->gptr();
-	  if (__check_facet(_M_codecvt).encoding() >= 0)
+	  if (_M_codecvt && _M_codecvt->encoding() >= 0)
 	    __ret += _M_file.showmanyc() / _M_codecvt->max_length();
+	  else if (!_M_codecvt)
+	    __ret += _M_file.showmanyc() / sizeof(char_type);
 	}
       return __ret;
     }
@@ -198,12 +200,14 @@
 	  // Number of internal characters produced.
 	  streamsize __ilen = 0;
 	  codecvt_base::result __r = codecvt_base::ok;
-	  if (__check_facet(_M_codecvt).always_noconv())
+	  if (!_M_codecvt || _M_codecvt->always_noconv())
 	    {
 	      __ilen = _M_file.xsgetn(reinterpret_cast<char*>(this->eback()),
-				      __buflen);
+				      __buflen * sizeof(char_type));
 	      if (__ilen == 0)
 		__got_eof = true;
+	      if (__ilen > 0)
+		__ilen /= sizeof(char_type);
 	    }
 	  else
 	    {
@@ -276,7 +280,8 @@
 		      size_t __avail = _M_ext_end - _M_ext_buf;
 		      __ilen = std::min(__avail, __buflen);
 		      traits_type::copy(this->eback(),
-					reinterpret_cast<char_type*>(_M_ext_buf), __ilen);
+					reinterpret_cast<char_type*>(_M_ext_buf),
+					__ilen);
 		      _M_ext_next = _M_ext_buf + __ilen;
 		    }
 		  else
@@ -439,10 +444,10 @@
       // Sizes of external and pending output.
       streamsize __elen;
       streamsize __plen;
-      if (__check_facet(_M_codecvt).always_noconv())
+      if (!_M_codecvt || _M_codecvt->always_noconv())
 	{
-	  __elen = _M_file.xsputn(reinterpret_cast<char*>(__ibuf), __ilen);
-	  __plen = __ilen;
+	  __plen = __ilen * sizeof(char_type);
+	  __elen = _M_file.xsputn(reinterpret_cast<char*>(__ibuf), __plen);
 	}
       else
 	{
@@ -505,7 +510,7 @@
        streamsize __ret = 0;
        const bool __testout = this->_M_mode & ios_base::out;
        if (__testout && !_M_reading
-	   && __check_facet(_M_codecvt).always_noconv())
+	   && (!_M_codecvt || _M_codecvt->always_noconv()))
 	{
 	  // Measurement would reveal the best choice.
 	  const streamsize __chunk = 1ul << 10;
@@ -520,9 +525,10 @@
 	    {
 	      const streamsize __buffill = this->pptr() - this->pbase();
 	      const char* __buf = reinterpret_cast<const char*>(this->pbase());
-	      __ret = _M_file.xsputn_2(__buf, __buffill,
-				       reinterpret_cast<const char*>(__s),
-				       __n);
+	      __ret = ((_M_file.xsputn_2(__buf, __buffill * sizeof(char_type),
+					 reinterpret_cast<const char*>(__s),
+					 __n * sizeof(char_type)))
+		       / sizeof(char_type));
 	      if (__ret == __buffill + __n)
 		{
 		  _M_set_buffer(0);
@@ -573,12 +579,7 @@
     basic_filebuf<_CharT, _Traits>::
     seekoff(off_type __off, ios_base::seekdir __way, ios_base::openmode)
     {
-      int __width = 0;
-      if (_M_codecvt)
-	__width = _M_codecvt->encoding();
-      if (__width < 0)
-	__width = 0;
-
+      const int __width = _M_codecvt ? _M_codecvt->encoding() : 1;
       pos_type __ret =  pos_type(off_type(-1));
       const bool __testfail = __off != 0 && __width <= 0;
       if (this->is_open() && !__testfail)
@@ -595,7 +596,7 @@
 	  off_type __computed_off = __off * __width;
 	  if (_M_reading && __way == ios_base::cur)
 	    {
-	      if (_M_codecvt->always_noconv())
+	      if (!_M_codecvt || _M_codecvt->always_noconv())
 		__computed_off += this->gptr() - this->egptr();
 	      else
 		{
@@ -645,7 +646,17 @@
       if (_M_terminate_output())
 	{
 	  // Returns pos_type(off_type(-1)) in case of failure.
-	  __ret = pos_type(_M_file.seekoff(__off, __way));
+	  streamoff __seek;
+	  if (!_M_codecvt || _M_codecvt->always_noconv())
+	    {
+	      __seek = _M_file.seekoff(__off * sizeof(char_type),
+				       __way);
+	      if (__seek > 0)
+		__seek /= sizeof(char_type);
+	    }
+	  else
+	    __seek = _M_file.seekoff(__off, __way);
+	  __ret = pos_type(__seek);
 	  _M_reading = false;
 	  _M_writing = false;
 	  _M_ext_next = _M_ext_end = _M_ext_buf;
@@ -671,7 +682,7 @@
 	}
 
       // Part two: output unshift sequence.
-      if (_M_writing && !__check_facet(_M_codecvt).always_noconv()
+      if (_M_writing && _M_codecvt && !_M_codecvt->always_noconv()
 	  && __testvalid)
 	{
 	  // Note: this value is arbitrary, since there is no way to
@@ -750,24 +761,25 @@
 	{
 	  // encoding() == -1 is ok only at the beginning.
 	  if ((_M_reading || _M_writing)
-	      && __check_facet(_M_codecvt).encoding() == -1)
+	      && _M_codecvt && _M_codecvt->encoding() == -1)
 	    __testvalid = false;
 	  else
 	    {
 	      if (_M_reading)
 		{
-		  if (__check_facet(_M_codecvt).always_noconv())
+		  if (!_M_codecvt || _M_codecvt->always_noconv())
 		    {
-		      if (_M_codecvt_tmp
-			  && !__check_facet(_M_codecvt_tmp).always_noconv())
-			__testvalid = this->seekoff(0, ios_base::cur, this->_M_mode)
-			              != pos_type(off_type(-1));
+		      if (_M_codecvt_tmp && !_M_codecvt_tmp->always_noconv())
+			__testvalid = (this->seekoff(0, ios_base::cur,
+						     this->_M_mode)
+				       != pos_type(off_type(-1)));
 		    }
 		  else
 		    {
 		      // External position corresponding to gptr().
 		      _M_ext_next = _M_ext_buf
-			+ _M_codecvt->length(_M_state_last, _M_ext_buf, _M_ext_next,
+			+ _M_codecvt->length(_M_state_last, _M_ext_buf,
+					     _M_ext_next,
 					     this->gptr() - this->eback());
 		      const streamsize __remainder = _M_ext_end - _M_ext_next;
 		      if (__remainder)