Storing void pointers in a struct

Question

I'm getting segmentation fault on code that is trying to initialize a struct of pointers to 0mq context and socket. The commented out code in the main method works, but it's only using local variables. I would like to initialize them and pass them around in a struct, but my google foo is failing me on how to do this properly.

#include "zhelpers.h"
#include <stdio.h>
#include <stdlib.h>
#include <zmq.h>

struct publisher{
    void *handle;
    void *context;
};

void init_publisher(struct publisher *p);
void destroy_publisher(struct publisher *p);
void publish(struct publisher *p,char *msg);

void init_publisher(struct publisher *p)
{
    p = (struct publisher *)malloc(sizeof(struct publisher));
    p->context = malloc(sizeof(void *));
    p->handle = malloc(sizeof(void *));
    void *context = zmq_ctx_new();
    void *handle = zmq_socket(context,ZMQ_PUB);
    zmq_bind(handle, "tcp://*:5556");
    zmq_bind(handle, "ipc://feed.ipc");
    p->context = context;
    p->handle = handle;
}

void destroy_publisher(struct publisher *p)
{
    zmq_close(p->handle);
    zmq_ctx_destroy(p->context);
    free(p->handle);
    free(p->context);
    free(p);
}

void publish(struct publisher *p,char *msg)
{
    s_send(p->handle, msg);
}

int main(void)
{
/**
    void *context = zmq_ctx_new();
    void *publisher = zmq_socket(context, ZMQ_PUB);
    int rc = zmq_bind(publisher, "tcp://*:5556");
    assert(rc == 0);
    rc = zmq_bind(publisher, "ipc://weather.ipc");
    assert(rc == 0);
    printf("Started Weather Server...\n");

    srandom((unsigned) time (NULL));
    int zipcode, temperature, relhumidity;
    zipcode = randof(100000);
    temperature = randof (215) - 80;
    relhumidity = randof (50) + 10;

    char update[20];
    sprintf(update, "%05d %d %d", zipcode, temperature, relhumidity);
    s_send(publisher, update);
    zmq_close(publisher);
    zmq_ctx_destroy(context);
*/

    struct publisher *p;
    init_publisher(p);
    printf("Setup pub\n");

    srandom((unsigned) time (NULL));
    int zipcode, temperature, relhumidity;
    zipcode = randof(100000);
    temperature = randof (215) - 80;
    relhumidity = randof (50) + 10;
    char update[20];
    sprintf(update, "%05d %d %d", zipcode, temperature, relhumidity);
    publish(p,update);
    printf("Published Message\n");

    destroy_publisher(p);
    printf("Destroyed publisher\n");
    return 0;
}

Answer 1

There appears to be nothing in this code that would make it crash. (Assuming you know how all your zmq_... stuff works.)

It would have helped if you told us precisely where the error occurs, but my guess would be that the error occurs outside of this code.

You see, you are passing struct publisher *p to your init_publisher() function, but then you are allocating memory for p inside that method, (which makes passing p pointless,) and then you are not returning p. As a result, the code that calls init_publisher() probably expects p to be initialized, but it is not. The memory pointed by p is just allocated and leaked locally within your init_publisher() function.

So, instead of passing p, just have the function declare it and return it.

Alternatively, if the caller has already allocated p, then do not allocate it all over again from within init_publisher().

---

^{Please also note that the statements p->context = malloc(sizeof(void *)); are unnecessary and they are leaking small amounts of memory, because you proceed to overwrite these struct members.}

Answer 2

The problem is that the passed pointer and the pointer you malloc()ed are not the same. The passed pointer contains the same address of your original pointer, presumably an invalid address, but the addresses of the poninters them selves are different because in c you can only pass a variable by value and hence, the pointer is copied.

That means that when you reassign p inside the function, the p from outside the function is unaltered. It would be different if it was allocated outside and you just use the function to access it's members.

You also don't need to malloc() every pointer you want to use, the thing is that it must point to a valid address before dereferencing it. When you want to request new uninitialized memory then you use malloc() otherwise you just make the pointer point to a valid address so that dereferencing it is defined, one example of using a pointer without malloc()ing it is

int *pointer;
int value;
value = 4;
pointer = &value; // Now `pointer' points to `value's` address
*pointer = 3;
printf("%d\n", value);

One way to write the function would be

int
init_publisher(struct publisher **pp)
{
    struct publisher *p;
    *pp = malloc(sizeof(struct publisher));
    if (*pp == NULL)
        return -1;
    p = *pp;
    p->context = zmq_ctx_new();
    p->handle = zmq_socket(context,ZMQ_PUB);
    if (p->handle != NULL) /* Just in case, do not dereference a NULL pointer */
    {
        zmq_bind(p->handle, "tcp://*:5556");
        zmq_bind(p->handle, "ipc://feed.ipc");
    }
    return 0;
}

and then you could use it like this

struct publisher *p;
if (init_publisher(&p) != 0)
    do_something_there_was_an_error();
/* Continue using `p' */

Note that the funcion is returning a value indicating whether allocations succeeded or not. Normally malloc() will not fail, but that doesn't mean that you should ignore the possible failure.

What I mean when I say if you allocate p first, is that if you instead do this

struct publisher *p;
p = malloc(sizeof(*p));
if (p == NULL)
    return handle_error();
init_publisher(p);

then init_publisher() could be

void
init_publisher(struct publisher *pp)
{
    void *context;
    void *handle;
    p->context = zmq_ctx_new();
    p->handle = zmq_socket(context,ZMQ_PUB);
    if (p->handle != NULL) /* Just in case, do not dereference a NULL pointer */
    {
        zmq_bind(p->handle, "tcp://*:5556");
        zmq_bind(p->handle, "ipc://feed.ipc");
    }
}

which is probably what you was trying to do.