cpointersstructipcsysv-ipc

Am I doing this right? Mapping pointer in struct to position outside of struct for IPC shared memory usage


Disclaimer: I am a C noob working on a project to conduct IPC using a shared memory segment. My plan is to define a struct that has a pointer (void *) to the remaining memory that is mapped (via smget()) which lives outside of the struct that I will be using as a header to pass information about the status of the request between processes (it will also have mutex/cond structs).

I just wanted to see if I was doing this correctly... my questions are in the comments of my main function.

Let me know if I am not being clear about something, again I'm pretty new to this.

#include <stdio.h>
#include <sys/shm.h>
#include <sys/stat.h>
#include <string.h>

#define clean_errno() (errno == 0 ? "None" : strerror(errno))
#define log_error(M, ...) fprintf(stderr, "[ERROR] (%s:%d: errno: %s) " M "\n", __FILE__, __LINE__, clean_errno(), ##__VA_ARGS__)
#define log_warn(M, ...) fprintf(stderr, "[WARN] (%s:%d: errno: %s) " M "\n", __FILE__, __LINE__, clean_errno(), ##__VA_ARGS__)
#define log_info(M, ...) fprintf(stderr, "[INFO] (%s:%d) " M "\n", __FILE__, __LINE__, ##__VA_ARGS__)

#define PATHNAME "/tmp"

typedef struct shm_data
{
    /* segment id assigned by shmget() */
    int segment_id;

    /* max size of char *data */
    unsigned int buffer_size;

    /* nbytes currently in char *data to be read */
    unsigned int nbytes_buffer;

    /* nbytes remaining to be sent to be read */
    unsigned int nbytes_remaining;

    /* nbytes total that need to be read */
    unsigned int nbytes_total;

    /* pointer to the memory poistion just outside of the struct */
    char *buffer;
} shm_data;

shm_data *create_shm(unsigned int segment_number, unsigned int segment_size)
{
    int segment_id;
    shm_data *shm;

    // just doing segment_size + 1 for this example so when I print data it has a '\0' for output
    segment_id = shmget(ftok(PATHNAME, segment_number), segment_size + 1, IPC_CREAT | S_IRUSR | S_IWUSR);
    void *shm_addr = shmat(segment_id, (void *) 0, 0);
    shm = (shm_data *) shm_addr;

    shm->segment_id = segment_id;
    shm->buffer_size = segment_size - sizeof(shm_data);
    shm->nbytes_buffer = 0;
    shm->nbytes_remaining = 0;
    shm->nbytes_total = 0;

    // 1 - am I initializing my pointer correctly? I want it to point to the first byte that comes after my struct
    shm->buffer = shm_addr + sizeof(shm_data) + 1;
    memset(&shm->buffer[0], '\0', shm->buffer_size);

    return shm;
}

int main(int argc, char *argv[])
{
    char *data = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890";
    unsigned int segment_size = 16;

    shm_data *shm = create_shm(1, sizeof(shm_data) + segment_size);
    shm->nbytes_total = strlen(data);
    shm->nbytes_remaining = shm->nbytes_total;

    int count = 0;
    while (shm->nbytes_remaining > 0)
    {
        // 2 - is this an appropriate way to "clear" the memory outside of the struct?
        memset(&shm->buffer[0], '\0', shm->buffer_size + 1);

        int nbytes = shm->nbytes_remaining;
        if (nbytes > shm->buffer_size)
        {
            // max we can buffer is this buffer_size
            nbytes = shm->buffer_size;
        }

        // 3 - is this an appropriate way to copy a segment of data with an offset into the memory outside of the struct?
        int offset = count * shm->buffer_size;
        memcpy(shm->buffer, &data[0] + offset, nbytes);
        log_info("[%d] %s", nbytes, shm->buffer);

        shm->nbytes_remaining = shm->nbytes_remaining - nbytes;

        count++;
    }

    return 0;
}

Solution

  • Yeah, that's not going to work. There's no guarantee that the address that one process maps shared memory to is the same as the address another process sees.

    The rules to remember are:

    Also, I doubt that there's any guarantee that the segment ID will be the same in both processes. Finally, there's no point in storing information about how to access a shared memory block within the shared memory block itself; it's circular reasoning.

    An example of a control block setup is:

    shm_data *create_shm(unsigned int segment_number, unsigned int segment_size)
    {
        shm_data *shm;
        shm = (shm_data*)malloc(sizeof(shm_data));
        if(shm == NULL) return shm;
    
        // The following should have more error-trapping code.
        shm->segment_id = shmget(ftok(PATHNAME, segment_number), 
            segment_size, IPC_CREAT | S_IRUSR | S_IWUSR);
        shm->buffer_size = segment_size;
        shm->data = shmat(shm->segment_id, (void *) 0, 0);
        memset(shm->data, 0, shm->buffer_size);
    
        return shm;
    }
    

    Also, don't hide extra "termination" bytes in this code. If the caller needs to put a string in there, it needs to make sure it asks for space for the terminator byte. This code should not assume that the shared memory block contains a string. It's typical to convert shm->data to a pointer to another kind of struct which just describes the internals of the shared memory (and would thus declare storage for a string as a char array). Then you can pass sizeof(struct) as the segment_size into this function. It might be a better idea to change data to a void* in this case.


    OK, I see you've edited the question to make the shared memory into a character buffer. I still think that it would be best if you separated the shared memory management code from the buffer code.

    I also recommend that you define your buffer variables in shared memory in terms of a read offset and a write offset. These offsets need to be in a data type (unsigned int?) that is accessed with atomic operations.

    There's no OS-level control over the order in which each process accesses the memory; it's perfectly normal for a process to get blocked in the middle of something, and another process would then see a "half-baked" state.

    So either use atomic types, and set your code up so that the change of a single atomic variable changes the state fully from one to another, or you'll need to use another IPC mechanism like mutexes or semaphores to control access to the shared memory.