Sign in to follow this  
Matty_alan

Chat Server Program Bug

Recommended Posts

I worte a Client\Server chat room program a few weeks ago and it worked just fine (to a degree) so in an effort to finely polish it and make it perfect iv'e included the use of linked lists so i can delete disconnected users sockets and give a (theoretically) infinate amount of clients to connect to However somewhere along the lines a bug has appeared that will only accept 3 clients then it seems to ignore the rest But if I comment out the line marked below the program will let me accept as much users as I want but then make the program unuseable so Im unsure on how to get around the problem. Any leads on these probs would be awesome because iv'e gone over the code so many times and everything just seems to be in order to me. Heres the Problematic code (The Listening socket and clients socets are all non blocking)
[source lang = cpp]
#define WELCOMEMSG "Welcome Console Chat V1.0 -- PRESS '^' TO EXIT!"

//My client structure
struct Client //Define our Client Data structure
{
  char Name[10];//this isn't being used yet...
  int  Socket;

  struct sockaddr_storage client_addr;
  socklen_t addr_size;  

  Client *Next;// for the linked list
};


int main()
{

Initialize, Bind Listen ect......



/* LISTENING LOOP */
while(1)
{

    Point->Socket = -1;

     while(Point->Socket <= -1)
     while(TempSock <= -1)
     {

    Point = Root;
    while(Point->Next != 0)
        {Point = Point->Next;}

         Point->Socket = accept(sockdis, (struct sockaddr *)&Point->client_addr, &Point->addr_size);


    Point = Root;
    while(Point->Next != 0)
        {


                 bytesrecvd = recv(Point->Socket , (char*)Data, sizeof Data, 0);                 // RECIVE DATA FROM EACH CLIENT

                    if(bytesrecvd > 1)                                                           //IF MORE THEN ONE BYTE WAS SENT 
                    {
                      status = WSAGetLastError();
                      cout << "ErrorNo; " << status << endl;

                      cout << Data << endl <<"Reviced Returned: " << bytesrecvd << endl;

                                Temp = Point;
                                Point = Root;
                                Last = Point;

                          while(1)                                         
                          {

                                //SEND RECIVED DATA TO ALL CLIENTS
                          }
                    }

     }// END OF WHILE


//goto Last Client
Point = Root;
while(Point->Next != 0)
{Point = Point->Next;}

Point->Socket = TempSock;

    cout << "Client Connected!" <<endl << "Client Port: " << Point->Socket << "\n\n";

    send(Point->Socket , (char*)WELCOMEMSG, sizeof WELCOMEMSG, 0);//Send welcome message

 
Point->Next = new Client;
Point = Point->Next; //<----- IF I COMMENT OUT HIS LINE IT WILL ACCEPT MORE CLIENTS BUT MAKE THE PROGRAM NOT FUNCTION
Point->Socket = -1;
Point->Next = 0;
ioctlsocket(Point->Socket, FIONBIO, &mode);

//    system("pause");

}

[Edited by - Matty_alan on April 28, 2010 4:54:09 AM]

Share this post


Link to post
Share on other sites
I haven't examined your code. But why don't you use an STL map to store Sessions.
Insertion and deletions are pretty fast, and you don't have to deal with the problems of writing your own Data Structure.

Share this post


Link to post
Share on other sites
Maybe I missed it but where are the list nodes coming from? You need to new/calloc for a new node everytime you accept a socket and add it to the list.

Share this post


Link to post
Share on other sites
There are all sorts of bugs there. You have a condition "while(Point->Next != 0)" but the code inside might never hit the function that changes the value of Point. (You don't check to see if recv fails or the other side disconnects, for example.) You check "if(bytesrecvd > 1)", yet if bytesrecvd == 1 you won't enter the block, despite that being a perfectly valid return value. You have a while condition of "while(TempSock <= -1)" but you never change that value in the code you posted. Plus all those nested while loops below are almost certainly doing the wrong thing, too. If you get 'stuck' in any one of them - which is quite likely, given the problems I can see with the code you've posted, never mind the code you left out - then you'd never accept another connection.

Also, why are you managing your own linked list? It's 2010, if you're using C++ there's almost no reason not to use one of the standard containers. If you don't know about the standard containers, now's the time to learn. It would also be wise to upgrade your Client from a struct to a class and to write a proper constructor for it.

I also suggest you get used to using a debugger as I'm sure that if you stepped through the code you'd see exactly why it seems to ignore further clients.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this