Thread: Function validation.

  1. #1
    Registered User
    Join Date
    Feb 2006
    Posts
    6

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

  2. #2
    Registered User
    Join Date
    Oct 2001
    Posts
    2,934
    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) )

  3. #3
    Registered User
    Join Date
    Feb 2006
    Posts
    6
    that would be optimized, yes. But both should work.

  4. #4
    Frequently Quite Prolix dwks's Avatar
    Join Date
    Apr 2005
    Location
    Canada
    Posts
    8,057
    Code:
    return (struct pending *)1;
    Shouldn't you return 0?
    dwk

    Seek and ye shall find. quaere et invenies.

    "Simplicity does not precede complexity, but follows it." -- Alan Perlis
    "Testing can only prove the presence of bugs, not their absence." -- Edsger Dijkstra
    "The only real mistake is the one from which we learn nothing." -- John Powell


    Other boards: DaniWeb, TPS
    Unofficial Wiki FAQ: cpwiki.sf.net

    My website: http://dwks.theprogrammingsite.com/
    Projects: codeform, xuni, atlantis, nort, etc.

  5. #5
    Registered User
    Join Date
    Feb 2006
    Posts
    6
    all return values are symbolic for now. I don't use them. I want to have the function up and working first

  6. #6
    Registered User
    Join Date
    Oct 2001
    Posts
    2,934
    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.

  7. #7
    Registered User
    Join Date
    Feb 2006
    Posts
    6
    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;
    }

  8. #8
    Registered User
    Join Date
    Oct 2001
    Posts
    2,934
    And the add function?

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

  9. #9
    Registered User
    Join Date
    Feb 2006
    Posts
    6
    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;
    
    }

  10. #10
    Registered User
    Join Date
    Feb 2006
    Posts
    6
    There's without doubt something wrong with delUserByFD() and delPenderByFD()

    Apprechiate you guys helping out

    Thanks!

  11. #11
    Registered User
    Join Date
    Oct 2001
    Posts
    2,934
    >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.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Seg Fault in Compare Function
    By tytelizgal in forum C Programming
    Replies: 1
    Last Post: 10-25-2008, 03:06 PM
  2. Another syntax error
    By caldeira in forum C Programming
    Replies: 31
    Last Post: 09-05-2008, 01:01 AM
  3. In over my head
    By Shelnutt2 in forum C Programming
    Replies: 1
    Last Post: 07-08-2008, 06:54 PM
  4. Undefined Reference Compiling Error
    By AlakaAlaki in forum C++ Programming
    Replies: 1
    Last Post: 06-27-2008, 11:45 AM
  5. const at the end of a sub routine?
    By Kleid-0 in forum C++ Programming
    Replies: 14
    Last Post: 10-23-2005, 06:44 PM