zeromq / zmqpp

0mq 'highlevel' C++ bindings
http://zeromq.github.io/zmqpp
Mozilla Public License 2.0
439 stars 195 forks source link

incorrect work of zmqpp::frame with the underlying zmq::msg_t through raw_new_msg API #139

Open kikosha opened 8 years ago

kikosha commented 8 years ago

I'm using zmqpp v4.1.2 together with zmq v4.1.3 When using

zmq_msg_t& raw = zmqpp::message::raw_new_msg(size_t)

the

  zmq_msg_t._

char array actually points at the beginning of the underlying

zmq::msg_t struct

first 16 bytes of which are occupied by the internal struct data, because of the following code:

zmq.cpp line 571:

int zmq_msg_init_size (zmq_msg_t *msg_, size_t size_)
{
    return ((zmq::msg_t*) msg_)->init_size (size_);
}

When I try to read the written data, using

const unsigned char* partData = (const unsigned char*)msg.raw_data(size_t);

I receive a pointer to the actual beginning of the raw data, namely, offset of 16 bytes, relative to the char array beginning, which is correct. The method

zmqpp::message::add_raw(const char*, size_t);

also copies the provided array into the internal storage with the proper offset, so it seems only

zmqpp::message::raw_new_msg(size_t)

I think that it stopped working, when a new member pointer was added to the

from zmq/msg.hpp:

zmq::msg_t::u::vsm struct:
            struct {
                metadata_t *metadata;   <------- the new member pointer
                unsigned char data [max_vsm_size];
                unsigned char size;
                unsigned char type;
                unsigned char flags;
            } vsm;
xaqq commented 8 years ago

Hello, I'm not totally sure I understand you correctly.

zmqpp::message::raw_new_msg(size_t) returns a reference to a zmq_msg_t, so if you want to access it's data you should call zmq_msg_data() on the result of zmqpp::raw_new_msg().

kikosha commented 8 years ago
zmq_msg_t& message::raw_new_msg(size_t const reserve_data_size)
{
    _parts.push_back( frame(reserve_data_size) );

    return _parts.back().msg();
}
from zmq.h :
typedef struct zmq_msg_t {unsigned char _ [64];} zmq_msg_t;

It has no methods to call, its a pure char array

xaqq commented 8 years ago

Yes indeed. zmq_msg_t is an opaque type provided by libzmq. However you are not supposed to touch the char array yourself.

If you want to access the data associated with a zmq_msg_t, you have to call zmq_msg_data(). It has no method, because it's a C API. Something like this would work:

zmq_msg_t &raw_msg = zmqpp_msg.raw_new_msg();
char *pointer_to_msg_data = zmq_msg_data(&raw_msg);

Note that you must do so because it's entirely possible that the data is located somewhere else in memory. Actually, the data is only packed with the zmq_msg_t for small message (in order to avoid memory allocation).

kikosha commented 8 years ago

Well, now that the change wrecked havoc upon my code and I delved into the internals I realize what you just said, but there's no way of knowing that beforehand, by solely looking at the API. Please, don't get me wrong, zmqpp is a great piece of software and I really enjoy using it. Whats more - its opensource and free. What I do want to say, is that its worth covering this point API-wise. The thing that you proposed makes perfect sense, so why not including it in zmqpp API itself?

xaqq commented 8 years ago

Please, don't get me wrong, zmqpp is a great piece of software and I really enjoy using it. Whats more - its opensource and free.

Thanks. Don't worry, we welcome feedback.

What would you want to include in the API? zmqpp::message::raw_new_msg() gives you a new zmq_msg_t, but zmq_msg_t is not a type from zmqpp, it's a libzmq type.

You cannot go from a pointer to raw data to a pointer to zmq_msg_t, but the reverse is possible through zmq_msg_data(). So I believe we have to provide a reference to the zmq_msg_t and not to its data.

[...] I realize what you just said, but there's no way of knowing that beforehand, by solely looking at the API.

It's correct that zmqpp doesn't mention anything about zmq_msg_t, but that's because it's not a zmqpp type. Some warning in the documentation would maybe be of use. However, the zmq_msg_*() functions (http://api.zeromq.org/4-1:zmq-msg-data or http://api.zeromq.org/4-1:zmq-msg-init) all state in their documentation that: Never access zmq_msg_t members directly, instead always use the zmq_msg family of functions.

This means that direct manipulation zmq_msg_t is not allowed by the API and will cause trouble (as you have unfortunately experienced).

Do you think that giving more information in the documentation for zmqpp::message::raw_new_msg() would have been enough?