I have written this function to delete the last node of a singly linked list.
The problem is, it is able to delete all the nodes except the first/starting node.
What is missing from this code snippet?
Please answer my particular problem.
#include <stdio.h>
#include <stdlib.h>
struct Node
{
char CharContent;
struct Node * NextNodePointer;
};
typedef struct Node Node;
#pragma region Prototypes
Node * CreateNewNode(char ch);
Node * AddNode(Node * start, Node * newNode);
void DeleteTailNode(Node * start);
void PrintAllNodes(const Node * start);
#pragma endregion Comments
main()
{
Node * start = NULL;
Node * newNode = NULL;
start = AddNode(start, CreateNewNode('A'));
start = AddNode(start, CreateNewNode('B'));
start = AddNode(start, CreateNewNode('C'));
start = AddNode(start, CreateNewNode('D'));
PrintAllNodes(start);
DeleteTailNode(start);
PrintAllNodes(start);
DeleteTailNode(start);
PrintAllNodes(start);
DeleteTailNode(start);
PrintAllNodes(start);
DeleteTailNode(start);
PrintAllNodes(start);
DeleteTailNode(start);
PrintAllNodes(start);
getch();
}
#pragma region Node * CreateNewNode(char ch)
Node * CreateNewNode(char ch)
{
struct Node * newNode = (struct Node *) malloc(sizeof(struct Node *));
newNode->CharContent = ch;
newNode->NextNodePointer = NULL;
return newNode;
}
#pragma endregion Comment
#pragma region UnifiedAddNode()
Node * AddNode(Node * start, Node * newNode)
{
Node * copyOfStart = start;
if(start == NULL)
{
return newNode;
}
else
{
while(copyOfStart->NextNodePointer != NULL)
{
copyOfStart = copyOfStart->NextNodePointer;
}
copyOfStart->NextNodePointer = newNode;
return s开发者_高级运维tart;
}
}
#pragma endregion Comment
void DeleteTailNode(Node * start)
{
Node * prev = NULL;
Node * current = start;
while(current->NextNodePointer != NULL)
{
prev = current;
current = current->NextNodePointer;
}
free (current);
if (prev != NULL)
{
prev->NextNodePointer = NULL;
}
}
#pragma region PrintAllNodes()
void PrintAllNodes(const Node * start)
{
struct Node * tempRoot = start;
while(tempRoot != NULL)
{
printf("%c, ", tempRoot->CharContent);
tempRoot = tempRoot->NextNodePointer;
}
printf("\n");
}
#pragma endregion Comment
Inside CreateNewNode()
struct Node * newNode = (struct Node *) malloc(sizeof(struct Node *));
^
|
Ouch!!
Change it to : struct Node * newNode = (struct Node *) malloc(sizeof(struct Node));
EDIT 2
Test Run HERE
You aren't detecting the case where start is NULL, i.e. the list is empty.
Don't you want to free the next node before setting it to NULL?
If the start node is the last node prev
will be NULL when the list traversal completes, but when that happens you are deleting the (NULL) start->NextNodePointer
, when you want to delete start itself.
Try:
void DeleteTailNode(Node *& start)
{
Node * prev = NULL;
Node * current = start;
if (start == NULL)
return;
while(current->NextNodePointer != NULL)
{
prev = current;
current = current->NextNodePointer;
}
if(current != NULL)
free(current);
if(current == start)
start = NULL;
if(prev != NULL)
prev->NextNodePointer = NULL;
}
How do you understand if it's deleted or not ? As I see, nothing here is deleted.. it's a 100% sure memory leak.. you're not using free in DeleteTailNode .. you're just making the allocated memory unaccessible..
Edit: call free( current ) after the loop. And there's no need from the check, if current is NULL, it's safe to delete NULL pointer.
精彩评论