-1

I am implementing a queue using a generic linked list in C where each node in list is a simple struct which contains a void pointer and a node pointer to the next node. In the dequeue operation I want to remove the head (which is working fine), but I also want to return that node after it is removed.

EDITED TO CLARIFY

What i did in the dequeue function is (example):

//This is my queue struct
typedef struct Queue{
    LinkedList* list;
    size_t item_size;
} Queue;

//this is the dequeue function
Node* dequeue(Queue* queue){
    Node* head = queue->list->head;
    Node* returnedValue = (Node*)malloc(sizeof(Node));
    memcpy(returnedValue, head, sizeof(Node));

    removeBegin(queue->list);
    return returnedValue;  
}

//this is the remove head function
void removeBegin(LinkedList* list){
    Node* tempHead = list->head;

    list->head = list->head->next;
    tempHead->next = NULL;
    free(tempHead->value);
    tempHead->value = NULL;
    free(tempHead);
    tempHead = NULL;
}

the problem is everything before the free function is ok. Everything is being copied correctly. But immediately after the free function call the value that is copied to the newly allocated node becomes garbage (or 0).

The way I call the function is simply initialize the queue using this function:

Queue* init_queue(size_t size){
    Queue* queue = (Queue*)malloc(sizeof(Queue));
    // int x = 10;
    queue->list = createList(NULL, size);

    return queue;
}

then call dequeue and pass it the pointer of the queue.

How can I solve this? thanks a lot.

K.A.Q
  • 469
  • 4
  • 13
  • Please show how you call `copyValue`, the problem is most likely _there_. Read this: [mcve]. – Jabberwocky Jul 27 '18 at 13:31
  • 1
    Oh, now (I think) I understood. Of course the copied value becomes invalid, because you free it. Still show a [MCVE]. – Jabberwocky Jul 27 '18 at 13:34
  • the function name is a bit of a misnomer - it implements a copy-and-free (or a move) rather than a copy – Sander De Dycker Jul 27 '18 at 13:54
  • 1
    It isn't clear why you would even want to do this. Rather than copying the node, surely all you need is the value it contained? – Chris Turner Jul 27 '18 at 13:55
  • You're using n1->value as destination pointer in that `memcpy()` call without having set it to anything. Try using `calloc()` instead and you're more likely to get a segment violation to tell you where the real problem is. – Mike Housky Jul 27 '18 at 14:04
  • Wouldn't it be a lot easier to just return the already-allocated note instead of allocating a new buffer, copying it, and deallocating the original? – Peter Cordes Aug 07 '18 at 05:30

1 Answers1

2

The memory allocating using

 Node* n1 = (Node*)malloc(sizeof(Node));

is uninitialised. This means that accessing the value of n1->value gives undefined behaviour. A consequence is that

memcpy(n1->value, n->value, sizeof(n->value));

also gives undefined behaviour.

When behaviour is undefined, the consequences of executing any further code could be anything. Your observation

newly allocated node becomes garbage (or 0)

is one possible outcome.

There are more problems as well. However, you haven't provided enough information (e.g. how is the function called? how is the pointer passed as n initialised? how is n->value initialised?) so it is not possible to give advice on how to FIX your function.

Peter
  • 35,646
  • 4
  • 32
  • 74
  • I apologize for the lack of clarity. I have edited the question so that its clear. I am relatively new to C. In most of the languages I know (java for example) when you dequeue, you remove the value and return it and this is what i am trying to do. – K.A.Q Jul 27 '18 at 14:41