On 10/09/20 5:05 pm, Jonathan Wakely wrote:
On 09/09/20 22:11 +0200, François Dumont via Libstdc++ wrote:
libstdc++: Use only public basic_streambuf methods in __copy_move_a2 overload

__copy_move_a2 for istreambuf_iterator can be implemented using public
basic_streambuf in_avail and sgetn so that __copy_move_a2 do not need to be
basic_streambuf friend.

libstdc++-v3/ChangeLog:

        * include/std/streambuf (__copy_move_a2): Remove friend declaration.         * include/bits/streambuf_iterator.h (__copy_move_a2): Re-implement using
        streambuf in_avail and sgetn.

Does this work?
tests are passing yes, and there are a good number of it for this code.

The original code doesn't use any virtual functions except underflow,
which is called to refill the get area.

I see, I really didn't consider the virtual function calls. So I just thought that less friend declaration and simpler code was better, my fault.


The new code calls in_avail() which is equivalent to the old
__sb->egptr() - __sb->gptr() code. If that returns a non-zero value
we'll use sgetn() to read that many characters (which makes a virtual
call to xsgetn). Then we call in_avail() again, which will now make a
virtual call to showmanyc(). There is no guarantee that showmanyc()
will return the correct value. It might return a conservative
underestimate, possibly even zero. If it returns zero then you break
out of the loop and return, but you haven't reached EOF.

I was concern about the 'If' in the showmanyc doc: If it returns a positive value... But I think I forgot when I saw that tests were passing.

So, let's forget this patch.

Thanks for the feedback.



Even if showmanyc() returns an accurate value (e.g. for filebuf it can
use a system call to find the size of the file on disk) you'll now
make another xsgetn() virtual call and showmanyc() virtual call for
each buffer's worth of characters. The original code only makes one
virtual call per buffer's worth of characters.


Tested under Linux x86_64.

Ok to commit ?

François


diff --git a/libstdc++-v3/include/bits/streambuf_iterator.h b/libstdc++-v3/include/bits/streambuf_iterator.h
index 184c82cd5bf..8712b90edd6 100644
--- a/libstdc++-v3/include/bits/streambuf_iterator.h
+++ b/libstdc++-v3/include/bits/streambuf_iterator.h
@@ -367,31 +367,19 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
           istreambuf_iterator<_CharT> __last, _CharT* __result)
    {
      typedef istreambuf_iterator<_CharT> __is_iterator_type;
-      typedef typename __is_iterator_type::traits_type traits_type;
      typedef typename __is_iterator_type::streambuf_type streambuf_type;
-      typedef typename traits_type::int_type int_type;

      if (__first._M_sbuf && !__last._M_sbuf)
    {
      streambuf_type* __sb = __first._M_sbuf;
-      int_type __c = __sb->sgetc();
-      while (!traits_type::eq_int_type(__c, traits_type::eof()))
+      std::streamsize __avail = __sb->in_avail();
+      while (__avail > 0)
        {
-          const streamsize __n = __sb->egptr() - __sb->gptr();
-          if (__n > 1)
-        {
-          traits_type::copy(__result, __sb->gptr(), __n);
-          __sb->__safe_gbump(__n);
-          __result += __n;
-          __c = __sb->underflow();
-        }
-          else
-        {
-          *__result++ = traits_type::to_char_type(__c);
-          __c = __sb->snextc();
-        }
+          __result += __sb->sgetn(__result, __avail);
+          __avail = __sb->in_avail();
        }
    }
+
      return __result;
    }

diff --git a/libstdc++-v3/include/std/streambuf b/libstdc++-v3/include/std/streambuf
index cae35e75bda..13db284eb58 100644
--- a/libstdc++-v3/include/std/streambuf
+++ b/libstdc++-v3/include/std/streambuf
@@ -149,12 +149,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
      friend streamsize
      __copy_streambufs_eof<>(basic_streambuf*, basic_streambuf*, bool&);

-      template<bool _IsMove, typename _CharT2>
-        friend typename __gnu_cxx::__enable_if<__is_char<_CharT2>::__value,
-                           _CharT2*>::__type
-        __copy_move_a2(istreambuf_iterator<_CharT2>,
-               istreambuf_iterator<_CharT2>, _CharT2*);
-
      template<typename _CharT2>
        friend typename __gnu_cxx::__enable_if<__is_char<_CharT2>::__value,
                  istreambuf_iterator<_CharT2> >::__type


Reply via email to