Function validation.

Hi, can someone validate this function? I have made a chat server that adds a user to a linked list. This functions deletes some data related to a user when he/she quits. But if I join two users, and the latter user reconnects a couple of times, the element isn't deleted. Can you see some flaw in the code?

Thanks in advance!

Code:

---

#include "flp.h"

struct pending *delPenderByFD (struct pending *pFF, int match)
{


        struct pending *a = pFF;
        struct pending *b = NULL;
       
        if (a == NULL)        // no users in the list
        {
                close (match);
                FD_CLR (match, &active_fd_set);
                return (struct pending *)1;
        }
        if (a->next == NULL) // one user in the list
        {
                if ( (b = findPenderByFD (a, match)) ) //the user is in the list
                {
                        ppFirst = NULL;
                        ppNew = NULL;
                        ppLast = NULL;
                        close (match);
                        FD_CLR (match, &active_fd_set);
                        free (b);
                        return (struct pending *)1;
                }
               
                // the user is not in the list
                close (match);
                FD_CLR (match, &active_fd_set);
                free (b);
                return (struct pending *)1;
       
        }
        b = a;
        if (b == findPenderByFD (ppFirst, match) ) // while there are users - check the first
                {
                        a = b->next;
                        ppFirst = a;
                        close (match);
                        FD_CLR (b->fd, &active_fd_set);
                        free (b);
                        return (struct pending *)1;
                }
       
        while (a->next) // while there is another user
        {
                b = a->next;
                if (b == findPenderByFD (ppFirst, match) )
                {
                        a->next = b->next;
                        close (match);
                        FD_CLR (b->fd, &active_fd_set);
                        free (b);
                        return (struct pending *)1;
                }
                a = b;
               
               
        }
                // user is not in list
                close (match);
                FD_CLR (match, &active_fd_set);
                return (struct pending *)1;

}

---

Code:

---

>        while (a->next) // while there is another user
>        {
>                b = a->next;
>                if (b == findPenderByFD (ppFirst, match) )

---

Does findPenderByFD check more than one node? If not, then shouldn't this be more like:

Code:

---

        while (a->next) // while there is another user
        {
                b = a->next;
                if (b == findPenderByFD (b, match) )

---

that would be optimized, yes. But both should work.

Code:

---

return (struct pending *)1;

---

Shouldn't you return 0?

all return values are symbolic for now. I don't use them. I want to have the function up and working first :)

I don't see anything major. I see one minor problem, but doubt it would cause what you're describing.

Code:

---

>                // the user is not in the list
>                close (match);
>                FD_CLR (match, &active_fd_set);
>                free (b);
>                return (struct pending *)1;

---

You shouldn't be freeing b here.

Maybe there's a problem elsewhere (adding a node). You could post the code for findPenderByFD() and your add function.

findPenderByFD() as follows:

Code:

---

#include "flp.h"

struct pending *findPenderByFD (struct pending *pFF, int match)
{
        struct pending *ptr = pFF;
        while (ptr)
        {
                if (ptr->fd == match)
                {
                        return (ptr);
                }
                else
                {
                        ptr = ptr->next;
                }
        }
        return NULL;
}

---

And the add function?

Beside the extra free(), one other bug is the lack of updating ppLast in the case of multiple nodes.

I'm adding some more functions and related variable declarations.
All functions have a own file which include "flp.h"

from "flp.h"

Code:

---

extern struct pending *ppNew;
extern struct pending *ppFirst;
extern struct pending *ppLast;
extern struct users *pNew;
extern struct users *pFirst;
extern struct users *pLast;

---

These are all initiated to NULL.

Code:

---

struct users
{
        int pending;
        int isop;
        int value;
        int fd;        //the users file descriptor
        char name[512];
        char password[512];
        char host[512];
        int port;
        struct users *next;
};

// pending

struct pending
{
        int pending;       
        int fd;
        struct pending *next;
};

---

Code:

---

#include "flp.h"

struct users *addUser (char n[512], char p[512], int pending, int fd)
{
        pNew = (struct users *) malloc (sizeof (struct users));
        char nI[512], pI[512];
        bzero (nI, 512);
        bzero (pI, 512);
        strcpy (nI, n);
        strcpy (pI, p);
       
        strcpy (pNew->name, nI);
        strcpy (pNew->password, pI);
        pNew->fd = fd;
        pNew->value = id++;
        strcpy (pNew->host, (char *)inet_ntoa (clientname.sin_addr) );
        pNew->port = ntohs (clientname.sin_port);
        pNew->isop = 0;
        pNew->pending = pending;
        pNew->next = NULL;

                       
        if (pFirst == NULL)
        {       
                pFirst = pNew;
        }               
       
        if (pLast != NULL)
        {
                pLast->next = pNew;
    }   
        pLast = pNew;
        return (struct users *) 0;
}

---

Code:

---

#include "flp.h"

struct pending *addPender (int fd, int pending)
{
        ppNew = (struct pending *) malloc (sizeof (struct pending));

        ppNew->pending = pending;
        ppNew->fd = fd;
        ppNew->next = NULL;
                       
        if (ppFirst == NULL)
        {       
                ppFirst = ppNew;
        }               
       
        if (ppLast != NULL)
        {
                ppLast->next = ppNew;
    }   
        ppLast = ppNew;
        return (struct pending *) 0;
}

---

Code:

---

#include "flp.h"

extern int FILEDES;
void userNames (struct users *pF)
{
                struct users *ptr = pF;
                char temp[1024];
                bzero (temp, 1024);
       
                if (!ptr)
                {
                        return; // there are no users
                }
                sprintf (temp, "USERS");

                while(ptr)
        {

                        strcat (temp, " ");
                        strcat (temp, ptr->name);
                        ptr = ptr->next;
                }
                strcat (temp, "\r\n");
                write (FILEDES, temp, strlen (temp));
}

---

Code:

---

#include "flp.h"

struct users *delUserByFD (struct users *pF, int match)
{


        struct users *a = pF;
        struct users *b = NULL;
        char tempbuf[MAXMSG];
        bzero (tempbuf, MAXMSG);
       
        if (a == NULL)        // no users in the list
        {
                close (match);
                FD_CLR (match, &active_fd_set);
                return (struct users *)1;
        }
        if (a->next == NULL) // one user in the list
        {
                if ( (b = findUserByFD (a, match)) ) //the user is in the list
                {
                        pFirst = NULL;
                        pNew = NULL;
                        pLast = NULL;
                        close (match);
                        sprintf (tempbuf, "QUIT %s\r\n", b->name);
                        writeToAllClients (pFirst, tempbuf);
                        FD_CLR (match, &active_fd_set);
                        free (b);
                        return (struct users *)1;
                }
               
                // the user is not in the list
                close (match);
                FD_CLR (match, &active_fd_set);
                free (b);
                return (struct users *)1;
       
        }
        b = a;
        if (b == findUserByFD (pFirst, match) ) // while there are users - check the first
                {
                        a = b->next;
                        pFirst = a;
                        close (match);
                        sprintf (tempbuf, "QUIT %s\r\n", b->name);
                        writeToAllClients (pFirst, tempbuf);
                        FD_CLR (b->fd, &active_fd_set);
                        free (b);
                        return (struct users *)1;
                }
       
        while (a->next) // while there is another user
        {
                b = a->next;
                if (b == findUserByFD (pFirst, match) )
                {
                        a->next = b->next;
                        close (match);
                        sprintf (tempbuf, "QUIT %s\r\n", b->name);
                        writeToAllClients (pFirst, tempbuf);
                        FD_CLR (b->fd, &active_fd_set);
                        free (b);
                        return (struct users *)1;
                }
                a = b;
               
               
        }
                // user is not in list
                close (match);
                FD_CLR (match, &active_fd_set);
                return (struct users *)1;

}

---

There's without doubt something wrong with delUserByFD() and delPenderByFD()

Apprechiate you guys helping out :)

Thanks!

>There's without doubt something wrong with delUserByFD() and delPenderByFD()
Yes, as I mentioned you have a bug in these two delete functions in that you don't ever update pLast and ppLast when there are multiple nodes. Fixing that should correct your problem.