0

This is my delete function

void deleteNode(Node** head, Node* deletingNode) {
    if (head == NULL || deletingNode == NULL)
        return;
    if (*head == deletingNode) {
        *head = deletingNode->next;
        displayNode(*head); //This is to check if the head has changed. And it has.
    }
    if (deletingNode->next != NULL)
        deletingNode->next->prev = deletingNode->prev;
    if (deletingNode->prev != NULL)
        deletingNode->prev->next = deletingNode->next;
    delete(deletingNode);
    return;
}

After I delete the head node and try do anything with the linked list, for example, display the whole linked list like so

void displayNode(Node* node) {
    cout << left << setw(4) << node->employee.empID
        << left << setw(20) << node->employee.empName
        << left << setw(8) << node->employee.empSalary
        << left << setw(25) << node->employee.empAddress
        << left << setw(15) << node->employee.empPhone
        << left << setw(2) << node->employee.depNo
        << left << setw(28) << node->employee.depName
        << left << setw(4) << node->employee.performance
        << '\n';
}
void displayAllNodes(Node* node) {
    displayColumnNames();
    while (node != NULL) {
        displayNode(node);
        node = node->next;
    }
}

An exception is thrown at the second line of the above block

Exception thrown at 0x78DBE26E (ucrtbased.dll) in Project.exe: 0xC0000005: Access violation reading location 0xDDDDDDDD. occurred

This ONLY happens when I delete the head node. Works just fine if I delete any other node.

So far I have tried making the double pointer in the deleteNode function into a single pointer but that didn't make a difference.

The whole code: https://codeshare.io/5oV8Jr

mismaah
  • 344
  • 5
  • 21
  • 1
    Show a [mcve]. I don't see anything obviously wrong with `deleteNode`. The problem likely lies elsewhere, in the way you set up the list in the first place, and/or the way you call `deleteNode`. – Igor Tandetnik Jan 01 '20 at 15:17
  • @IgorTandetnik I'm not sure how I could do a mre without messing things up. However the program is quite small around 500 lines https://codeshare.io/5oV8Jr – mismaah Jan 01 '20 at 15:25
  • `0xDDDDDDDD` means you are dereferencing a deleted pointer that exists on the heap. Here is the explanation: `0xDDDDDDDD` is a magic code meaning Used by Microsoft's C++ debugging heap to mark freed heap memory see here: [https://stackoverflow.com/a/127404/487892](https://stackoverflow.com/a/127404/487892) – drescherjm Jan 01 '20 at 15:28
  • 1
    ***I'm not sure how I could do a mre without messing things up.*** You need to use your debugger to figure this one out. Thankfully you have one of the best debuggers. – drescherjm Jan 01 '20 at 15:30
  • @drescherjm So it means the program is trying to access the head pointer I just deleted right? But I don't know how that would happen if head was changed to the node after head. – mismaah Jan 01 '20 at 15:33
  • `deleteRecord(Node* node)` should be `deleteRecord(Node** node) {` the bug is caused by &node in deleteRecord(). This is related to what @IgorTandetnik mentioned. – drescherjm Jan 01 '20 at 15:37
  • 1
    `deleteRecord` takes `head` by value. Changes made to `head` inside the function are not visible to the caller: `head` in `mainMenu` is not updated, and becomes a dangling pointer. As I predicted, the problem lies in the code not shown; hence the need for a [mcve] – Igor Tandetnik Jan 01 '20 at 15:37
  • Thanks a lot, that fixed the problem. I'm new to C++ and am still having trouble with pointers. Still not sure what the double pointer does but the problem has been fixed for now. – mismaah Jan 01 '20 at 15:40
  • I believe the answer shows an additional bug. – drescherjm Jan 01 '20 at 15:40

1 Answers1

0

Update: Following the fix from comments section(thanks to drescherjm and Igor Tandetnik), here's another bug I found and a quick fix to that.

I believe there's an issue is in this part of code:

if (*head == deletingNode) {
  *head = deletingNode->next;
  displayNode(*head); //This is to check if the head has changed. And it has.
}

What if you have just 1 node in the linked list and you call deleteNode(). Now head and deletingNode points to the same node and *head = deletingNode->next; sets the head to NULL after which you're calling displayNode(*head); which will throw an error.

Quick fix:

if (*head == deletingNode) {
  *head = deletingNode->next;
  if(*head != NULL)
    displayNode(*head); //This is to check if the head has changed. And it has.
}
Ajay Dabas
  • 1,404
  • 1
  • 5
  • 15