Thread: Divided my chat into classes and whoops..

Hey guys,
I divided my chat server into a class and added a thread which is created when a user is trying to enter so he could enter the nickname and the "queue" (the loop) will be interrupted or delayed.
Well, it doesn't show any compilations errors but still, doesn't seem to work (what actually happens that the server is running but when a user connections, it doesn't show a message or neither asks the user for a nick, what could be the problem!?)
Here you go (I "translated" it into windows sockets, because I'm gonna add a GUI using WINAPI)

parsed version: http://pastebin.com/m5f703f9e (looks way better)
BTW, there's probably a problem with the shifting (stupid Dev C++, Geany is way better)
The problem appears to be this statement:

Code:

if(FD_ISSET(i, this->readyList))

Why the hell this statement goes wrong? it's supposed to success in some iterations.
(I added the cout << "We are here!";'s just so I could debug the program and see where it gets to)

Code:

#include <iostream>
#include <stdlib.h>
#include <string.h>
#include <winsock.h>
#include <unistd.h>
#include <time.h>

#define PORT 10020
#define PACKETSIZE 100
#define MAXCONNECTIONS 4
#define USERNAMELENGTH 20
#define STRING 50
#define ENTERNICK "Please enter your nickname: "
#define SOCK_ERROR -1

using namespace std;

typedef struct
{
	char *ipaddress;
	char name[USERNAMELENGTH];
} userInfo;

char *usernamebyip(userInfo *users, struct sockaddr_in *client);
void getcurrenttime(char *str, char *newstr);
void cutNewLine(char *str);
DWORD WINAPI CallRecvNick(void *ptr);

class ServerSocket
{
      private:
                int port, maxConnections, sock;
                struct sockaddr_in server;
      public:
                ServerSocket(int port = 10000, int maxConnections = 5)
                {
                     this->port = port;
                     this->maxConnections = maxConnections;      
                }
                
                int ConfigureSocket()
                {
                    WSADATA info; 
                    if (WSAStartup(MAKEWORD(1,1), &info) != 0)
                    {
                       MessageBox(NULL, "Cannot initialize WinSock!", "WSAStartup", MB_OK);
                       exit(1);
                    }
                    if ( (this->sock = socket(AF_INET, SOCK_STREAM, 0)) == SOCK_ERROR) // creating a TCP socket
                    {
                         perror("socket");
                         exit(1);     
                    }
                	return this->sock;
                }
                
                void Bind()
                {
                     this->server.sin_family = AF_INET;
            	     this->server.sin_addr.s_addr = htonl(INADDR_ANY);
            	     this->server.sin_port = htons(this->port);
            	     memset(&(this->server.sin_zero), '\0', 8);
            	     if(bind(this->sock, (struct sockaddr *)&server, sizeof(struct sockaddr)) == SOCK_ERROR) // binding to the given port
            	     {
            		     perror("bind");
            		     exit(1);
                     }
                }
                
                void Listen()
                {
                     if(listen(this->sock, this->maxConnections) == SOCK_ERROR) // listening only to MAXCONNECTIONS connections
            	     {
            		     perror("listen");
            	       	 exit(1);
                     }
                }
};

class Select
{
	private:
		userInfo users[MAXCONNECTIONS + 10];
	    int serverSock, userIndexer, *biggestSocket, tempfd;
	    fd_set *mainList, *readyList;
		struct sockaddr_in tempClient;
		char talker[PACKETSIZE/2], buffer[PACKETSIZE], data[PACKETSIZE + (PACKETSIZE/2)];
	public:
		Select(int sock, fd_set *mainList, fd_set *clients, int *biggestSocket, int userIndexer)
		{
			this->serverSock = sock;
			this->mainList = mainList;
			this->readyList = clients;
			this->biggestSocket = biggestSocket;
			this->userIndexer = userIndexer;
		}
		void StartSelecting()
		{
            *readyList = *mainList;                 
			if(select(*biggestSocket + 1, readyList, NULL, NULL, NULL) == SOCK_ERROR)
			{
				perror("select");
				exit(1);
			}
		}

		void AddClient(int newfd)
		{
            cout << "We are here!" << endl;
            this->tempfd = newfd;
			int struct_size = sizeof(struct sockaddr_in);
			getpeername(newfd, (struct sockaddr *)&tempClient, &struct_size);

			memset(users[this->userIndexer].name, '\0', USERNAMELENGTH);
			users[this->userIndexer].ipaddress = inet_ntoa(this->tempClient.sin_addr);
			
		    void *testing;
            testing = CreateThread(NULL, 0, CallRecvNick, this, 0, (LPDWORD)testing);
            if(testing == NULL)
                cout << "Couldn't create thread!";
            else
                cout << "Thread created successfully.";
		}
		
		void SendToAllButThis(int fd, char *message, int SendToAll = 0, char *msgtoUser = "")
		{
			int h = 0;
			for(; h <= *biggestSocket; h++) // informing the others that i has joined
			{
				if(FD_ISSET(h, mainList) && h != this->serverSock)
				{
					if(h != fd)
					{
						if(send(h, message, strlen(message), 0) == SOCK_ERROR)
							perror("send");
					}
					else if(SendToAll)
					{
						if(send(h, msgtoUser, strlen(msgtoUser), 0) == SOCK_ERROR)
							perror("send");
					}
				}
			}
		}
		
		void SendToAll(char *msg)
		{
             for(int h = 0; h < *biggestSocket; h++)
                  if(FD_ISSET(h, mainList) && h != this->serverSock)
                       if(send(h, msg, strlen(msg), 0) == SOCK_ERROR)
                                  perror("send");
        }
		
		void HandleMsgRecv(int i)
		{
              if(FD_ISSET(i, this->readyList))
              {
                      time_t current_time = time(0);
	                  char time[10];
                      getcurrenttime(ctime(&current_time), time);
                      cout << "We are here in FD_ISSET(i, this->readyList)" << endl;
                      if(i == this->serverSock) // there's a new connection
                      {
                             cout << "We are here in i == this->serverSock!" << endl;
                             int newfd;
   					      int size = sizeof(struct sockaddr_in);
       					  if( (newfd = accept(this->serverSock, (struct sockaddr *)&tempClient, &size)) == SOCK_ERROR)
               					perror("accept");
           				  else
      					  {
                               cout << "We are here in accept()!" << endl;
                               AddClient(newfd);
                          }     
                      }
                      else // a user has sent a message
                      {
                          char userMsg[STRING];
                          int recvNum;
                          if( (recvNum = recv(i, userMsg, STRING, 0)) == SOCK_ERROR)
                                  perror("recv");
                          else if(recvNum == 0)
                          {
       						   shutdown(i, 2);
       						   FD_CLR(i, mainList);
        					   FD_CLR(i, readyList); // we want to inform them that i left
                        						
       						   char userLeftMsg[PACKETSIZE];
          					   memset(userLeftMsg, '\0', PACKETSIZE);
       						   int struct_size = sizeof(struct sockaddr);
      						   getpeername(i, (struct sockaddr *)&tempClient, &struct_size);
                        						
                 			   sprintf(userLeftMsg, "&#37;s <%s, %s> has left the server.\n", usernamebyip((userInfo *)&users, &tempClient), inet_ntoa(tempClient.sin_addr), time);
       						   cout << userLeftMsg;
                        						
                        	   SendToAll(userLeftMsg);
                          }				
					  else
					  {
                    		 memset(&talker, '\0', PACKETSIZE/2); // holds 'IP' says
                             int struct_size_def = sizeof(struct sockaddr);
		                     getpeername(i, (struct sockaddr *)&tempClient, &struct_size_def);
                            					
  					         sprintf(talker, "%s says <%s>: ", usernamebyip((userInfo *)&users, &tempClient), time);
                            					
                             memset(&data, '\0', PACKETSIZE + (PACKETSIZE / 2));
                             strcat(data, talker);
                             strcat(data, buffer); // appending talker and buffer to data
                             cout << data;
                            					
                             SendToAllButThis(i, data);
                      }
                  }
              }    
        }
        
		void RecvNick(int newfd)
		{
			char input[USERNAMELENGTH];
			cout << "Recv nick!" << endl;
			int numOfBytesRecv;
			if(send(newfd, ENTERNICK, strlen(ENTERNICK), 0) == SOCK_ERROR)
			               perror("send");
            else
            {
		    	do
    			{
    				if((numOfBytesRecv = recv(newfd, input, USERNAMELENGTH, 0)) == SOCK_ERROR)
    				{
    					perror("recv");
    					return;
    				}
    				else if(numOfBytesRecv == 0)
    					return;
    				else
    					strcpy(users[userIndexer].name, input);
    			} while(strlen(users[userIndexer].name) < 2);
            }
			cutNewLine(users[userIndexer].name);

			FD_SET(newfd, this->mainList);
			FD_SET(newfd, this->readyList);
            if(newfd > *biggestSocket) // if it is bigger, swap because we do not want to miss this connection later in our loop
				*biggestSocket = newfd;
			DisplayWelcome(newfd, users[userIndexer].name);
			userIndexer++;
		}
		
		void DisplayWelcome(int newfd, char name[USERNAMELENGTH])
		{
			char userJoinedMsg[PACKETSIZE];
			// bzero(&userJoinedMsg, PACKETSIZE);
			sprintf(userJoinedMsg, "\n%s <%s> has joined the server.\n", name, time);
			cout << userJoinedMsg;

			char greeting[STRING/2];
			sprintf(greeting, "Hello %s and welcome to the server.\n", name);
			SendToAllButThis(newfd, userJoinedMsg, 1, greeting);
		}
		friend DWORD WINAPI CallRecvNick(void *ptr);
};

int main()
{
    // variable declarations
	int sockfd, biggestSocket, userIndexer = 0;
	struct sockaddr_in server, client;
	userInfo users[MAXCONNECTIONS + 10];
	fd_set mainList, readyList;
    
   	ServerSocket handleServer(PORT, MAXCONNECTIONS);
	sockfd = handleServer.ConfigureSocket();
	handleServer.Bind();
	handleServer.Listen();
    
    time_t now = time(0);
	cout << "CC Server is running, " <<  ctime(&now);
    
   	// initializing the lists
	FD_ZERO(&mainList);
	FD_ZERO(&readyList);
	
	biggestSocket = sockfd;
	FD_SET(sockfd, &mainList); // adding the server to our streams list
	Select select(sockfd, &mainList, &readyList, &biggestSocket, userIndexer);
    while(1)
    {
          select.StartSelecting();
          for(int i = 0; i < biggestSocket; i++)
                  select.HandleMsgRecv(i);
    }
    getchar();
    return 0;   
}

char *usernamebyip(userInfo *users, struct sockaddr_in *client)
{
     char *seekAddress = inet_ntoa(client->sin_addr);
     int i = 0;
     for(; i < MAXCONNECTIONS + 10; i++)
           if(strcmp(users[i].ipaddress, seekAddress) == 0)
                   return users[i].name;
    return "Unknown";
}

void getcurrenttime(char *str, char *newstr)
{
	int index = 0, i = 11;
	for(; i < 19; i++)
		newstr[index++] = str[i];
	newstr[index] = '\0';
}

void cutNewLine(char *str)
{
	int i = 0;
	for(; i < strlen(str); i++)
	{
		if(str[i] == '\r')
		{
			str[i] = '\0';
			break;
        }
    }
}

DWORD WINAPI CallRecvNick(void *ptr)
{
    Select *a = (Select *)ptr;
    a->RecvNick(a->tempfd);
    return 0;
}

Thanks!

As they say, post a minimal example that demonstrates the problem. It can be difficult to dig through all of that code.
Btw, you've made a mortal sin: do not mix spaces and tabs; because you did, the indentation is messed up.

Sorry, can't help with the actual problem you are having, but I have some stylistic comments.

Your code is fairly full of warnings (at least when compiling with gcc). Most are harmless ones about mixing signed and unsigned in comparisons, but changing your "int" to "unsigned int" in almost every place will fix those.

Code:

	    sprintf(userJoinedMsg, "\n%s <%s> has joined the server.\n", name, time);

The "time" here is actually the address of the function time. You do have a variable "time" in a different function:

Code:

    void HandleMsgRecv(unsigned int i)
	{
	    if(FD_ISSET(i, this->readyList))
	    {
		time_t current_time = time(0);
		char time[10];
		getcurrenttime(ctime(&current_time), time);

This does a very nasty hiding of the function time - using the same name for variables and functions is a bad thing.

I don't think you need to put all those functions inside the class declarations - it just clutters the class declaration up. I'd say the limit goes at 5-7 lines of code in a function that is inside the class, although that is of course a personal choice.

Code:

    struct sockaddr_in server, client;
    userInfo users[MAXCONNECTIONS + 10];

I personally prefer:

Code:

int i;
for(i = 0; ...)

over

Code:

int i = 0;
for(; ...)

(And in serveral places, you could use for(int i = 0; ...) instead).

--
Mats
Never used.


--
Mats

Originally Posted by eXeCuTeR

Thanks for the comment.
About the loops...how can you actually tell whether you can use, int i; for(i = 0;...) or for(int i = 0;..)?

If you use the loop variable after the end of the loop itself, then you need it declared before the loop itself [the compiler will tell you if you get it wrong].

--
Mats

Originally Posted by eXeCuTeR

Yeah, I know, the indentation is messed up here but it looks just fine in Dev C++ (where I wrote the class)

Again, because you mix spaces and tabs
That's not normally something you should do. Use either and it'll look fine.