C segmentation fault when I try to dequeue-CodePudding

I'm making server-client program.

Client's collector need to enqueue packet data and the sender dequeue them.

Whenever I try to dequeue packet, I get a segmentation error.

Here is entire queue implementation :

aqueue *init_aqueue(void)
{
    aqueue *q;

    q = malloc(sizeof(aqueue));
    q->head = NULL;
    q->tail = NULL;

    return (q);
}

aqueue_node  *init_node(packet *data, aqueue_node *tail)
{
    aqueue_node  *new;

    new = malloc(sizeof(aqueue_node));
    new->data = data;
    new->prev = tail;
    new->next = NULL;
}

packet   *peek(aqueue *q)
{
    packet  *res;

    if (!q->size)
        return (NULL);
    else
    {
        res = malloc(sizeof(packet));
        res->length = q->head->data->length;
        res->payload = strndup(q->head->data->payload, res->length);
    }
        return (res);
}

void    enqueue (aqueue *q, packet *data)
{
    aqueue_node *new;

    new = init_node(data, q->tail);
    if (q->head == NULL) //adding first node of aqueue
    {
        q->head = new;
        q->size = 1;
    }
    else
    {
        q->tail->next = new;
        q->size  = 1;
    }
    q->tail = new;
}

packet  *dequeue(aqueue *q)
{
    packet      *data;
    aqueue_node *temp;

    if (q->size == 0)
    {
        return (NULL);
    }
    else
    {
        if (q->head == q->tail)
        {
            //When aqueue contains only one node
            free(q->head->data);
            free(q->head);
            q->tail = NULL;
            q->head = NULL;
        }
        else
        {
            data = peek(q);
            temp = q->head;
            q->head = q->head->next;
            q->head->prev = NULL;
            free(temp->data);
            free(temp);
        }
        q->size--;
        return (data);
    }
}

Then I have this as a shared resource in the agent.c file :

aqueue * queue;

Collector code :

//send four packet
pthread_mutex_lock(&p->aqueue_lock);
        enqueue(q, get_mem_info());
        enqueue(q, get_net_info());
        enqueue(q, get_cpu_info());
        enqueue(q, get_proc_info());

Sender code :

   packet  *data;

    data = NULL;
    if (q->size > 0)
    {
        data = dequeue(q);
    }
    if (data)
    {
        if (0 > send(clientfd, data->payload, data->length,
        {
            perror("send error");
            exit (EXIT_FAILURE);
        }
    }

lldb result :

* thread #2, name = 'agent', stop reason = signal SIGSEGV: invalid address (fault address: 0x50e8)
    frame #0: 0x0000aaaaaaaa3c64 agent`dequeue(q=0x0000aaaaaaab62e0) at agent_queue.c:85:18
   82           {
   83               cur = q->head;
   84               q->head = q->head->next;
-> 85               q->head->prev = NULL;
   86               free(cur->data);
   87               free(cur);
   88           }

EDIT :

I was dumb. I fixed init_node to return value:

aqueue_node  *init_node(packet *data, aqueue_node *tail)
{
    aqueue_node  *new;

    new = malloc(sizeof(aqueue_node));
    new->data = data;
    new->prev = tail;
    new->next = NULL;
    return (new); //added
}

But I still got same error..

Return data of dequeue is different from freed data inside dequeue. I changed variable name to make it clear :

packet  *dequeue(aqueue *q)
{
    packet      *copied_data;
    aqueue_node *temp;

    printf("queue size : %d\n", q->size);
    if (q->size == 0)
    {
        return (NULL);
    }
    else
    {
        if (q->head == q->tail)
        {
            //When aqueue contains only one node
            free(q->head->data);
            free(q->head);
            q->tail = NULL;
            q->head = NULL;
        }
        else
        {
            copied_data = peek(q);
            temp = q->head;
            q->head = q->head->next;
            q->head->prev = NULL;
            free(temp->data);
            free(temp);
        }
        q->size--;
        return (copied_data);
    }
}

I found I didn't alloc prev properly in enqueue function. But still same error occurs

void    enqueue (aqueue *q, packet *data)
{
    aqueue_node *new;

    new = init_node(data, q->tail);
    if (q->head == NULL) //adding first node of aqueue
    {
        q->head = new;
        q->size = 1;
    }
    else
    {
        q->tail->next = new;
        new->prev = q->tail; //added
        q->size  = 1;
    }
    q->tail = new;
}

EDIT :

I freed entire queue after collect for debugging and missed to erase it. That was the problem. Sorry for confusing

CodePudding user response：

Enable and heed your compiler's warnings!!! (I use -Wall -Wextra -pedantic with gcc.) It would have found the problem.

init_node doesn't return a value. So you're not adding a node to the queue but garbage. Your compiler would have told you that.

You have a similar problem in dequeue where not all paths set data before reaching return data.

Additionally, it appears that when you do return a value from dequeue, it's a pointer to memory you've freed! That's no good.