Singly Linked-List (How to properly remove/free an entity?)

1

Background

I have the following code

VOID FD_Remove(FDescs List, PVOID FileDesc) {
    while (List) {
        if (List->Sock == FileDesc) {
            List->Next = (FDescs) LocalAlloc(sizeof(Network::FDesc));
            if (List->Next != ERROR) {
                FDescs Element = List->Next;

                List->Sock = List->Next->Sock;
                List->Host = List->Next->Host;
                List->Path = List->Next->Path;

                List->Next = Element->Next;
                LocalFree(Element);
            }
        } List = List->Next;
    }
}

Notes

Basically, when I am creating a new entry in the linked list. I will allocate a new memory block of the given structure size and set it to the end of the list, finding a block the same way as I do in this _Remove function.

However, when I am trying to remove an entity. I free the entity from the list as seen above, and set the current entity to the next entry in the list. The problem is, it appears that the LocalFree call doesn't actual set the allocated memory back to an unallocated block of memory. When I look at the linked-list the current entity simply has all of its entries set to NULL (0).

Assume the FileDesc that I am passing to FD_Remove is the last entry of the list.

FD_Remove(FDescList, 0x00000005);

So we can assume that the memory that has the 0x00000005 entity is now freed.

Now I call FD_Add(FDescList, FileDesc, 'whatever');

I would only assume that these values would appear in the previously freed entity in the Linked-list.

Problem

The problem is that entity is set to NULL instead of freed memory, and the function adds the FileDesc and 'whatever' data to the entity after the NULL memory block instead of inside of it.

Question

What am I doing wrong in my FD_Remove function that is causing this, and how can I improve my function to fix this problem?

c
winapi
singly-linked-list
asked on Stack Overflow Dec 20, 2017 by (unknown user)

1 Answer

1

Removing an element from a linked list simply involves updating the surrounding element pointers to the element being removed, and then freeing the element. You don't need to allocate or copy anything.

Try this instead:

VOID FD_Remove(FDescs List, PVOID FileDesc) {
    FDescs Previous = NULL;
    while (List) {
        if (List->FDesc == FileDesc) {
            if (Previous)
                Previous->Next = List->Next;
            LocalFree(List);
            return;
        }
        Previous = List;
        List = List->Next;
    }
}

FD_Remove(FDescList, (PVOID)0x00000005);

But, watch out if the element being removed is the head of your linked list. If so, you need to update that pointer too, which the above function can't do directly. You would have to do something more like this instead:

VOID FD_Remove(FDescs *List, PVOID FileDesc) {
    if (!List) return;
    FDescs Element = *List, Previous = NULL;
    while (Element) {
        if (Element->FDesc == FileDesc) {
            if (*List == Element)
                *List = Element->Next;
            if (Previous)
                Previous->Next = Element->Next;
            LocalFree(Element);
            return;
        }
        Previous = Element;
        Element = Element->Next;
    }
}

FD_Remove(&FDescList, (PVOID)0x00000005);
answered on Stack Overflow Dec 20, 2017 by Remy Lebeau

User contributions licensed under CC BY-SA 3.0