+1  A: 

Here is an example for SyS. I hope it will help.

The way you are using malloc seems to be correct but you should be very careful when making memory allocations for IPC. You should check how the other process is managing memory (byte alignment, size, platform...)

In your code, What is the purpose of the mtype? Does the size you receive takes this mtype into account? Or is it only the size of mdata?

Update: Is mtype part of the message?

If so:

msgsize = size * sizeof(char) + sizeof(long)

pmsg = malloc(msgsize);

msgrcv(MSGQ_ID, pmsg, msgsize, MSQ_ID, 0);

if not

msg.data = (char *)malloc(size * sizeof(char));

msgrcv(MSGQ_ID, msg.data, size, MSQ_ID, 0);

The mtype is alocated on the stack while the data is allocated on the heap. If the msgreceive makes a kind of memcpy on the given pointer, it will cause some troubles.

luc
+2  A: 

You can't pass a pointer to a structure that contains a std::string member to msgrcv, this violates the interface contract.

The second parameter passed to msgrcv needs to point to a buffer with sufficient space to store a 'plain' C struct of the form struct { long mtype; char mdata[size]; }; where size is the third parameter to msgrcv.

Unfortunately, determining the size of this buffer might depend on size due to possible alignment issues but you have to assume that it doesn't on a system that provides this sort of interface. You can use the standard offsetof macro to help determine this size.

As a vector stores its components contiguously, once you know the size of the buffer, you can resize a vector of char and use this to hold the buffer. Using a vector relieves you of the obligation to free or delete[] a buffer manually.

You need to do something like this.

std::string RecvMessage()
{
    extern size_t size; // maximum size, should be a parameter??
    extern int MSGQ_ID; // message queue id, should be a parameter??
    extern long MSG_ID; // message type, should be a parameter??

    // ugly struct hack required by msgrcv
    struct RawMessage {
        long mtype;
        char mdata[1];
    };

    size_t data_offset = offsetof(RawMessage, mdata);

    // Allocate a buffer of the correct size for message
    std::vector<char> msgbuf(size + data_offset);

    ssize_t bytes_read;

    // Read raw message
    if((bytes_read = msgrcv(MSGQ_ID, &msgbuf[0], size, MSG_ID, 0)) < 0)
    {
        throw MsgRecvFailedException();
    }

    // a string encapsulates the data and the size, why not just return one
    return std::string(msgbuf.begin() + data_offset, msgbuf.begin() + data_offset + bytes_read);
}

To go the other way, you just have to pack the data into a struct hack compatible data array as required by the msgsnd interface. As others have pointer out, it's not a good interface, but glossing over the implementation defined behaviour and alignment concerns, something like this should work.

e.g.

void SendMessage(const std::string& data)
{
    extern int MSGQ_ID; // message queue id, should be a parameter??
    extern long MSG_ID; // message type, should be a parameter??

    // ugly struct hack required by msgsnd
    struct RawMessage {
        long mtype;
        char mdata[1];
    };

    size_t data_offset = offsetof(RawMessage, mdata);

    // Allocate a buffer of the required size for message
    std::vector<char> msgbuf(data.size() + data_offset);

    long mtype = MSG_ID;
    const char* mtypeptr = reinterpret_cast<char*>(&mtype);

    std::copy(mtypeptr, mtypeptr + sizeof mtype, &msgbuf[0]);
    std::copy(data.begin(), data.end(), &msgbuf[data_offset]);

    int result = msgsnd(MSGQ_ID, &msgbuf[0], msgbuf.size(), 0);
    if (result != 0)
    {
        throw MsgSendFailedException();
    }
}
Charles Bailey
This look great, exactly what I wanted to do. But it doesn't work either. My problem seems to be that sending data suffers from the same problems.Would you mind posting a similar method for sending a std::string? I tried doing it myself, but this didn't work.
Koraktor
Thanks again for all your help.I already found a working implementation for the send method.To my surprise your receive method didn't work as expected. Although the data is send correctly, I get corrupted data inside the vector<char>.When using my malloc implementation (or calloc or new char[]) from above (striked out) I get the correct data in msg.mdata and I can put it into a std::string. But as soon as I try to return the string I get a pretty cryptic SIGSEGV with no obvious source.Too sad the project is nearing it's deadline - seems like I'll never find the right way.
Koraktor
Why not post your send method, then we have a chance of helping. What do you mean by corrupted data? the vector of char is just a buffer so it's not going to contain only the contents of the sent message. the extracted string *should* be correct but without being able to see the data it's impossible to tell. What does the stack trace look like when the SIGSEGV is raised?
Charles Bailey
Is it safe to pass the address of msgbuf[0] to msgrcv()? Is there any guarantee that the implementation of std::vector<char> has the data in a contiguous array starting at msgbuff[0] ?
Dave Rigby
Charles Bailey
@Charles: Thanks for the clarification - I wasn't aware of that!
Dave Rigby
+1  A: 

You seem to be freely mixing C and C++, so I'll do the same. Note that you probably ought to write the function entirely in C, put it in its own translation unit and then call it from C++, but this should solve your immediate problem. It seems that the root of your confusion is arguably poor design of the API of msgrcv. You cannot have the data member of the msg struct be a pointer or a reference or anything other than a raw chunk of memory--msgrcv is going to put the data directly into the structure.

#include <stddef.h>
#include <stdlib.h>
#include <string.h>


/* Caller must ensure that data points to at least length chars */
bool read_msg( char *data, int length )
{
    bool status = false;
    struct msg {
        long mtype;
        char data[ 1 ];
    } *m = (struct msg *)malloc( length + offsetof( struct msg, data ));

    if( m != NULL ) {
        if( msgrcv( MSGQ_ID, m, length, MSQ_ID, 0 ) == length ) {
            memcpy( data, m->data, length );
            status = true;
        }
    }

    free( m );

    return status;
}



William Pursell
+1  A: 

Root cause The cause of your segementation error is that memcpy tries to copy a message greater then the size of your struct. Your struct has only room for a long (4 bytes) and a std:string. A string can be a variable size, but only when used as string (and it will automatically allocate memory for this and maintain a pointer). The std:string variable will look like

struct {
  unsigned int stringlength;
  char *pString;
}

Your code copies into msg, which in this (simplified) example is only 8 bytes long. And you overwrite the pointer with data, which is later interpreted as a memory location.

Solution A solution is to allocate a memory block large enough to hold both the header (mtype) and the message of size bytes. Since it is only temporary space, I'd suggest to use a stack variable for this. So

 struct msg {
        long mtype;
        char data[ size ];
    }

I suggest to replace size with something more unique and meaningfull if it is a global constant. Please note that size is expressed in bytes; if you need to send a std::string of size characters you'll need to increase the array length with at least sizeof(std::string)+1 (for the structure overhead and the \0 character); better a bit more.

Adriaan
+1  A: 

The reason for segmentation faults or corrupted data is that you're using an improper message structure for msgrcv() and msgsnd() functions. These functions require the following structure to be used:

struct msgbuf {
    long mtype;     /* message type, must be > 0 */
    char mtext[1];  /* message data */
};

But your Message structure contains an std::string which allocates memory on the heap, but the msgbuf structure expects a memory region starting at mtext[0]. The mtext member is not a pointer, but a char array inside the structure. As the size is unknown, the array is declared as a 1 sized array, but actually the caller shall provide a larger buffer. This technique is also used in some Windows APIs.

For being able to receive messages, you need a buffer

static const unsigned int BufferSize = 256;
char* buffer = new char[ sizeof(long) + BufferSize ];
msgbuf* msg = reinterpret_cast< msgbuf* >( buffer );

Then this msg can be passed to msgrcv(). And don't forget to delete[] the buffer.

Also be aware that the text received in the buffer is not null terminated, unless the 0 character is explicitly written when sending the message. Such non-null terminated text can be used either constructing an std::string from it - e.g. std::string( msg->mtext, len ) or reserving in the buffer one byte for the 0 and write it after receiving the message at the length returned by msgrcv()

Instead of plain char arrays, you could use std::vector which guarantees that the items will be stored in a continuous memory range. The following code can be used in a message receiving loop, which automatically increases the buffer size if the message does not fit into.

static const unsigned int InitialBufferSize = 32;
std::vector<char> buffer( InitialBufferSize );
msgbuf* msg = new (&buffer[0]) msgbuf();
// reserve space in the vector for the mtype member of msgbuf structure
std::vector<char>::size_type available = buffer.size() - sizeof(long);

for(;;)
{
    const ssize_t count = ::msgrcv( MSGQ_ID, msg, available, 0, 0 );
    if( -1 == count )
    {
     if( E2BIG == errno )
     {
      buffer.resize( buffer.size() * 2 );
      msg = new (&buffer[0]) msgbuf();
      available = buffer.size() - sizeof(long);
      continue;
     }
     perror( "Failed to read message from queue" );
     break;
    }

    // handle the received message
    ...
}
CsTamas