[iostreams]Is this a bug?
boost-1_34\boost\iostreams\copy.hpp line 80 - line 100 template<typename Source, typename Sink> std::streamsize copy_impl( Source& src, Sink& snk, std::streamsize buffer_size, mpl::false_, mpl::true_ ) { // Copy from an indirect Source to a direct Sink. using namespace std; typedef typename char_type_of<Source>::type char_type; typedef pair<char_type*, char_type*> pair_type; detail::basic_buffer<char_type> buf(buffer_size); pair_type p = snk.output_sequence(); streamsize total = 0; bool done = false; while (!done) { streamsize amt; done = (amt = iostreams::read(src, buf.data(), buffer_size)) == -1; std::copy(buf.data(), buf.data() + amt, p.first + total); if (amt != -1) total += amt; } return total; } When "iostreams::read" return -1 ,amt equal -1,it will assert in std::copy because iterator last < first. I use MSVC8.0 . It crashed only in DEBUG mode. I think it should be: while (!done) { streamsize amt; done = (amt = iostreams::read(src, buf.data(), buffer_size)) == -1; if (amt != -1){ std::copy(buf.data(), buf.data() + amt, p.first + total); total += amt; } }
邱宇舟 wrote:
When "iostreams::read" return -1 ,amt equal -1,it will assert in std::copy because iterator last < first.
I use MSVC8.0 .
It crashed only in DEBUG mode.
I think this same issue was identified in a thread last December (on the users list)... and a patch was posted there. But I don't think it ever went anywhere from there. see: http://thread.gmane.org/gmane.comp.lib.boost.user/23462 Can you enter this issue into the new tracker so it doesn't get lost again. It can be found at: http://svn.boost.org/trac/boost/
On Friday 29 June 2007 16:49:18 邱宇舟 wrote:
boost-1_34\boost\iostreams\copy.hpp line 80 - line 100
template<typename Source, typename Sink> std::streamsize copy_impl( Source& src, Sink& snk, std::streamsize buffer_size, mpl::false_, mpl::true_ ) { // Copy from an indirect Source to a direct Sink. using namespace std; typedef typename char_type_of<Source>::type char_type; typedef pair<char_type*, char_type*> pair_type; detail::basic_buffer<char_type> buf(buffer_size); pair_type p = snk.output_sequence(); streamsize total = 0; bool done = false; while (!done) { streamsize amt; done = (amt = iostreams::read(src, buf.data(), buffer_size)) == -1; std::copy(buf.data(), buf.data() + amt, p.first + total); if (amt != -1) total += amt; } [...] When "iostreams::read" return -1 ,amt equal -1,it will assert in std::copy because iterator last < first.
I'd say it is a bug. Further, I find that code horribly obfuscated, it first takes the effort to fill 'done' in a multi-statement line, then ignores its content and instead checks the same condition again. Further I would even make it more local than that...
I think it should be:
while (!done) { streamsize amt; done = (amt = iostreams::read(src, buf.data(), buffer_size)) == -1; if (amt != -1){ std::copy(buf.data(), buf.data() + amt, p.first + total); total += amt; } }
Too complicated still (obviously), and the not-checking/double-computation of 'done' still applies. ;) I'd suggest this version: while(true) { // try to read a chunk of data streamsize amt = iostreams::read(...); if(amt==-1) break; // handle data std::copy(...); total += amt; } ...which also removes the unnecessary bool 'done'. Uli
Ulrich Eckhardt wrote:
Too complicated still (obviously), and the not-checking/double-computation of 'done' still applies. ;)
I'd suggest this version:
while(true) { // try to read a chunk of data streamsize amt = iostreams::read(...); if(amt==-1) break; // handle data std::copy(...); total += amt; }
...which also removes the unnecessary bool 'done'.
Please note that there are two overloads of copy_impl() which contain the original while logic. The one reported has the bug, while the other one (line 102- line 122) does not. If you are going to refactor this function, then the other one should be done the same way for consistency.
participants (4)
-
eg -
egoots
-
Ulrich Eckhardt -
邱宇舟