On Thu, 28 Sep 2017 11:34:25 +0100 Jonathan Wakely <jwak...@redhat.com> wrote:
> On 23/09/17 09:54 +0300, Petr Ovtchenkov wrote: > >istreambuf_iterator should not forget about attached > >streambuf when it reach EOF. > > > >Checks in debug mode has no infuence more on character > >extraction in istreambuf_iterator increment operators. > >In this aspect behaviour in debug and non-debug mode > >is similar now. > > > >Test for detached srteambuf in istreambuf_iterator: > >When istreambuf_iterator reach EOF of istream, it should not > >forget about attached streambuf. > From fact "EOF in stream reached" not follow that > >stream reach end of life and input operation impossible > >more. > >--- > > libstdc++-v3/include/bits/streambuf_iterator.h | 41 +++++++-------- > > .../24_iterators/istreambuf_iterator/3.cc | 61 > > ++++++++++++++++++++++ > > 2 files changed, 80 insertions(+), 22 deletions(-) > > create mode 100644 > > libstdc++-v3/testsuite/24_iterators/istreambuf_iterator/3.cc > > > >diff --git a/libstdc++-v3/include/bits/streambuf_iterator.h > >b/libstdc++-v3/include/bits/streambuf_iterator.h index f0451b1..45c3d89 > >100644 > >--- a/libstdc++-v3/include/bits/streambuf_iterator.h > >+++ b/libstdc++-v3/include/bits/streambuf_iterator.h > >@@ -136,12 +136,20 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > istreambuf_iterator& > > operator++() > > { > >- __glibcxx_requires_cond(!_M_at_eof(), > >+ __glibcxx_requires_cond(_M_sbuf, > > _M_message(__gnu_debug::__msg_inc_istreambuf) > > ._M_iterator(*this)); > > if (_M_sbuf) > > { > >+#ifdef _GLIBCXX_DEBUG_PEDANTIC > >+ int_type _tmp = > >+#endif > > _M_sbuf->sbumpc(); > >+#ifdef _GLIBCXX_DEBUG_PEDANTIC > >+ > >__glibcxx_requires_cond(!traits_type::eq_int_type(_tmp,traits_type::eof()), > >+ > >_M_message(__gnu_debug::__msg_inc_istreambuf) > >+ ._M_iterator(*this)); > >+#endif > > _M_c = traits_type::eof(); > > } > > return *this; > >@@ -151,14 +159,16 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > istreambuf_iterator > > operator++(int) > > { > >- __glibcxx_requires_cond(!_M_at_eof(), > >+ _M_get(); > >+ __glibcxx_requires_cond(_M_sbuf > >+ && > >!traits_type::eq_int_type(_M_c,traits_type::eof()), > > _M_message(__gnu_debug::__msg_inc_istreambuf) > > ._M_iterator(*this)); > > > > istreambuf_iterator __old = *this; > > if (_M_sbuf) > > { > >- __old._M_c = _M_sbuf->sbumpc(); > >+ _M_sbuf->sbumpc(); > > _M_c = traits_type::eof(); > > } > > return __old; > >@@ -177,18 +187,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > _M_get() const > > { > > const int_type __eof = traits_type::eof(); > >- int_type __ret = __eof; > >- if (_M_sbuf) > >- { > >- if (!traits_type::eq_int_type(_M_c, __eof)) > >- __ret = _M_c; > >- else if (!traits_type::eq_int_type((__ret = _M_sbuf->sgetc()), > >- __eof)) > >- _M_c = __ret; > >- else > >- _M_sbuf = 0; > >- } > >- return __ret; > >+ if (_M_sbuf && traits_type::eq_int_type(_M_c, __eof)) > >+ _M_c = _M_sbuf->sgetc(); > >+ return _M_c; > > } > > > > bool > >@@ -339,7 +340,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > typedef typename __is_iterator_type::streambuf_type streambuf_type; > > typedef typename traits_type::int_type int_type; > > > >- if (__first._M_sbuf && !__last._M_sbuf) > >+ if (__first._M_sbuf && (__last == istreambuf_iterator<_CharT>())) > > { > > streambuf_type* __sb = __first._M_sbuf; > > int_type __c = __sb->sgetc(); > >@@ -374,7 +375,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > typedef typename __is_iterator_type::streambuf_type streambuf_type; > > typedef typename traits_type::int_type int_type; > > > >- if (__first._M_sbuf && !__last._M_sbuf) > >+ if (__first._M_sbuf && (__last == istreambuf_iterator<_CharT>())) > > { > > const int_type __ival = traits_type::to_int_type(__val); > > streambuf_type* __sb = __first._M_sbuf; > >@@ -395,11 +396,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > else > > __c = __sb->snextc(); > > } > >- > >- if (!traits_type::eq_int_type(__c, traits_type::eof())) > >- __first._M_c = __c; > >- else > >- __first._M_sbuf = 0; > >+ __first._M_c = __c; > > } > > return __first; > > } > >diff --git a/libstdc++-v3/testsuite/24_iterators/istreambuf_iterator/3.cc > >b/libstdc++-v3/testsuite/24_iterators/istreambuf_iterator/3.cc new file mode > >100644 > >index 0000000..803ede4 > >--- /dev/null > >+++ b/libstdc++-v3/testsuite/24_iterators/istreambuf_iterator/3.cc > >@@ -0,0 +1,61 @@ > >+// { dg-options "-std=gnu++17" } > >+ > >+// Copyright (C) 2017 Free Software Foundation, Inc. > >+// > >+// This file is part of the GNU ISO C++ Library. This library is free > >+// software; you can redistribute it and/or modify it under the > >+// terms of the GNU General Public License as published by the > >+// Free Software Foundation; either version 3, or (at your option) > >+// any later version. > >+ > >+// This library is distributed in the hope that it will be useful, > >+// but WITHOUT ANY WARRANTY; without even the implied warranty of > >+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > >+// GNU General Public License for more details. > >+ > >+// You should have received a copy of the GNU General Public License along > >+// with this library; see the file COPYING3. If not see > >+// <http://www.gnu.org/licenses/>. > >+ > >+#include <algorithm> > >+#include <sstream> > >+#include <iterator> > >+#include <cstring> > >+#include <testsuite_hooks.h> > >+ > >+void test03() > >+{ > >+ using namespace std; > >+ bool test __attribute__((unused)) = true; > >+ > >+ std::stringstream s; > >+ char b[] = "c2ee3d09-43b3-466d-b490-db35999a22cf"; > >+ char r[] = "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"; > >+ char q[] = "3c4852d6-d47b-4f46-b05e-b5edc1aa440e"; > >+ // 012345678901234567890123456789012345 > >+ // 0 1 2 3 > >+ s << b; > >+ VERIFY( !s.fail() ); > >+ > >+ istreambuf_iterator<char> i(s); > >+ copy_n(i, 36, r); > >+ ++i; // EOF reached > >+ VERIFY(i == std::istreambuf_iterator<char>()); > >+ > >+ VERIFY(memcmp(b, r, 36) == 0); > >+ > >+ s << q; > >+ VERIFY(!s.fail()); > >+ > >+ copy_n(i, 36, r); > > This is undefined behaviour. The end-of-stream iterator value cannot > be dereferenced. Within this test istreambuf_iterator in eof state never dereferenced. The test itself simulate "stop and go" istream usage. stringstream is convenient for behaviuor illustration, but in "real life" I can assume socket or tty on this place. debugmode-dependent behaviour is also issue in this patch; I don't suggest it as a separate patch because solutions are intersect. WBR, -- - ptr