Re: [boost] [iostreams]Is this a bug?

From: Ulrich Eckhardt (doomster_at_[hidden])
Date: 2007-06-30 03:40:23

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