Divided my chat into classes and whoops..

This is a discussion on Divided my chat into classes and whoops.. within the C++ Programming forums, part of the General Programming Boards category; Hey guys, I divided my chat server into a class and added a thread which is created when a user ...

  1. #1
    Registered User
    Join Date
    Oct 2007
    Posts
    242

    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!
    Last edited by eXeCuTeR; 07-28-2008 at 07:03 PM.

  2. #2
    Registered User
    Join Date
    Oct 2007
    Posts
    242
    Anyone?
    Again, there's no compilation errors it just doesn't work well:
    if(FD_ISSET(i, this->readyList)) seem not to be working, although this statement is totally fine (and the select() function is fine either and should be "updating" the readyList)

  3. #3
    C++まいる!Cをこわせ! Elysia's Avatar
    Join Date
    Oct 2007
    Posts
    22,614
    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.
    Quote Originally Posted by Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

  4. #4
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    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
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  5. #5
    Registered User
    Join Date
    Oct 2007
    Posts
    242
    Quote Originally Posted by matsp View Post
    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
    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;..)?

    Btw, you've made a mortal sin: do not mix spaces and tabs; because you did, the indentation is messed up.
    Yeah, I know, the indentation is messed up here but it looks just fine in Dev C++ (where I wrote the class)

  6. #6
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    Quote Originally Posted by eXeCuTeR View Post
    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
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  7. #7
    C++まいる!Cをこわせ! Elysia's Avatar
    Join Date
    Oct 2007
    Posts
    22,614
    Quote Originally Posted by eXeCuTeR View Post
    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.
    Quote Originally Posted by Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

  8. #8
    Registered User
    Join Date
    Oct 2007
    Posts
    242
    Quote Originally Posted by Elysia View Post
    Again, because you mix spaces and tabs
    That's not normally something you should do. Use either and it'll look fine.
    Working with DevC++ is a nightmare, hehe.
    I usually program on Linux with Geany, which is SO MUCH better.

Popular pages Recent additions subscribe to a feed

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21