search

achille-roussel / nanomsgxx

Nanomsg binding for C++11
MIT License
115 stars 35 forks source link

Resizing the output messages to the right size. #4

Closed steven-martins closed 8 years ago

steven-martins commented 8 years ago

Hello,

When a message is generated from a message_ostream, the size of the message is not changed (by default 1000). This leads to two issues:

The following example demonstrates the "problem":
example.zip Server's side: 

nnxx::message_ostream os;

os << "I'm a server ";
os << getpid();

socket_.send(os.msg());

Client's side: 

m = socket_.recv();
std::cout << m.size() << " " << m << std::endl;

The following correction resizes the message to the right size and returns it.

execution summary
  tests that pass 14/14
    /vagrant/SKNet/dep/nanomsgxx/build/tests/test_message
    /vagrant/SKNet/dep/nanomsgxx/build/tests/test_message_istream
    /vagrant/SKNet/dep/nanomsgxx/build/tests/test_message_ostream
    /vagrant/SKNet/dep/nanomsgxx/build/tests/test_nnxx_ext
    /vagrant/SKNet/dep/nanomsgxx/build/tests/test_pair
    /vagrant/SKNet/dep/nanomsgxx/build/tests/test_pipeline
    /vagrant/SKNet/dep/nanomsgxx/build/tests/test_poll
    /vagrant/SKNet/dep/nanomsgxx/build/tests/test_pubsub
    /vagrant/SKNet/dep/nanomsgxx/build/tests/test_readme
    /vagrant/SKNet/dep/nanomsgxx/build/tests/test_reqrep
    /vagrant/SKNet/dep/nanomsgxx/build/tests/test_reqrep_multi
    /vagrant/SKNet/dep/nanomsgxx/build/tests/test_socket
    /vagrant/SKNet/dep/nanomsgxx/build/tests/test_survey
    /vagrant/SKNet/dep/nanomsgxx/build/tests/test_timeout
  tests that fail 0/14
achille-roussel commented 8 years ago

Hello Martin, thanks for the pull request.

While I agree you definitely found a bug I think the way you address it has an issue. The intent of the nnxx::message_ostream::msg member function was to have the same semantics as std::ostringstream::str, it shouldn't modify the internal buffer and just return a copy of the current content (this is why I provided the move_msg function as well if you don't want to afford a copy and just extract the content).

How about changing this pull request to something like this?

template <typename Char, typename Traits>
message basic_message_streambuf<Char, Traits>::msg(int type) {
  using std::copy;

  message m(this->pptr() - this->pbase(), type);

  copy(reinterpret_cast<const char_type *>(this->pbase()),
       reinterpret_cast<const char_type *>(this->pptr()),
       reinterpret_cast<char_type *>(m.data()));

  return m;
}
steven-martins commented 8 years ago

Hello,

I’m totally agree with you … I was so focused on my use case that I didn’t follow the same semantics of ostringstream object.

As you suggested, I updated the pull request.

Thanks.

execution summary
  tests that pass 14/14
    /vagrant/SKNet/dep/nanomsgxx/build/tests/test_message
    /vagrant/SKNet/dep/nanomsgxx/build/tests/test_message_istream
    /vagrant/SKNet/dep/nanomsgxx/build/tests/test_message_ostream
    /vagrant/SKNet/dep/nanomsgxx/build/tests/test_nnxx_ext
    /vagrant/SKNet/dep/nanomsgxx/build/tests/test_pair
    /vagrant/SKNet/dep/nanomsgxx/build/tests/test_pipeline
    /vagrant/SKNet/dep/nanomsgxx/build/tests/test_poll
    /vagrant/SKNet/dep/nanomsgxx/build/tests/test_pubsub
    /vagrant/SKNet/dep/nanomsgxx/build/tests/test_readme
    /vagrant/SKNet/dep/nanomsgxx/build/tests/test_reqrep
    /vagrant/SKNet/dep/nanomsgxx/build/tests/test_reqrep_multi
    /vagrant/SKNet/dep/nanomsgxx/build/tests/test_socket
    /vagrant/SKNet/dep/nanomsgxx/build/tests/test_survey
    /vagrant/SKNet/dep/nanomsgxx/build/tests/test_timeout
  tests that fail 0/14
achille-roussel commented 8 years ago

Awesome, thanks for the fix!