UDP multi-client chat server

Question

I have a multi-client chat server and for some reason only the first client is being added. I used a tutorial to help get me started. I have included my code below. When I try and add another client it doesnt appear to be added. If I add one client I get a response from the server like I want but only the first message I enter then after that it stops sending correctly.

Server Code:

 int main(void)  
 {  
 struct sockaddr_in my_addr, cli_addr[10],cli_temp;  
 int sockfd;  
 socklen_t slen[10],slen_temp;

 slen_temp = sizeof(cli_temp);
 char buf[BUFLEN];  
 int clients = 0;
 int client_port[10];

 if ((sockfd = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP))==-1)
 {
     printf("test\n");
     err("socket");  
 }else{  
     printf("Server : Socket() successful\n");  
 }   
 bzero(&my_addr, sizeof(my_addr));  
 my_addr.sin_family = AF_INET;  
 my_addr.sin_port = htons(PORT);  
 my_addr.sin_addr.s_addr = htonl(INADDR_ANY);  

 if (bind(sockfd, (struct sockaddr* ) &my_addr, sizeof(my_addr))==-1)
 {
     err("bind");  
 }else{
     printf("Server : bind() successful\n");  
 }

 int num_clients = 0;
 while(1)  
 {
     //receive
     printf("Receiving...\n");
     if (recvfrom(sockfd, buf, BUFLEN, 0, (struct sockaddr*)&cli_temp, &slen_temp)==-1)  
         err("recvfrom()");

     if (clients <= 10) {
        cli_addr[clients] = cli_temp;
        client_port[clients] = ntohs(cli_addr[clients].sin_port);
        clients++;
        printf("Client added\n");
        //printf("%d",clients);
        int i;
        for(i=0;sizeof(clients);i++) {
            sendto(sockfd, buf, BUFLEN, 0, (struct sockaddr*)&cli_addr[i], sizeof(cli_addr[i]));

        }
     }
 }  

 close(sockfd);  
 return 0;

}

I have included the client code as well in case it helps.

void err(char *s)  
{  
perror(s);  
exit(1);  
}  
sig_atomic_t child_exit_status;

void clean_up_child_process (int signal_number) 
{
/* Clean up the child process. */ 
int status; 
wait (&status); 
/* Store its exit status in a global variable. */ 
child_exit_status = status;
}

 int main(int argc, char** argv)  
 {  
 struct sockaddr_in serv_addr;  
 int sockfd, slen=sizeof(serv_addr);  
 char buf[BUFLEN];  
 struct sigaction sigchld_action; 
 memset (&sigchld_action, 0, sizeof (sigchld_action)); 
 sigchld_action.sa_handler = &clean_up_child_process; 
 sigaction (SIGCHLD, &sigchld_action, NULL);
 int pid,ppid;

 if(argc != 2)  
 {  
   printf("Usage : %s <Server-IP>\n",argv[0]);  
   exit(0);  
 }  

 if ((sockfd = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP))==-1)  
     err("socket");  

 bzero(&serv_addr, sizeof(serv_addr));  
 serv_addr.sin_family = AF_INET;  
 serv_addr.sin_port = htons(PORT);  
 if (inet_aton(argv[1], &serv_addr.sin_addr)==0)  
 {  
     fprintf(stderr, "inet_aton() failed\n");  
     exit(1);  
 }
 pid = fork();
 if (pid<0) {
     err("Fork Error");
 }else if (pid==0) {
     //child process will receive from server
     while (1) {
         bzero(buf,BUFLEN);
         //printf("Attempting to READ to socket %d: ",sockfd);
         fflush(stdout);
         //recvfrom here
         if (recvfrom(sockfd, buf, BUFLEN, 0, (struct sockaddr*)&serv_addr, &slen)==-1)  
             err("recvfrom()");

         printf("The message from the server is: %s \n",buf);
         if (strcmp(buf,"bye\n") == 0) {
             ppid = getppid();
             kill(ppid, SIGUSR2);

             break;
         }
     }
 }else {
     //parent will send to server
     while(1){
         printf("Please enter the message to send: ");
         bzero(buf,BUFLEN);
         fgets(buf,BUFLEN,stdin);
         printf("Attempting to write to socket %d: ",sockfd);
         fflush(stdout);
         //send to here
         if (sendto(sockfd, buf, BUFLEN, 0, (struct sockaddr*)&serv_addr, slen)==-1)
         {
             err("sendto()");
         }
     }
 }
 close(sockfd);  
 return 0;

}

You fill out another entry in your arrays every time you receive a UDP packet. That means that if a single client sends 10 UDP packets, your array would have 10 copies of the same client address and could not handle any other clients. That's probably not what you want. — Jeremy Friesner

Chris Ryding Chris Ryding · Accepted Answer · 2013-05-09T23:24:49

Several problems jump out at me. First, every time you receive a message it will consider that to be a new client. Instead of just incrementing the clients variable for a message, you'll need to scan through the array to see if the source address is already present. Second, sizeof(clients) will return a static value (probably 4) depending on how many bytes an int occupies on your machine. That loop should be for( int i = 0; i < clients; i++ ).

You also have a variable named num_clients which is not used. Is that supposed to be there for something and maybe is causing some confusion?

Finally, instead of using the magic value 10 all over the place, use #define MAX_CONNECTIONS 10 and then replace all those numbers with MAX_CONNECTIONS. It's a lot easier to read and change later.

UDP multi-client chat server

1 Answers