[iostreams] : bug in <boost/iostreams/copy.hpp> ?
Hello, i'm a new user to these great boost libraries... I think i found a bug in <boost/iostreams/copy.hpp> (version 1.33.1) the current code starting line 80 is 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; } I think it should be somthing like 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; streamsize amt = 0; //bool done = false; while ((amt = iostreams::read(src, buf.data(),buffer_size))!= -1) { std::copy(buf.data(), buf.data() + amt, p.first + total); total += amt; } return total; } The current code happens to crash whereas the proposed version works properly... Did I miss something or a I right ? Regards, Mathieu -- http://matioupi.free.fr/
Mathieu Peyréga wrote:
Hello,
i'm a new user to these great boost libraries... I think i found a bug in <boost/iostreams/copy.hpp> (version 1.33.1)
<snip>
The current code happens to crash whereas the proposed version works properly... Did I miss something or a I right ?
I am not a boost iostreams expert... just a user. Having said that, if iostreams::read() can return -1 during the copy_impl then it looks like the original code could be a problem. What was your test case to demonstrate this?
I am not a boost iostreams expert... just a user. Having said that, if iostreams::read() can return -1 during the copy_impl then it looks like the original code could be a problem.
and it happens... it is indeed the test used to detect the end of the loop...
What was your test case to demonstrate this?
was something like short *data = new short[w*h]; for(int index_ligne=0;index_ligne<w;++index_ligne) { TIFFReadScanline(handler,data+(index_ligne*w), index_ligne); } ofstream file(filename.c_str(), ios_base::out|ios_base::binary); filtering_streambuf<output> out; out.push(bzip2_compressor()); out.push(file); basic_array_source<char> datastream((char*)data,w*h*sizeof(short)); boost::iostreams::copy(datastream, out); Regards, Mathieu -- http://matioupi.free.fr/
Mathieu Peyréga wrote:
What was your test case to demonstrate this?
was something like
short *data = new short[w*h];
for(int index_ligne=0;index_ligne<w;++index_ligne) { TIFFReadScanline(handler,data+(index_ligne*w), index_ligne); }
ofstream file(filename.c_str(), ios_base::out|ios_base::binary); filtering_streambuf<output> out; out.push(bzip2_compressor()); out.push(file);
basic_array_source<char> datastream((char*)data,w*h*sizeof(short)); boost::iostreams::copy(datastream, out);
I just glanced at the sources in CVS and it appears that this code is unchanged in the head revision. I hope someone who has check-in privileges can look at this and confirm/apply the patch before 1.34 gets released.
I hope someone who has check-in privileges can look at this and confirm/apply the patch before 1.34 gets released.
I hope too... They could at least let us know what they think about this point... (as far as i'm concerned, the patch is applied on the boost sources i use) I saw that they prefer patch delivered in "patch" format, but i don't know how to do that (especially on a windows environnement...)
Mathieu Peyréga wrote:
I hope someone who has check-in privileges can look at this and confirm/apply the patch before 1.34 gets released.
I hope too... They could at least let us know what they think about this point... (as far as i'm concerned, the patch is applied on the boost sources i use)
I saw that they prefer patch delivered in "patch" format, but i don't know how to do that (especially on a windows environnement...)
I think the following change should work as well AND it mimics the copy_impl() function right below the one you changed... (i.e. the one that does a "Copy from an indirect Source to a indirect Sink...") It just moves the "if" statement up one line. So the while loop now looks like: 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; } } In any case, I have attached a patch file for this change if you want to pass it on. --- C:\dev\boost_1_33_1\boost\iostreams\copy.hpp Thu May 26 00:37:30 2005 +++ C:\dev\boost_1_33_1\boost\iostreams\Newcopy.hpp Mon Dec 11 17:14:34 2006 @@ -92,9 +92,10 @@ 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) + if (amt != -1) { + std::copy(buf.data(), buf.data() + amt, p.first + total); total += amt; + } } return total; }
eg wrote:
I think the following change should work as well AND it mimics the copy_impl() function right below the one you changed... (i.e. the one that does a "Copy from an indirect Source to a indirect Sink...")
It just moves the "if" statement up one line. So the while loop now looks like:
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; } }
In any case, I have attached a patch file for this change if you want to pass it on.
I should read the patch guidelines before posting. The attached patch should conform to the writeup (it applies the same change to "copy.orig" to produce "copy.hpp") --- C:\dev\boost_1_33_1\boost\iostreams\copy.orig Thu May 26 00:37:30 2005 +++ C:\dev\boost_1_33_1\boost\iostreams\copy.hpp Mon Dec 11 17:14:34 2006 @@ -92,9 +92,10 @@ 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) + if (amt != -1) { + std::copy(buf.data(), buf.data() + amt, p.first + total); total += amt; + } } return total; }
participants (5)
-
eg -
Mathieu Peyréga