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]

RFC: basic_filebuf supporting non-modal I/O without seekpos


Hi all,

If the user performs a basic_filebuf (or fstream) input operation immediately followed by an output operation, or vice versa, the second operation fails, and that is confusing and non-compliant. The Standard does not describe separate modes for reading and writing, and it certainly doesn't describe seekpos (called by the tellp or tellg methods of fstream) as having the side effect of switching between such modes.

Eliminating the modal interface does not mean eliminating the modal implementation. For performance reasons, a filebuf may only have either a get or a put area at any given time. This is necessary to allow direct modification of them by super-speedy functions like sgetc. The current implementation is mostly correct; it is simply missing a few mode transitions. The fix amounts to trapping and correcting the "we're in the wrong state!" errors that currently abort. I believe the attached patch is minimal, and cannot possibly have adverse side-effects. It passes the testsuite on the latest 4.5.2, aside from two bogus tests which do nothing except check for the incorrect behavior by replacing seekpos with sync and verifying failure. (I cannot seem to run the performance suite; perhaps that requires 4.6?)

To summarize the changes: Read-mode functions which previously aborted in write-mode call overflow(eof), enter uncommitted mode, and continue. This is OK because the postcondition of overflow(eof) is that the put area is empty. It is trivial to transition to uncommitted mode at that point. Write-mode method overflow, which previously aborted in read mode, sets the file position by calling _M_seek and continues. This is quite like calling seekpos, and I moved some code from seekpos into a new function so that overflow may calculate the parameter for _M_seek.

So, the new code is entirely guarded by simple Boolean conditions which previously caused a crash-and-burn. No existing state transitions were modified; no new states were created. This patch can alleviate the justified confusion and suspicion of iostreams newbies, so please take it into consideration.

	- Cheers,
	David

Index: include/bits/fstream.tcc
===================================================================
--- include/bits/fstream.tcc    (revision 164378)
+++ include/bits/fstream.tcc    (working copy)
@@ -205,8 +205,16 @@
     {
       int_type __ret = traits_type::eof();
       const bool __testin = _M_mode & ios_base::in;
-      if (__testin && !_M_writing)
+      if (__testin)
        {
+         if (_M_writing)
+           {
+             __ret = overflow();
+             if (__ret == traits_type::eof())
+               return __ret;
+             _M_set_buffer(-1);
+             _M_writing = false;
+           }
          // Check for pback madness, and if so switch back to the
          // normal buffers and jet outta here before expensive
          // fileops happen...
@@ -357,8 +365,16 @@
     {
       int_type __ret = traits_type::eof();
       const bool __testin = _M_mode & ios_base::in;
-      if (__testin && !_M_writing)
+      if (__testin)
        {
+         if (_M_writing)
+           {
+             __ret = overflow();
+             if (__ret == traits_type::eof())
+               return __ret;
+             _M_set_buffer(-1);
+             _M_writing = false;
+           }
          // Remember whether the pback buffer is active, otherwise below
          // we may try to store in it a second char (libstdc++/9761).
          const bool __testpb = _M_pback_init;
@@ -410,8 +426,16 @@
       int_type __ret = traits_type::eof();
       const bool __testeof = traits_type::eq_int_type(__c, __ret);
       const bool __testout = _M_mode & ios_base::out;
-      if (__testout && !_M_reading)
+      if (__testout)
        {
+         if (_M_reading)
+           {
+             _M_destroy_pback();
+             const int __gptr_off = _M_get_ext_pos(_M_state_last);
+             if (_M_seek(__gptr_off, ios_base::cur, _M_state_last)
+                 == pos_type(off_type(-1)))
+               return __ret;
+           }
          if (this->pbase() < this->pptr())
            {
              // If appropriate, append the overflow char.
@@ -707,22 +731,8 @@
          off_type __computed_off = __off * __width;
          if (_M_reading && __way == ios_base::cur)
            {
-             if (_M_codecvt->always_noconv())
-               __computed_off += this->gptr() - this->egptr();
-             else
-               {
-                 // Calculate offset from _M_ext_buf that corresponds
-                 // to gptr(). Note: uses _M_state_last, which
-                 // corresponds to eback().
-                 const int __gptr_off =
-                   _M_codecvt->length(_M_state_last, _M_ext_buf, _M_ext_next,
-                                      this->gptr() - this->eback());
-                 __computed_off += _M_ext_buf + __gptr_off - _M_ext_end;
-
-                 // _M_state_last is modified by codecvt::length() so
-                 // it now corresponds to gptr().
-                 __state = _M_state_last;
-               }
+             __computed_off += _M_get_ext_pos(_M_state_last);
+             __state = _M_state_last;
            }
          __ret = _M_seek(__computed_off, __way, __state);
        }
@@ -771,7 +781,28 @@
       return __ret;
     }
 
+  // Returns the distance from the end of the ext buffer to the point
+  // corresponding to gptr(). This is a negative value. Updates __state
+  // from eback() correspondence to gptr().
   template<typename _CharT, typename _Traits>
+    int basic_filebuf<_CharT, _Traits>::
+    _M_get_ext_pos(__state_type &__state)
+    {
+      if (_M_codecvt->always_noconv())
+       return this->gptr() - this->egptr();
+      else
+       {
+         // Calculate offset from _M_ext_buf that corresponds to
+         // gptr(). Precondition: __state == _M_state_last, which
+         // corresponds to eback().
+         const int __gptr_off =
+           _M_codecvt->length(__state, _M_ext_buf, _M_ext_next,
+                              this->gptr() - this->eback());
+         return _M_ext_buf + __gptr_off - _M_ext_end;
+       }
+    }
+    
+  template<typename _CharT, typename _Traits>
     bool
     basic_filebuf<_CharT, _Traits>::
     _M_terminate_output()
Index: include/std/fstream
===================================================================
--- include/std/fstream (revision 164202)
+++ include/std/fstream (working copy)
@@ -351,9 +351,12 @@
       seekpos(pos_type __pos,
              ios_base::openmode __mode = ios_base::in | ios_base::out);
 
-      // Common code for seekoff and seekpos
+      // Common code for seekoff, seekpos, and overflow
       pos_type
       _M_seek(off_type __off, ios_base::seekdir __way, __state_type __state);
+      
+      int
+      _M_get_ext_pos(__state_type &__state);
 
       virtual int
       sync();


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