Not being able to send message between clients (C)

Question

I want to have a minitalk done using client-server (server with multiply clients). Client is launched with the name parameter, and connection to the server is done.

The server remembers names of every client. Then, in every client I can send message to another - for example I have 2 clients launched - c1 and c2, I can send message from c1 - c2 hello and it should be shown in c2 as a received one. I can also send from c1 to c1, it doesn't matter. What I expect is below the clients code (because server works well)

    #include <stdio.h> 
#include <string.h>   //strlen 
#include <stdlib.h> 
#include <errno.h> 
#include <unistd.h>   //close 
#include <arpa/inet.h>    //close 
#include <sys/types.h> 
#include <sys/socket.h> 
#include <netinet/in.h> 
#include <sys/time.h> //FD_SET, FD_ISSET, FD_ZERO macros 
#define PORT 8080

int main(int argc, char *argv[])
{
    struct sockaddr_in address;
    int sock = 0, thisclient = 0, valread;
    struct sockaddr_in serv_addr;
    int activity;

    fd_set readfds; 

    char *hello = malloc(100);
    char tab[1024] = {0};
    char* name = argv[1];

    if((thisclient = socket(AF_INET , SOCK_STREAM , 0)) == 0)  
    {  
        perror("socket failed");  
        exit(EXIT_FAILURE);
    } 

    memset(&serv_addr, '0', sizeof(serv_addr));

    serv_addr.sin_family = AF_INET;
    serv_addr.sin_port = htons(PORT);

    // Convert IPv4 and IPv6 addresses from text to binary form
    if(inet_pton(AF_INET, "127.0.0.1", &serv_addr.sin_addr)<=0) 
    {
        printf("\nInvalid address/ Address not supported \n");
        return -1;  
    }

    if (connect(thisclient, (struct sockaddr *)&serv_addr, sizeof(serv_addr)) < 0) {
        printf("\nConnection Failed \n");
        return -1;
    } else {

    send(thisclient, name, strlen(name), 0);
    }



    valread = read(thisclient , tab, 1024);

    if(strcmp(tab,name) != 0) {
        printf("Already used user!");
        return -1;
    } else {
        bzero(tab, sizeof(tab));
        send(thisclient, "Connection", 11, 0);
        valread = read(thisclient , tab, 1024); 
        printf("%s---\nSending message to not in a list causes server exiting\n---\n",tab);
        bzero(tab, sizeof(tab));
    }



    while(1) { 

     FD_ZERO(&readfds);
     FD_SET(0,&readfds); //stdin
     FD_SET(thisclient,&readfds);

        activity = select( thisclient+1 , &readfds , NULL , NULL , NULL);  

        if ((activity < 0) && (errno!=EINTR))  
        {  
            printf("select error");  
        }  

        for(int i=0; i<thisclient+1; ++i) {

            if (FD_ISSET( i , &readfds))  
            { 

                if(i== thisclient) {
                    char* buffer = malloc(1025);

                    if ((valread = read( thisclient , buffer, 1024)) != 0) {  
                        if(strcmp(buffer,"Server error, is closing") == 0) {
                            exit(0);
                        }

                        printf("Received (yours name) (message): %s\n",buffer );
                        bzero(buffer, sizeof(buffer));

                    } 
                    free(buffer);
                }
                else if(i == 0) {
                    char* buffer = malloc(1025);
                    if ((valread = read( 0 , buffer, 1024)) != 0) {  
                        if(strcmp(buffer,"Server error, is closing") == 0) {
                            exit(0);
                        }

                        send(thisclient, buffer, 1024, 0);
                        printf("Send (yours name) (message): %s\n",buffer );
                        bzero(buffer, sizeof(buffer));

                    } 
                    free(buffer);
                }
            }
        }

    }
    return 0;
}

I should be able to send messages like "ping-pong", so c1 sends to c2, c2 sends to c1, c1 sends two messages to c2, c2 sends one to c1 etc..

At the moment, the connection works well, then I send first message (it doesn't matter who is sender and receiver, for example c1 sends to c2). I can send many messages from c1 to c2, but when I want to respond (message from c2 to c1) nothing happens.

I also noticed, that this message isn't even is server, so it's certainly a client's fault. I can still send messages from c1 to c2, but not inside out. What "flushes" this state? I send a message from c1 to c1. After this message, the previous ones from c2 are shown in c1 too (with the most recent from c1 to c1). But I have to "flush" every time when I want to receive message from c2.

Should I add something to select() call? (For example the parameter for writing). Or the loop isn't done well?

-EDIT server code

#include <stdio.h> 
#include <string.h>   //strlen 
#include <stdlib.h> 
#include <errno.h> 
#include <unistd.h>   //close 
#include <arpa/inet.h>    //close 
#include <sys/types.h> 
#include <sys/socket.h> 
#include <netinet/in.h> 
#include <sys/time.h> //FD_SET, FD_ISSET, FD_ZERO macros 

#define TRUE   1 
#define FALSE  0 
#define PORT 8080

int main(int argc , char *argv[])  
{  
    int opt = TRUE;  
    int master_socket , addrlen , new_socket , client_socket[30] , 
          max_clients = 30 , activity, i , valread , sd;  
    int max_sd;  
    struct sockaddr_in address;  

    char client_name[30][100];


    //set of socket descriptors 
    fd_set readfds;  

    //initialise all client_socket[] to 0 so not checked 
    for (i = 0; i < max_clients; i++)  
    {  
        client_socket[i] = 0;  
    }  

    //create a master socket 
    if( (master_socket = socket(AF_INET , SOCK_STREAM , 0)) == 0)  
    {  
        perror("socket failed");  
        exit(EXIT_FAILURE);  
    }  

    //set master socket to allow multiple connections , 
    //this is just a good habit, it will work without this 
    if( setsockopt(master_socket, SOL_SOCKET, SO_REUSEADDR, (char *)&opt, 
          sizeof(opt)) < 0 )  
    {  
        perror("setsockopt");  
        exit(EXIT_FAILURE);  
    }  

    //type of socket created 
    address.sin_family = AF_INET;  
    address.sin_addr.s_addr = INADDR_ANY;  
    address.sin_port = htons( PORT );  

    //bind the socket to localhost port 8888 
    if (bind(master_socket, (struct sockaddr *)&address, sizeof(address))<0)  
    {  
        perror("bind failed");  
        exit(EXIT_FAILURE);  
    }  
    printf("Listener on port %d \n", PORT);  

    //try to specify maximum of 3 pending connections for the master socket 
    if (listen(master_socket, 3) < 0)  
    {  
        perror("listen");  
        exit(EXIT_FAILURE);  
    }  

    //accept the incoming connection 
    addrlen = sizeof(address);  
    puts("Waiting for connections ...");  

    while(TRUE)  
    {  
        //clear the socket set 
        FD_ZERO(&readfds);  

        //add master socket to set 
        FD_SET(master_socket, &readfds);  
        max_sd = master_socket;  

        //add child sockets to set 
        for ( i = 0 ; i < max_clients ; i++)  
        {  
            //socket descriptor 
            sd = client_socket[i];  

            //if valid socket descriptor then add to read list 
            if(sd > 0)  
                FD_SET( sd , &readfds);  

            //highest file descriptor number, need it for the select function 
            if(sd > max_sd)  
                max_sd = sd;  
        }  

        //wait for an activity on one of the sockets , timeout is NULL , 
        //so wait indefinitely 
        activity = select( max_sd + 1 , &readfds , NULL , NULL , NULL);  

        if ((activity < 0) && (errno!=EINTR))  
        {  
            printf("select error");  
        }  

        //If something happened on the master socket , 
        //then its an incoming connection 
        if (FD_ISSET(master_socket, &readfds))  
        {  
            if ((new_socket = accept(master_socket, 
                    (struct sockaddr *)&address, (socklen_t*)&addrlen))<0)  
            {  
                perror("accept");  
                exit(EXIT_FAILURE);  
            }  

            //inform user of socket number - used in send and receive commands 
            printf("New connection, ip- %s, port- %d\n" , inet_ntoa(address.sin_addr) , ntohs
                  (address.sin_port));  

            char* tempclient = malloc(100);
            read(new_socket, tempclient, 100);


            //add new socket to array of sockets 
            for (i = 0; i < max_clients; i++)  
            {  
                //if position is empty 
                if( client_socket[i] == 0 )  
                {  
                    client_socket[i] = new_socket;
                    strcpy(client_name[i], tempclient);
                    printf("Adding to list of sockets as %s - pos %d\n",client_name[i],i);  
                    send(new_socket, client_name[i], strlen(client_name[i]), 0);
                    read(new_socket, tempclient, 100);

                    char list[1000];
                    strcpy(list, "List of users\n");
                    for(int j=0; client_socket[j] != 0; ++j) {
                        strcat(list, client_name[j]);
                        strcat(list,"\n");
                    }
                    send(new_socket, list, strlen(list), 0);
                    break;  
                } else if(strcmp(client_name[i],tempclient) == 0) {
                    send(new_socket, "ERROR", 5, 0);
                    break;
                } else if(strcmp(tempclient,"exit ") == 0) {
                    send(new_socket, "ERROR", 5, 0);
                    break;
                }
            }  
        }  

        //else its some IO operation on some other socket
        for (i = 0; i < max_clients; i++)  
        {  
            char* buffer = malloc(1025);
            char* name = malloc(100);
            sd = client_socket[i];  

            if (FD_ISSET( sd , &readfds))  
            {  

                //Check if it was for closing , and also read the 
                //incoming message 
                if ((valread = read( sd , buffer, 1024)) == 0)  
                {  
                    //Somebody disconnected , get his details and print 
                    getpeername(sd , (struct sockaddr*)&address , \
                        (socklen_t*)&addrlen);  
                    printf("Host disconnected , ip %s , port %d \n" , 
                          inet_ntoa(address.sin_addr) , ntohs(address.sin_port));  

                    //Close the socket and mark as 0 in list for reuse 
                    close( sd );  
                    client_socket[i] = 0;  
                }       
                //Echo back the message that came in 
                else {  
                    strcpy(name, buffer);
                    strtok(name, " ");
                    puts("bufor = ");
                    puts( buffer);
                    for (i = 0; i < max_clients; i++) //loop seraching for name
                    {
                        if( client_socket[i] != 0 ) { //client in array
                            if(strcmp(client_name[i], name) == 0) {// message name same as client name

                                 send(client_socket[i] , buffer , 1024 , 0 );
                                 bzero(buffer, sizeof(buffer));
                                bzero(name, sizeof(name)); 
                                 break;
                            }
                        }
                        else { //no more clients
                            for (i = 0; i < max_clients; i++) //loop after clients 
                            {
                                if( client_socket[i] != 0 ) { //send error
                                    send(client_socket[i], "Server error, is closing", 30, 0);
                                } else break;
                            }

                            puts("Message error, whole communication is closing\n");
                            exit(0);
                        }
                    }
                    bzero(buffer, sizeof(buffer));
                    bzero(name, sizeof(name));
                }  
            }  
             free(buffer);
             free(name);
        }

    } 

    return 0;  
}  

Answer 1

The primary error is in these loops in the server:

        //else its some IO operation on some other socket
        for (i = 0; i < max_clients; i++)  
        {  
            …
                //Check if it was for closing , and also read the 
                //incoming message 
                if ((valread = read( sd , buffer, 1024)) == 0)  
                …
                //Echo back the message that came in 
                else {  
                    …
                    for (i = 0; i < max_clients; i++) //loop seraching for name
                    …
                                 break;
                    …
                }  
            …
        }

The same loop variable i is used in the inner loop as in the outer one, leading to the attempt to read the same client message twice, thus blocking the server until that client sends something anew.

A secondary error is that tempclient is not always null-terminated, though used as a string. Quick fix: replace char* tempclient = malloc(100) with char *tempclient = calloc(100, 1).