| Message ID | 09b876d2-a2bc-bc6d-3abc-c3e50d094144@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Make istreambuf_iterator::_M_sbuf immutable and add debug checks | expand |
Hi I completed execution of all tests and found one test impacted by this patch. It is a good example of the impact of the patch. Users won't be able to build a istreambuf_iterator at a point where the underlying streambuf is at end-of-stream and then put some data in the streambuf and then use the iterator. This is similar to what Petr was proposing, some eof iterator becoming valid again through an operation on the streambuf. I would prefer we forbid it completely or we accept it completely but current middle way situation is strange. The fix is easy, let the compiler build the streambuf_iterator when needed. Even if patch is not accepted I think we should keep the change on the test which is fragile. François On 13/10/2017 19:14, François Dumont wrote: > Hi > > Here is the last patch I will propose for istreambuf_iterator. > This is mostly to remove the mutable keyword on _M_sbuf. > > To do so I had to reset _M_sbuf in valid places that is to say > constructors and increment operators. Despite that we might still have > eof iterators with _M_sbuf not null when you have for instance several > iterator instance but only increment one. It seems fine to me because > even in this case iterator will still be considered as eof and using > several istreambuf_iterator to go through a given streambuf is not usual. > > As _M_sbuf is immutable I have been able to restore the simple > call to _M_at_eof() in the increment operators debug check. > > Ok to commit after successful tests ? > > François > > > diff --git a/libstdc++-v3/include/bits/streambuf_iterator.h b/libstdc++-v3/include/bits/streambuf_iterator.h index 081afe5..0a6c7f9 100644 --- a/libstdc++-v3/include/bits/streambuf_iterator.h +++ b/libstdc++-v3/include/bits/streambuf_iterator.h @@ -94,7 +94,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION // the "end of stream" iterator value. // NB: This implementation assumes the "end of stream" value // is EOF, or -1. - mutable streambuf_type* _M_sbuf; + streambuf_type* _M_sbuf; int_type _M_c; public: @@ -110,11 +110,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION /// Construct start of input stream iterator. istreambuf_iterator(istream_type& __s) _GLIBCXX_USE_NOEXCEPT - : _M_sbuf(__s.rdbuf()), _M_c(traits_type::eof()) { } + : _M_sbuf(__s.rdbuf()), _M_c(traits_type::eof()) + { _M_init(); } /// Construct start of streambuf iterator. istreambuf_iterator(streambuf_type* __s) _GLIBCXX_USE_NOEXCEPT - : _M_sbuf(__s), _M_c(traits_type::eof()) { } + : _M_sbuf(__s), _M_c(traits_type::eof()) + { _M_init(); } /// Return the current character pointed to by iterator. This returns /// streambuf.sgetc(). It cannot be assigned. NB: The result of @@ -138,13 +140,16 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION istreambuf_iterator& operator++() { - __glibcxx_requires_cond(_M_sbuf && - (!_S_is_eof(_M_c) || !_S_is_eof(_M_sbuf->sgetc())), + __glibcxx_requires_cond(!_M_at_eof(), _M_message(__gnu_debug::__msg_inc_istreambuf) ._M_iterator(*this)); _M_sbuf->sbumpc(); _M_c = traits_type::eof(); + + if (_S_is_eof(_M_sbuf->sgetc())) + _M_sbuf = 0; + return *this; } @@ -152,14 +157,17 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION istreambuf_iterator operator++(int) { - __glibcxx_requires_cond(_M_sbuf && - (!_S_is_eof(_M_c) || !_S_is_eof(_M_sbuf->sgetc())), + __glibcxx_requires_cond(!_M_at_eof(), _M_message(__gnu_debug::__msg_inc_istreambuf) ._M_iterator(*this)); istreambuf_iterator __old = *this; __old._M_c = _M_sbuf->sbumpc(); _M_c = traits_type::eof(); + + if (_S_is_eof(_M_sbuf->sgetc())) + _M_sbuf = __old._M_sbuf = 0; + return __old; } @@ -172,12 +180,20 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { return _M_at_eof() == __b._M_at_eof(); } private: + void + _M_init() + { + if (_M_sbuf && _S_is_eof(_M_sbuf->sgetc())) + _M_sbuf = 0; + } + int_type _M_get() const { int_type __ret = _M_c; - if (_M_sbuf && _S_is_eof(__ret) && _S_is_eof(__ret = _M_sbuf->sgetc())) - _M_sbuf = 0; + if (_M_sbuf && __builtin_expect(_S_is_eof(__ret), true)) + __ret = _M_sbuf->sgetc(); + return __ret; } @@ -391,10 +407,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION __c = __sb->snextc(); } + if (!traits_type::eq_int_type(__c, __eof)) + { __first._M_c = __eof; + return __first; + } } - return __first; + return __last; } // @} group iterators diff --git a/libstdc++-v3/testsuite/22_locale/money_get/get/char/9.cc b/libstdc++-v3/testsuite/22_locale/money_get/get/char/9.cc index 9b69956..476e38f 100644 --- a/libstdc++-v3/testsuite/22_locale/money_get/get/char/9.cc +++ b/libstdc++-v3/testsuite/22_locale/money_get/get/char/9.cc @@ -41,7 +41,6 @@ int main() = std::use_facet<std::money_get<char> >(liffey.getloc()); typedef std::istreambuf_iterator<char> iterator_type; - iterator_type is(liffey); iterator_type end; std::ios_base::iostate err01 = std::ios_base::goodbit; @@ -50,7 +49,7 @@ int main() // Feed it 1 digit too many, which should fail. liffey.str("12.3456"); - greed.get(is, end, false, liffey, err01, coins); + greed.get(liffey, end, false, liffey, err01, coins); if (! (err01 & std::ios_base::failbit )) fails |= 0x01; @@ -58,7 +57,7 @@ int main() // Feed it exactly what it wants, which should succeed. liffey.str("12.345"); - greed.get(is, end, false, liffey, err01, coins); + greed.get(liffey, end, false, liffey, err01, coins); if ( err01 & std::ios_base::failbit ) fails |= 0x02; @@ -66,7 +65,7 @@ int main() // Feed it 1 digit too few, which should fail. liffey.str("12.34"); - greed.get(is, end, false, liffey, err01, coins); + greed.get(liffey, end, false, liffey, err01, coins); if (! ( err01 & std::ios_base::failbit )) fails |= 0x04; @@ -74,7 +73,7 @@ int main() // Feed it only a decimal-point, which should fail. liffey.str("12."); - greed.get(is, end, false, liffey, err01, coins); + greed.get(liffey, end, false, liffey, err01, coins); if (! (err01 & std::ios_base::failbit )) fails |= 0x08; @@ -82,7 +81,7 @@ int main() // Feed it no decimal-point at all, which should succeed. liffey.str("12"); - greed.get(is, end, false, liffey, err01, coins); + greed.get(liffey, end, false, liffey, err01, coins); if ( err01 & std::ios_base::failbit ) fails |= 0x10; diff --git a/libstdc++-v3/testsuite/22_locale/money_get/get/wchar_t/9.cc b/libstdc++-v3/testsuite/22_locale/money_get/get/wchar_t/9.cc index a08a713..e5f8def 100644 --- a/libstdc++-v3/testsuite/22_locale/money_get/get/wchar_t/9.cc +++ b/libstdc++-v3/testsuite/22_locale/money_get/get/wchar_t/9.cc @@ -41,7 +41,6 @@ int main() = std::use_facet<std::money_get<wchar_t> >(liffey.getloc()); typedef std::istreambuf_iterator<wchar_t> iterator_type; - iterator_type is(liffey); iterator_type end; std::ios_base::iostate err01 = std::ios_base::goodbit; @@ -50,7 +49,7 @@ int main() // Feed it 1 digit too many, which should fail. liffey.str(L"12.3456"); - greed.get(is, end, false, liffey, err01, coins); + greed.get(liffey, end, false, liffey, err01, coins); if (! (err01 & std::ios_base::failbit )) fails |= 0x01; @@ -58,7 +57,7 @@ int main() // Feed it exactly what it wants, which should succeed. liffey.str(L"12.345"); - greed.get(is, end, false, liffey, err01, coins); + greed.get(liffey, end, false, liffey, err01, coins); if ( err01 & std::ios_base::failbit ) fails |= 0x02; @@ -66,7 +65,7 @@ int main() // Feed it 1 digit too few, which should fail. liffey.str(L"12.34"); - greed.get(is, end, false, liffey, err01, coins); + greed.get(liffey, end, false, liffey, err01, coins); if (! ( err01 & std::ios_base::failbit )) fails |= 0x04; @@ -74,7 +73,7 @@ int main() // Feed it only a decimal-point, which should fail. liffey.str(L"12."); - greed.get(is, end, false, liffey, err01, coins); + greed.get(liffey, end, false, liffey, err01, coins); if (! (err01 & std::ios_base::failbit )) fails |= 0x08; @@ -82,7 +81,7 @@ int main() // Feed it no decimal-point at all, which should succeed. liffey.str(L"12"); - greed.get(is, end, false, liffey, err01, coins); + greed.get(liffey, end, false, liffey, err01, coins); if ( err01 & std::ios_base::failbit ) fails |= 0x10; diff --git a/libstdc++-v3/testsuite/24_iterators/istreambuf_iterator/debug/1_neg.cc b/libstdc++-v3/testsuite/24_iterators/istreambuf_iterator/debug/1_neg.cc new file mode 100644 index 0000000..241fc58 --- /dev/null +++ b/libstdc++-v3/testsuite/24_iterators/istreambuf_iterator/debug/1_neg.cc @@ -0,0 +1,35 @@ +// 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/>. + +// { dg-do run { xfail *-*-* } } +// { dg-require-debug-mode "" } + +#include <iterator> + +void test01() +{ + typedef std::istreambuf_iterator<char> cistreambuf_iter; + + cistreambuf_iter eof; + ++eof; // Invalid. +} + +int main() +{ + test01(); + return 0; +} diff --git a/libstdc++-v3/testsuite/24_iterators/istreambuf_iterator/debug/2_neg.cc b/libstdc++-v3/testsuite/24_iterators/istreambuf_iterator/debug/2_neg.cc new file mode 100644 index 0000000..407f00b --- /dev/null +++ b/libstdc++-v3/testsuite/24_iterators/istreambuf_iterator/debug/2_neg.cc @@ -0,0 +1,35 @@ +// 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/>. + +// { dg-do run { xfail *-*-* } } +// { dg-require-debug-mode "" } + +#include <iterator> + +void test01() +{ + typedef std::istreambuf_iterator<char> cistreambuf_iter; + + cistreambuf_iter eof; + eof++; // Invalid. +} + +int main() +{ + test01(); + return 0; +}
Hi Any final decision regarding this patch ? François On 23/10/2017 21:08, François Dumont wrote: > Hi > > I completed execution of all tests and found one test impacted by > this patch. > > It is a good example of the impact of the patch. Users won't be > able to build a istreambuf_iterator at a point where the underlying > streambuf is at end-of-stream and then put some data in the streambuf > and then use the iterator. This is similar to what Petr was proposing, > some eof iterator becoming valid again through an operation on the > streambuf. I would prefer we forbid it completely or we accept it > completely but current middle way situation is strange. > > The fix is easy, let the compiler build the streambuf_iterator > when needed. Even if patch is not accepted I think we should keep the > change on the test which is fragile. > > François > > > On 13/10/2017 19:14, François Dumont wrote: >> Hi >> >> Here is the last patch I will propose for istreambuf_iterator. >> This is mostly to remove the mutable keyword on _M_sbuf. >> >> To do so I had to reset _M_sbuf in valid places that is to say >> constructors and increment operators. Despite that we might still >> have eof iterators with _M_sbuf not null when you have for instance >> several iterator instance but only increment one. It seems fine to me >> because even in this case iterator will still be considered as eof >> and using several istreambuf_iterator to go through a given streambuf >> is not usual. >> >> As _M_sbuf is immutable I have been able to restore the simple >> call to _M_at_eof() in the increment operators debug check. >> >> Ok to commit after successful tests ? >> >> François >> >> >> > >
On Mon, 6 Nov 2017 22:19:22 +0100 François Dumont <frs.dumont@gmail.com> wrote: > Hi > > Any final decision regarding this patch ? > > François https://gcc.gnu.org/ml/libstdc++/2017-11/msg00036.html https://gcc.gnu.org/ml/libstdc++/2017-11/msg00035.html https://gcc.gnu.org/ml/libstdc++/2017-11/msg00037.html https://gcc.gnu.org/ml/libstdc++/2017-11/msg00034.html -- - ptr
On 16/11/17 08:51 +0300, Petr Ovtchenkov wrote: >On Mon, 6 Nov 2017 22:19:22 +0100 >François Dumont <frs.dumont@gmail.com> wrote: > >> Hi >> >> Any final decision regarding this patch ? >> >> François > >https://gcc.gnu.org/ml/libstdc++/2017-11/msg00036.html >https://gcc.gnu.org/ml/libstdc++/2017-11/msg00035.html >https://gcc.gnu.org/ml/libstdc++/2017-11/msg00037.html >https://gcc.gnu.org/ml/libstdc++/2017-11/msg00034.html It would be helpful if you two could collaborate and come up with a good solution, or at least discuss the pros and cons, instead of just sending competing patches.
On 16/11/17 10:57 +0000, Jonathan Wakely wrote: >On 16/11/17 08:51 +0300, Petr Ovtchenkov wrote: >>On Mon, 6 Nov 2017 22:19:22 +0100 >>François Dumont <frs.dumont@gmail.com> wrote: >> >>>Hi >>> >>> Any final decision regarding this patch ? >>> >>>François >> >>https://gcc.gnu.org/ml/libstdc++/2017-11/msg00036.html >>https://gcc.gnu.org/ml/libstdc++/2017-11/msg00035.html >>https://gcc.gnu.org/ml/libstdc++/2017-11/msg00037.html >>https://gcc.gnu.org/ml/libstdc++/2017-11/msg00034.html > >It would be helpful if you two could collaborate and come up with a >good solution, or at least discuss the pros and cons, instead of just >sending competing patches. Let me be more clear: I'm not going to review further patches in this area while you two are proposing different alternatives, without commenting on each other's approach. If you think your solution is better than François's solution, you should explain why, not just send a different patch. If François thinks his solution is better than yours, he should state why, not just send a different patch. I don't have time to infer all that from just your patches, so I'm not going to bother.
On Thu, 16 Nov 2017 11:46:48 +0000 Jonathan Wakely <jwakely@redhat.com> wrote: > On 16/11/17 10:57 +0000, Jonathan Wakely wrote: > >On 16/11/17 08:51 +0300, Petr Ovtchenkov wrote: > >>On Mon, 6 Nov 2017 22:19:22 +0100 > >>François Dumont <frs.dumont@gmail.com> wrote: > >> > >>>Hi > >>> > >>> Any final decision regarding this patch ? > >>> > >>>François > >> > >>https://gcc.gnu.org/ml/libstdc++/2017-11/msg00036.html > >>https://gcc.gnu.org/ml/libstdc++/2017-11/msg00035.html > >>https://gcc.gnu.org/ml/libstdc++/2017-11/msg00037.html > >>https://gcc.gnu.org/ml/libstdc++/2017-11/msg00034.html > > > >It would be helpful if you two could collaborate and come up with a > >good solution, or at least discuss the pros and cons, instead of just > >sending competing patches. > > > Let me be more clear: I'm not going to review further patches in this > area while you two are proposing different alternatives, without > commenting on each other's approach. > > If you think your solution is better than François's solution, you > should explain why, not just send a different patch. If François > thinks his solution is better than yours, he should state why, not > just send a different patch. > > I don't have time to infer all that from just your patches, so I'm not > going to bother. > References here is a notification that - there is another opinion; - discussion is in another thread. Nothing more.
On 16/11/2017 12:46, Jonathan Wakely wrote: > On 16/11/17 10:57 +0000, Jonathan Wakely wrote: >> On 16/11/17 08:51 +0300, Petr Ovtchenkov wrote: >>> On Mon, 6 Nov 2017 22:19:22 +0100 >>> François Dumont <frs.dumont@gmail.com> wrote: >>> >>>> Hi >>>> >>>> Any final decision regarding this patch ? >>>> >>>> François >>> >>> https://gcc.gnu.org/ml/libstdc++/2017-11/msg00036.html >>> https://gcc.gnu.org/ml/libstdc++/2017-11/msg00035.html >>> https://gcc.gnu.org/ml/libstdc++/2017-11/msg00037.html >>> https://gcc.gnu.org/ml/libstdc++/2017-11/msg00034.html >> >> It would be helpful if you two could collaborate and come up with a >> good solution, or at least discuss the pros and cons, instead of just >> sending competing patches. > > > Let me be more clear: I'm not going to review further patches in this > area while you two are proposing different alternatives, without > commenting on each other's approach. > > If you think your solution is better than François's solution, you > should explain why, not just send a different patch. If François > thinks his solution is better than yours, he should state why, not > just send a different patch. > > I don't have time to infer all that from just your patches, so I'm not > going to bother. > > Proposing to revert my patch doesn't sound to me like a friendly action to start a collaboration. My only concern has always been the Debug mode impact which is now fixed. I already said that I disagree with Petr's main goal to keep eof iterator linked to the underlying stream. So current implementation is just fine to me and I'll let Petr argument for any change. @Jonathan, You can ignore my last request to remove mutable keywork on _M_sbuf. François
On Thu, 16 Nov 2017 18:40:08 +0100 François Dumont <frs.dumont@gmail.com> wrote: > On 16/11/2017 12:46, Jonathan Wakely wrote: > > On 16/11/17 10:57 +0000, Jonathan Wakely wrote: > >> On 16/11/17 08:51 +0300, Petr Ovtchenkov wrote: > >>> On Mon, 6 Nov 2017 22:19:22 +0100 > >>> François Dumont <frs.dumont@gmail.com> wrote: > >>> > >>>> Hi > >>>> > >>>> Any final decision regarding this patch ? > >>>> > >>>> François > >>> > >>> https://gcc.gnu.org/ml/libstdc++/2017-11/msg00036.html > >>> https://gcc.gnu.org/ml/libstdc++/2017-11/msg00035.html > >>> https://gcc.gnu.org/ml/libstdc++/2017-11/msg00037.html > >>> https://gcc.gnu.org/ml/libstdc++/2017-11/msg00034.html > >> > >> It would be helpful if you two could collaborate and come up with a > >> good solution, or at least discuss the pros and cons, instead of just > >> sending competing patches. > > > > > > Let me be more clear: I'm not going to review further patches in this > > area while you two are proposing different alternatives, without > > commenting on each other's approach. > > > > If you think your solution is better than François's solution, you > > should explain why, not just send a different patch. If François > > thinks his solution is better than yours, he should state why, not > > just send a different patch. > > > > I don't have time to infer all that from just your patches, so I'm not > > going to bother. > > > > > Proposing to revert my patch doesn't sound to me like a friendly action > to start a collaboration. I'm already say that this is technical issue: this patch present only in trunk yet. Series is more useful for applying in different branches. BTW, https://gcc.gnu.org/ml/libstdc++/2017-11/msg00037.html was inspired by you https://gcc.gnu.org/ml/libstdc++/2017-10/msg00029.html > > My only concern has always been the Debug mode impact which is now fixed. I hope I'm suggest identical behaviour for Debug and non-Debug mode (no difference in interaction with associated streambuf). > > I already said that I disagree with Petr's main goal to keep eof > iterator linked to the underlying stream. Ok. > So current implementation is > just fine to me and I'll let Petr argument for any change. Please, clear for me: what is the "current implementation"? Is it what we see now in trunk? > @Jonathan, > You can ignore my last request to remove mutable keywork on _M_sbuf. > > François > > -- - ptr
On 16/11/2017 19:12, Petr Ovtchenkov wrote: > On Thu, 16 Nov 2017 18:40:08 +0100 > François Dumont <frs.dumont@gmail.com> wrote: > >> On 16/11/2017 12:46, Jonathan Wakely wrote: >> >>> Let me be more clear: I'm not going to review further patches in this >>> area while you two are proposing different alternatives, without >>> commenting on each other's approach. >>> >>> If you think your solution is better than François's solution, you >>> should explain why, not just send a different patch. If François >>> thinks his solution is better than yours, he should state why, not >>> just send a different patch. >>> >>> I don't have time to infer all that from just your patches, so I'm not >>> going to bother. >>> >>> >> Proposing to revert my patch doesn't sound to me like a friendly action >> to start a collaboration. > I'm already say that this is technical issue: this patch present only in > trunk yet. Doesn't explain why you want to revert it. > Series is more useful for applying in different branches. Which branches ? There are mostly maintenance branches. None of your patches is fixing a bug so it won't go to a maintenance branch. > BTW, https://gcc.gnu.org/ml/libstdc++/2017-11/msg00037.html > was inspired by you https://gcc.gnu.org/ml/libstdc++/2017-10/msg00029.html Which was already rejected, why submitting it again ? > >> So current implementation is >> just fine to me and I'll let Petr argument for any change. > Please, clear for me: what is the "current implementation"? > Is it what we see now in trunk? Yes. Other proposals were mostly code quality changes. None of them had no impact so Jonathan decision to keep current implementation is just fine.
diff --git a/libstdc++-v3/include/bits/streambuf_iterator.h b/libstdc++-v3/include/bits/streambuf_iterator.h index 081afe5..ef22aa9 100644 --- a/libstdc++-v3/include/bits/streambuf_iterator.h +++ b/libstdc++-v3/include/bits/streambuf_iterator.h @@ -94,8 +94,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION // the "end of stream" iterator value. // NB: This implementation assumes the "end of stream" value // is EOF, or -1. - mutable streambuf_type* _M_sbuf; - int_type _M_c; + streambuf_type* _M_sbuf; + int_type _M_c; public: /// Construct end of input stream iterator. @@ -110,11 +110,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION /// Construct start of input stream iterator. istreambuf_iterator(istream_type& __s) _GLIBCXX_USE_NOEXCEPT - : _M_sbuf(__s.rdbuf()), _M_c(traits_type::eof()) { } + : _M_sbuf(__s.rdbuf()), _M_c(traits_type::eof()) + { _M_init(); } /// Construct start of streambuf iterator. istreambuf_iterator(streambuf_type* __s) _GLIBCXX_USE_NOEXCEPT - : _M_sbuf(__s), _M_c(traits_type::eof()) { } + : _M_sbuf(__s), _M_c(traits_type::eof()) + { _M_init(); } /// Return the current character pointed to by iterator. This returns /// streambuf.sgetc(). It cannot be assigned. NB: The result of @@ -138,13 +140,16 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION istreambuf_iterator& operator++() { - __glibcxx_requires_cond(_M_sbuf && - (!_S_is_eof(_M_c) || !_S_is_eof(_M_sbuf->sgetc())), + __glibcxx_requires_cond(!_M_at_eof(), _M_message(__gnu_debug::__msg_inc_istreambuf) ._M_iterator(*this)); _M_sbuf->sbumpc(); _M_c = traits_type::eof(); + + if (_S_is_eof(_M_sbuf->sgetc())) + _M_sbuf = 0; + return *this; } @@ -152,14 +157,17 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION istreambuf_iterator operator++(int) { - __glibcxx_requires_cond(_M_sbuf && - (!_S_is_eof(_M_c) || !_S_is_eof(_M_sbuf->sgetc())), + __glibcxx_requires_cond(!_M_at_eof(), _M_message(__gnu_debug::__msg_inc_istreambuf) ._M_iterator(*this)); istreambuf_iterator __old = *this; __old._M_c = _M_sbuf->sbumpc(); _M_c = traits_type::eof(); + + if (_S_is_eof(_M_sbuf->sgetc())) + _M_sbuf = __old._M_sbuf = 0; + return __old; } @@ -172,12 +180,20 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { return _M_at_eof() == __b._M_at_eof(); } private: + void + _M_init() + { + if (_M_sbuf && _S_is_eof(_M_sbuf->sgetc())) + _M_sbuf = 0; + } + int_type _M_get() const { int_type __ret = _M_c; - if (_M_sbuf && _S_is_eof(__ret) && _S_is_eof(__ret = _M_sbuf->sgetc())) - _M_sbuf = 0; + if (_M_sbuf && __builtin_expect(_S_is_eof(__ret), true)) + __ret = _M_sbuf->sgetc(); + return __ret; } @@ -391,10 +407,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION __c = __sb->snextc(); } - __first._M_c = __eof; + if (!traits_type::eq_int_type(__c, __eof)) + { + __first._M_c = __eof; + return __first; + } } - return __first; + return __last; } // @} group iterators diff --git a/libstdc++-v3/testsuite/24_iterators/istreambuf_iterator/debug/1_neg.cc b/libstdc++-v3/testsuite/24_iterators/istreambuf_iterator/debug/1_neg.cc new file mode 100644 index 0000000..241fc58 --- /dev/null +++ b/libstdc++-v3/testsuite/24_iterators/istreambuf_iterator/debug/1_neg.cc @@ -0,0 +1,35 @@ +// 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/>. + +// { dg-do run { xfail *-*-* } } +// { dg-require-debug-mode "" } + +#include <iterator> + +void test01() +{ + typedef std::istreambuf_iterator<char> cistreambuf_iter; + + cistreambuf_iter eof; + ++eof; // Invalid. +} + +int main() +{ + test01(); + return 0; +} diff --git a/libstdc++-v3/testsuite/24_iterators/istreambuf_iterator/debug/2_neg.cc b/libstdc++-v3/testsuite/24_iterators/istreambuf_iterator/debug/2_neg.cc new file mode 100644 index 0000000..407f00b --- /dev/null +++ b/libstdc++-v3/testsuite/24_iterators/istreambuf_iterator/debug/2_neg.cc @@ -0,0 +1,35 @@ +// 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/>. + +// { dg-do run { xfail *-*-* } } +// { dg-require-debug-mode "" } + +#include <iterator> + +void test01() +{ + typedef std::istreambuf_iterator<char> cistreambuf_iter; + + cistreambuf_iter eof; + eof++; // Invalid. +} + +int main() +{ + test01(); + return 0; +}