Remove value from a List

Question

I'm writing a program which creates lists, print it and remove from the list (3 functions).

Print and pushBack are fine, they work nice but I can't figure out how to pick up a number to remove from the list in removeFromList() function.

Don't pay attention on names (like client, socket), it's for my client-server application to save active sockets (that's why I need to remove them from a list when client has disconnected).

Here I have 2 structures: listElement and clientList (which contains a pointer to head element of listElement)

struct listElement
{
    SOCKET socket;
    struct listElement* next;
};

struct clientList
{
    listElement * head;
};

My pushBack function:

int pushBackСlient(struct clientList* list, int socket)
{
    struct listElement* newClient = (struct listElement*)malloc(sizeof(struct listElement));
    struct listElement* currentElement = list->head;
    newClient->socket = socket;
    newClient->next = 0;
    do
    {
        // IT'S PUSHBACK
        if (list->head == 0)
        {
            list->head = newClient;
            break;
        }
        while (currentElement->next != 0)
        {
            currentElement = currentElement->next;
        }
        currentElement->next = newClient;
    } while (false);

    return 0;
}

My print:

void print(struct clientList* list)
{
    struct listElement* currentElement = list->head;
    while (currentElement != 0)
    {
        printf("%d\n", currentElement->socket);
        currentElement = currentElement->next;
    }
}

And the function I have a problem with (I made debug messages to see if a "socket" was added correctly). I suppose I don't need the first 3 lines but not sure.

Updated13/05/2017

void removeFromList(struct clientList* list, int socket)
{
    struct listElement* currentElement = list->head;
    do
    {
        if (list->head == 0)
        {
            return;
        }

        while (currentElement != 0 && currentElement->next != 0)
        {
            if (currentElement->socket == socket)
            {
                printf("currentElement == %d\n", currentElement);
                currentElement = currentElement->next;
                printf("currentElement == %d\n", currentElement);
                free(currentElement);
                //break; // if I only want to remove the first socket?
            }
            currentElement = currentElement->next;
        }
    } while (false);
}

Thank you.

Answer 1

The function removeFromList is wrong at least because this condition of the while statement can be equal to false when the list contains only one element. In this case even this one element contains the target vakue it will not be removed.

while (currentElement != 0 && currentElement->next != 0)

The functions can look as it is shown in the demonstrative program.

#include <stdio.h>
#include <stdlib.h>

typedef int SOCKET;

struct listElement
{
    SOCKET socket;
    struct listElement *next;
};

struct clientList
{
    struct listElement *head;
};

int pushBackClient( struct clientList *list, SOCKET socket )
{
    struct listElement *newClient = malloc( sizeof( struct listElement ) );
    int success = newClient!= NULL;

    if ( success )
    {
        newClient->socket = socket;
        newClient->next   = NULL;

        struct listElement **current = &list->head;

        while ( *current != NULL ) current = &( *current )->next;

        *current = newClient;
    }

    return success;
}


int removeFromList( struct clientList *list, SOCKET socket )
{
    int success;

    struct listElement **current = &list->head;

    while ( *current != NULL && ( *current )->socket != socket ) 
    {
        current = &( *current )->next;
    }

    if ( ( success = *current != NULL ) )
    {
        struct listElement *tmp = *current;
        *current = ( *current )->next;

        free( tmp );
    }

    return success;
}

void print(struct clientList *list)
{
    for ( struct listElement *current = list->head;
          current != NULL;
          current = current->next )
    {         
        printf( "%d ", current->socket );
    }
}

int main(void) 
{
    const int N = 10;
    struct clientList list = { NULL };

    for ( int i = 0; i < N; i++ ) pushBackClient( &list, i );

    print( &list );
    putchar( '\n' );

    for ( int i = 0; i < N; i++ )
    {
        if ( i % 2 == 0 ) removeFromList( &list, i );
    }

    print( &list );
    putchar( '\n' );

    for ( int i = 0; i < N; i++ )
    {
        if ( i % 2 == 1 ) removeFromList( &list, i );
    }

    print( &list );
    putchar( '\n' );

    return 0;
}

The program output is

0 1 2 3 4 5 6 7 8 9 
1 3 5 7 9 

You need at least to add a function that will free all elements of the list.

Answer 2

For your remove function I suggest something like this:

void removeFromList(struct clientList* list, int socket)
{
    struct listElement* aux, prev;
    if(list->head == 0)
        return;

    aux = list->head;
    prev = aux;

    while(aux != 0){
        if(aux->socket == socket) {
            prev->next = aux->next;
            free(aux);
            break; // if you only want to remove the first socket
        }
        prev = aux;
        aux = aux->next;    
    }
} 

As for your list structure, I suggest using a structure of structs, like follows:

struct list
{
    int numberOfElements;
    NODE * first;
} LIST;

struct node
{
    ELEMENT * info;
    NODE * prev; // If you want to have a double connection between the nodes
    NODE * next;
} NODE;

struct element
{
    int id;
    /* Other Properties */
} ELEMENT;

It should give you a better control of your list.