What is wrong with this linked list delete tail node function?

Question

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 start;
    }
}
#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

Answer 1

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.

Answer 2

Where's the call to free() ?

Try this : (Passing address of start to DeleteTailNode)

void DeleteTailNode(Node **start)
{
    Node * prev = NULL;
    Node * current = *start;
    while(current!=NULL && current->NextNodePointer != NULL)
    {
           prev = current;
           current = current->NextNodePointer;
    }

    free (current);

    if (prev != NULL)
    {
           prev->NextNodePointer = NULL;
    }
    if (prev == NULL && *start!= NULL)
        *start = NULL;
}

EDIT

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

Answer 3

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;
}