Modify

Ticket #3723 (closed Bugs: fixed)

Opened 4 years ago

Last modified 4 years ago

gzip_docompressor error - basic_array_source no putback member

Reported by: jose Owned by: danieljames
Milestone: Boost 1.43.0 Component: iostreams
Version: Boost 1.41.0 Severity: Showstopper
Keywords: Cc:

Description

std::string decompress_gzip(const std::string& in) {

std::string res; bio::array_source src(in.data(),in.length()); bio::copy(bio::compose(bio::gzip_decompressor(), src),

bio::back_inserter(res));

return res;

}

I get this compile error with 1.41 (not with earlier releases):

home/code/boost_1_41_0/boost/iostreams/read.hpp:191: error: 'class boost::iostreams::detail::direct_adapter<boost::iostreams::basic_array_source<char>

' has no member named 'putback'

Attachments

gzip.patch Download (2.0 KB) - added by danieljames 4 years ago.

Change History

comment:1 Changed 4 years ago by nikiml

Would you accept this patch? Or is it not general enough?

$ svn diff -r HEAD Index: detail/adapter/direct_adapter.hpp =================================================================== --- detail/adapter/direct_adapter.hpp (revision 59992) +++ detail/adapter/direct_adapter.hpp (working copy) @@ -116,6 +116,7 @@

std::streamsize write(const char_type* s, std::streamsize n); std::streampos seek( stream_offset, BOOST_IOS::seekdir,

BOOST_IOS::openmode = BOOST_IOS::in | BOOST_IOS::out );

+ bool putback(const char_type c);

void close(); void close(BOOST_IOS::openmode which);

#ifndef BOOST_IOSTREAMS_NO_LOCALE

@@ -204,6 +205,18 @@

}

template<typename Direct>

+inline bool direct_adapter<Direct>::putback + (const char_type c) +{ + pointers& get = ptrs_.first(); + if( get.ptr == get.beg ) + throw bad_putback(); + get.ptr -= 1; + *get.ptr = c; + return true; +} + +template<typename Direct>

inline std::streamsize direct_adapter<Direct>::write

(const char_type* s, std::streamsize n)

{

comment:2 Changed 4 years ago by nikiml

Sorry for the mess. Reposting the patch.

$ svn diff -r HEAD
Index: detail/adapter/direct_adapter.hpp
===================================================================
--- detail/adapter/direct_adapter.hpp   (revision 59992)
+++ detail/adapter/direct_adapter.hpp   (working copy)
@@ -116,6 +116,7 @@
     std::streamsize write(const char_type* s, std::streamsize n);
     std::streampos seek( stream_offset, BOOST_IOS::seekdir,
                          BOOST_IOS::openmode = BOOST_IOS::in | BOOST_IOS::out );
+    bool putback(const char_type c);
     void close();
     void close(BOOST_IOS::openmode which);
 #ifndef BOOST_IOSTREAMS_NO_LOCALE
@@ -204,6 +205,18 @@
 }

 template<typename Direct>
+inline bool direct_adapter<Direct>::putback
+    (const char_type c)
+{
+    pointers& get = ptrs_.first();
+    if( get.ptr == get.beg )
+        throw bad_putback();
+    get.ptr -= 1;
+    *get.ptr = c;
+    return true;
+}
+
+template<typename Direct>
 inline std::streamsize direct_adapter<Direct>::write
     (const char_type* s, std::streamsize n)
 {

comment:3 Changed 4 years ago by danieljames

This looks wrong to me, I don't think you should be modifying the source array. IMO the correct solution is to remove the call to putback in basic_gzip_decompressor::peekable_source::putback and change it so that it either throws an error or puts the character at the start of the buffer. The attached patch does the former, looking at the gzip implementation it only ever calls putback with a string or immediately after reading a character so it should never throw. Does it look okay to you?

Changed 4 years ago by danieljames

comment:4 Changed 4 years ago by anonymous

I see, I never even tried to understand the gzip code, just assumed that it *really* needs to putback in the source.

Does it look ok to you?

Of course. As long as it fixes my issue.

But I don't see why not have the array source peekable anyway(why is it wrong)? Aren't there other filters that require peekability?

Not trying to be pushy:) ...

comment:5 follow-up: ↓ 8 Changed 4 years ago by danieljames

  • Milestone changed from Boost 1.42.0 to Boost 1.43.0

The problem is that the 'peekable' interface doesn't just peek at a character, it lets you put back a different character, modifying the source array, which is read only according to the documentation (it probably should take a const reference, but that's another matter). Did you test my patch with your code?

comment:6 Changed 4 years ago by danieljames

comment:7 Changed 4 years ago by danieljames

(In [60415]) Gzip filter shouldn't require its source to be peekable. Refs #3723.

In a recent version, the gzip filter stopped working for array sources, this is because it started to require them to be peekable, which they aren't and can't be because the peek interface modifies the source, which for an array source is immutable.

Looking at the implementation, gzip decompressor has an internal class to emulate a peekable source, which calls the putback member on the original source if it runs out of space (requiring the source to be peekable). It shouldn't really need to do that so I changed it to throw an exception instead.

If it does need to do that, we could change it to store the character that was put back at the beginning of the string instead.

comment:8 in reply to: ↑ 5 Changed 4 years ago by nikiml

Replying to danieljames:

Did you test my patch with your code?

Yes. It works for me. Thank you!

<teasing>

modifying the source array, which is read only

what about basic_array?

</teasing>

comment:9 Changed 4 years ago by danieljames

  • Owner changed from turkanis to danieljames
  • Status changed from new to assigned

comment:10 Changed 4 years ago by danieljames

  • Status changed from assigned to closed
  • Resolution set to fixed

(In [60666]) Merge iostreams.

  • Fix write_device_impl<ostream_tag>. Fixes #3839
  • Fix error checks after calling SetFilePointer?. Fixes #3953
  • Gzip filter shouldn't require its source to be peekable. Fixes #3723.
  • In position_to_offset, only cast to stream_offset after calculating _Myoff. Fixes #3969.
  • ptrdiff_t is in std. Fixes #2505.
View

Add a comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
The resolution will be deleted. Next status will be 'reopened'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.