Values aren't being saved in a queue in C

Question

I'm trying to implement a queue in C using a linked list. I ran my code through a C visualizer and it seems that the values A-E aren't being saved. As in, one node will contain the letter A then when queue_enqueue is called again, another node is created that holds the letter B and then the previous node that contained A just, disappears... My code contains other functions like dequeue and one to check if the queue is empty, but I took them out to keep my code short and they are independent of the functions provided here.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <stdbool.h>

struct node {
    struct node *next;
    struct node *prev;
    char *value;
};

// The type for a list.
typedef struct list {
    struct node head;
} List;

typedef struct queue {
    List *list;
} Queue;


Queue *queue_create(void) {
    Queue *q = (Queue*)malloc(sizeof(struct queue));
    List *ptr = (List*)malloc(sizeof(struct list));
    q->list = ptr;
    q->list->head.next = NULL;
 
    return q;
}

void queue_destroy(Queue *q) {
    free(q->list->head.value);
    free(q);
}

void queue_enqueue(Queue *q, const char *value) {
    struct node *temp;
    temp = malloc(sizeof(struct node));
    temp->value = strdup(value);
    temp->next = NULL;

    q->list->head.next = temp;
    q->list->head.value = temp;
}

int main(void) {
    // Create an empty queue.
    Queue *q = queue_create();

    // Add the values A, B, ..., Z to the queue.
    char str[] = "A";
    for (char ch = 'A'; ch <= 'E'; ch++) {
        str[0] = ch;
        queue_enqueue(q, str);
    }

    // Clean up.
    queue_destroy(q);

    return 0;
}

Answer 1

To simplify your problem you should begin with a single-linked list and in your example there is no need to use char* as node value:

struct node {
    struct node* next;
    char value;
};

You also might want to add a element counter for your list:

typedef struct list {
    struct node *head;
    size_t num;
}

and a function to create a new node with the given value:

struct node *node_create(char value) {
    struct node *nd = malloc(sizeof(struct node));

    if (nd)
        nd->value = value;

    return nd;
}

The magic happens in the insert function but it is no rocket science at all. You either create your new node where head is pointing (empty list) or at the end of the list.

void list_insert(List *list, char value) {
    if (!list)
        return;

    if (!list->head) {
        // first element of list
        list->head = node_create(value);
        if (list->head)
            list->num++;
    }
    else {
        // move to the end of the list
        struct node *nd = list->head;
        
        while (nd->next) {
            nd = nd->next;
        }
        
        nd->next = node_create(value);
        if (nd->next)
             list->num++;
    }
}

Also make sure to properly initialize your list with some list_create function and when cleaning the list take care to free all list elements before releasing the memory of the list itself

Answer 2

You are not linking the chain pointers correctly.

And, setting q->list->head.value = temp; won't even compile.

For a doubly linked list, reusing a node struct as the front/back pointers (e.g prev/next) is doable but unclear. Better to redefine List slightly to use front/back--it's clearer.

Your destroy code is also wrong.

When appending to a list, the first time is slightly different than subsequent ones.

---

Here's the refactored code.

Since your code didn't change any of the list's next/prev pointers or temp's next/prev pointers, it wasn't clear whether you wanted to enqueue to the front or the back of the list, so I added both functions.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <stdbool.h>

struct node {
    struct node *next;
    struct node *prev;
    char *value;
};

// The type for a list.
typedef struct list {
    struct node *front;
    struct node *back;
    size_t count;
} List;

typedef struct queue {
    List *list;
} Queue;

Queue *
queue_create(void)
{
    Queue *q = malloc(sizeof(*q));
    q->list = calloc(1,sizeof(List));

    return q;
}

void
queue_destroy(Queue *q)
{
    List *list = q->list;
    struct node *cur;
    struct node *next;

    if (list != NULL)
        cur = list->front;
    else
        cur = NULL;

    for (;  cur != NULL;  cur = next) {
        next = cur->next;
        free(cur->value);
    }

    free(list);
    free(q);
}

struct node *
node_create(const char *value)
{
    struct node *temp = calloc(1,sizeof(*temp));

    temp->value = strdup(value);

    return temp;
}

void
queue_enqueue_front(Queue *q,const char *value)
{
    struct node *temp = node_create(value);
    List *list = q->list;

    temp->next = list->front;
    if (list->front != NULL)
        list->front->prev = temp;
    list->front = temp;

    if (list->back == NULL)
        list->back = temp;

    list->count += 1;
}

void
queue_enqueue_back(Queue *q,const char *value)
{
    struct node *temp = node_create(value);
    List *list = q->list;

    temp->prev = list->back;
    if (list->back != NULL)
        list->back->next = temp;
    list->back = temp;

    if (list->front == NULL)
        list->front = temp;

    list->count += 1;
}

void
queue_print_fwd(Queue *q,const char *who)
{
    List *list = q->list;
    struct node *cur;

    if (who != NULL)
        printf("%s:\n",who);

    for (cur = list->front;  cur != NULL;  cur = cur->next)
        printf("  %s\n",cur->value);
}

void
queue_print_rev(Queue *q,const char *who)
{
    List *list = q->list;
    struct node *cur;

    if (who != NULL)
        printf("%s:\n",who);

    for (cur = list->back;  cur != NULL;  cur = cur->prev)
        printf("  %s\n",cur->value);
}

int
main(void)
{
    // Create an empty queue.
    Queue *q = queue_create();

    // Add the values A, B, ..., Z to the queue.
    char str[] = "A";

    for (char ch = 'A'; ch <= 'E'; ch++) {
        str[0] = ch;
        queue_enqueue_back(q, str);
#ifdef DEBUG
        queue_print_fwd(q,"pushback");
#endif
    }

    for (char ch = 'K'; ch >= 'F'; ch--) {
        str[0] = ch;
        queue_enqueue_front(q, str);
#ifdef DEBUG
        queue_print_fwd(q,"pushfront");
#endif
    }

    queue_print_fwd(q,"Forward");
    queue_print_rev(q,"Reverse");

    // Clean up.
    queue_destroy(q);

    return 0;
}

---

Here's the program output:

Forward:
  F
  G
  H
  I
  J
  K
  A
  B
  C
  D
  E
Reverse:
  E
  D
  C
  B
  A
  K
  J
  I
  H
  G
  F

---

UPDATE:

Here's a slightly cleaned up version.

There's probably no need to allocate q->list--It could just be declared as List list; instead of List *list;

Just for grins, I also added a node removal function (e.g. queue_unlink).

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <stdbool.h>

typedef struct node Node;
struct node {
    Node *next;
    Node *prev;
    char *value;
};

// The type for a list.
typedef struct list {
    Node *front;
    Node *back;
    size_t count;
} List;

typedef struct queue {
    List list;
} Queue;

Queue *
queue_create(void)
{
    Queue *q = calloc(1,sizeof(*q));

    return q;
}

void
queue_destroy(Queue *q)
{
    List *list = &q->list;
    Node *cur;
    Node *next;

    for (cur = list->front;  cur != NULL;  cur = next) {
        next = cur->next;
        free(cur->value);
        list->count -= 1;
    }

    free(q);
}

void
queue_unlink(Queue *q,Node *cur)
{
    List *list = &q->list;

    do {
        if (cur == NULL)
            break;

        Node *prev = cur->prev;
        Node *next = cur->next;

        if (prev != NULL)
            prev->next = next;

        if (next != NULL)
            next->prev = prev;

        if (list->front == cur)
            list->front = next;

        if (list->back == cur)
            list->back = prev;

        cur->prev = NULL;
        cur->next = NULL;

        list->count -= 1;
    } while (0);
}

Node *
node_create(const char *value)
{
    Node *temp = calloc(1,sizeof(*temp));

    temp->value = strdup(value);

    return temp;
}

void
queue_enqueue_front(Queue *q,const char *value)
{
    Node *temp = node_create(value);
    List *list = &q->list;

    Node *front = list->front;
    temp->next = front;
    if (front != NULL)
        front->prev = temp;
    list->front = temp;

    if (list->back == NULL)
        list->back = temp;

    list->count += 1;
}

void
queue_enqueue_back(Queue *q,const char *value)
{
    Node *temp = node_create(value);
    List *list = &q->list;

    Node *back = list->back;
    temp->prev = back;
    if (back != NULL)
        back->next = temp;
    list->back = temp;

    if (list->front == NULL)
        list->front = temp;

    list->count += 1;
}

int
queue_print_node(Node *cur,int totlen)
{
    int curlen;

    curlen = strlen(cur->value);

    if ((totlen + curlen + 1) >= 78) {
        fputc('\n',stdout);
        totlen = 0;
    }

    fputc(' ',stdout);
    totlen += 1;

    fputs(cur->value,stdout);
    totlen += curlen;

    return totlen;
}

void
queue_print_fwd(Queue *q,const char *who)
{
    List *list = &q->list;
    Node *cur;
    int totlen = 0;

    if (who != NULL)
        printf("%s:\n",who);

    for (cur = list->front;  cur != NULL;  cur = cur->next)
        totlen = queue_print_node(cur,totlen);

    if (totlen > 0)
        fputc('\n',stdout);
}

void
queue_print_rev(Queue *q,const char *who)
{
    List *list = &q->list;
    Node *cur;
    int totlen = 0;

    if (who != NULL)
        printf("%s:\n",who);

    for (cur = list->back;  cur != NULL;  cur = cur->prev)
        totlen = queue_print_node(cur,totlen);

    if (totlen > 0)
        fputc('\n',stdout);
}

int
main(void)
{
    // Create an empty queue.
    Queue *q = queue_create();

    // Add the values A, B, ..., Z to the queue.
    char str[] = "A";

    for (char ch = 'A'; ch <= 'E'; ch++) {
        str[0] = ch;
        queue_enqueue_back(q, str);
#ifdef DEBUG
        queue_print_fwd(q,"pushback");
#endif
    }

    for (char ch = 'K'; ch >= 'F'; ch--) {
        str[0] = ch;
        queue_enqueue_front(q, str);
#ifdef DEBUG
        queue_print_fwd(q,"pushfront");
#endif
    }

    for (int iter = 1;  iter <= 10;  ++iter) {
        char buf[35];
        int len = (rand() % (sizeof(buf) - 1)) + 1;

        int idx = 0;
        for (;  idx < len;  ++idx) {
            int chr = (rand() % 26) + 'a';
            buf[idx] = chr;
        }
        buf[idx] = 0;

        queue_enqueue_back(q, buf);
    }

    queue_print_fwd(q,"Forward");
    queue_print_rev(q,"Reverse");

    // Clean up.
    queue_destroy(q);

    return 0;
}