CODEWITHSUNDEEP
c
pejman behravan
pejman behravan

Reputation: 1

Delete node from linked list in c

I'm trying to make a simple chat service (socket programming) in C. Server is a concurrent and can accept multiple connections. I use thread for server and linked list to save socket id. Everything works fine except delete function which I use for deleting a node from linked list. Whenever a client types DONE, I have to delete its socket id from linked list, but it doesn't work properly. Could somebody help me to figure out what do I have to do in delete function.

here is my structure: 

struct ClientList {
    struct ClientList *Next;
    int socket;
    char username[100];
    int count;
    FILE *file;
} ;

here is insert function for adding node 

void insert(struct ClientList *newItem,int new_s) {
    pthread_mutex_lock(&mymutex);
    struct ClientList *temp = (struct ClientList *) malloc(sizeof(struct ClientList)) ;
    temp->socket = new_s;
    temp->Next = head;
    head = temp;
    pthread_mutex_unlock(&mymutex);
}//insert function

here is delete function 

int del(struct ClientList *temp,struct ClientList *newItem) {
    struct ClientList *cur = head;
    if (temp == head) {
        head = temp->Next;
        free(temp);
        return 0;   
    }//if
    else {
        while (cur) {
            if (cur->Next == temp) {
                cur->Next = temp->Next;
                free(temp);
            }//if
            cur = cur->Next;
        }//while
    }//else
}//del

For first node I don't have problem, but for all others it doesn't work.

i have to add my broadcast function which i use to broadcast any messages from any clinet to all. here is broadcast code: 

void broadcast(struct ClientList* temp,char buf[MAX_LINE],struct ClientList * newItem) {

    int len;

    pthread_mutex_lock(&mymutex); 

    for(temp = head; temp != NULL; temp  =temp->Next) {
        if (temp->socket ! =newItem->socket) {
            buf[MAX_LINE-1]= '\0';
            len = strlen(buf) + 1;
            send(temp->socket, buf, len, 0);
        }//if
    }//for

    pthread_mutex_unlock(&mymutex);
}//broadcast

Upvotes: 0

Views: 297

Answers (3)

DoxyLover
DoxyLover

Reputation: 3484

While you actual question has already been answered, here are a couple of additional comments on your delete function:

  1. Once you find the entry to be deleted, there is no reason to continue searching the list. I'd add a break statement after the second free.
  2. You don't have a return 0 statement at the end of the function. Thus, if the entry to be deleted is not at the head of the list, the return values of the function will be undefined.

Neither of these will cause your program to malfunction (unless you are checking the return value) but these are nice to correct.

Upvotes: 0

Mats Petersson
Mats Petersson

Reputation: 129524

Most likely because of the trivial mistake of only using one equal sign when wanting two:

if(cur->Next=temp)

should be:

if(cur->Next==temp)

(Also remove the extra argument that isn't needed for "del"!)

Tip: if you are using a good compiler, for example gcc, and you enable all warnings -Wall, then it would give you a warning when you make this mistake.

Upvotes: 3

Justin Edwards
Justin Edwards

Reputation: 340

You're doing assignment in your second if, not comparison. It should be:

if(cur->Next == temp)

not

if(cur->Next=temp)

Upvotes: 4

Related Questions