I'm using a simple queue implementation and I'm trying to make a simple program that has two threads: first one queue events on the queue and the second one processes them. I'm using mutexes to avoid sync problems. However, I'm still getting quite often (not always) segmentation faults.

Mind you, I built my codes using the -D_REENTRANT option as well. Also, the code works perfectly fine if queue/dequeue is called from one thread only.

Here's the simple code that shows how I am synchronizing the stuff:

int main(void) {
    init_queue(&my_queue);

    pthread_t thread_queue, thread_process;
    int iret1, iret2;

    iret1 = pthread_create( &thread_queue, NULL, svetlin_queue_events, (void*) NULL);
    iret2 = pthread_create( &thread_process, NULL, svetling_process_events, (void*) fb);

    pthread_join(thread_queue, NULL);
    pthread_join(thread_process, NULL);
}

t开发者_开发技巧he queuing function:

void svetlin_queue_events(void * v) {
    int fd;
    if ((fd = open("/dev/input/mouse1", O_RDONLY)) == -1) {
        printf("error with touchscreen device\n");
        exit(1);
    }

    struct input_event ev;
    struct input_event *being_sent;
    int rd;

    while (1) {
        read(fd, &ev, sizeof(struct input_event));

        being_sent = malloc(sizeof(struct input_event));
        memcpy(being_sent, &ev, sizeof(struct input_event));

        pthread_mutex_lock(&my_mutex);
        enqueue(&my_queue, being_sent);
        pthread_mutex_unlock(&my_mutex);

        printf("enqueueing...\n");
    }
}

and the processing function:

void svetling_process_events(void *v) {

    printf("FB pointer is: %p\n", fb);

    int x, y = 0;
    int has_item = 0;
    struct input_event **being_fetched;
    struct input_event *ev;

    while(1) {

        pthread_mutex_lock(&my_mutex);
        has_item = dequeue(&my_queue, being_fetched);
        pthread_mutex_unlock(&my_mutex);

        if (has_item) {
            ev = *being_fetched;
            printf("dequeueing...\n");
            if (ev->type == 3) {
                if (ev->code == 0) {
                    x = ev->value;
                }

                if (ev->code == 1) {
                    y = ev->value;
                }
            }
        }
    }
}

I hope I'm not overwhelming you with so much code, but it's something simple really.

So my question is what regarding synchronization am I doing wrong?

Thanks!

UPDATE

Here's the queue implementation:

struct queue_node
{
    struct queue_node *next;
    void *data;
};

struct queue
{
    struct queue_node *first;
    struct queue_node *last;
};

int enqueue(struct queue *q, void * const value)
{
    struct queue_node *node = malloc(sizeof(struct queue_node));

    if (node == NULL) {
        errno = ENOMEM;
        return 1;
    }

    node->data = value;

    if (q->first == NULL) {
        q->first = q->last = node;
    } else {
        q->last->next = node;
        q->last = node;
    }

    node->next = NULL;
    return 0;
}

int dequeue(struct queue *q, void **value)
{
    if (!q->first) {
        value = NULL;
        return 0;
    }
    *value = q->first->data;
    q->first = q->first->next;
    return 1;
}

void init_queue(struct queue *q)
{
    q->first = q->last = NULL;
}

int queue_empty_p(const struct queue *q)
{
    return q->first == NULL;
}

It segfaults on the line *value = q->first->data; in dequeue.

Thanks!

One minor problem is:

if (!q->first) {
    *value = NULL;
    return 0;
}

(note the added asterisk).

The main problem seems to be that you don't allocate memory for *being_fetched in the process_events function.

To be honest, I would change the type of that variable to struct input_event *being_fetched and simply pass its address to dequeue: has_item = dequeue(&my_queue, &being_fetched). This would also enable you to get rid of ev as a separate variable.

As aix said, you didn't init *being_fetched. But I think you're better off defining it differently:

struct input_event *being_fetched;
...
has_item = dequeue(&my_queue, &being_fetched);
...
ev = being_fetched;

And then make absolute sure you fix the other bug that aix mentions, otherwise you won't get the null pointer back out of the function.