C recv function doesnt work all the time, it sometimes doesnt read and store all incoming data

Question

We are working on a project where we want to communicate with a server.

This is our function to communicate with the server, but somehow it does not read the incoming messages correctly all the time.

Sometimes in the buffer there is something like:

(Server sends)"+ Client version acClient: ID 38ail6ii3s8jc"

instead of:

(Server sends)"+ Client version accepted - please send Game-ID to join"

(We send)"Client: ID 38ail6ii3s8jc"

So, I think the error is within the char *receiveAnswer(int sock) function.

#include <stdio.h>
#include <stdlib.h>
#include <sys/socket.h>
#include <sys/types.h>
#include <string.h>

#define BUFFERSIZE 1024
#define bzeroNew(b,len) (memset((b), '\0', (len)), (void) 0) //buffer loeschen
#define VERSION "VERSION 3.4\n"

#include "functions.h"

char buffer[BUFFERSIZE];
int prologEnd = 0;
int proof;

//liest von Server eine Nachricht ein und speichert sie im buffer ab
char *receiveAnswer(int sock) {
    bzeroNew(buffer, BUFFERSIZE);
    if(recv(sock, buffer, sizeof(buffer), 0) < 0) {
       perror("ERROR: Empfangen fehlgeschlagen\n");
    }
    printf("%s", buffer);
    return buffer;
}

void sendResponse(int sock, char* message) {
    bzeroNew(buffer, BUFFERSIZE);
    strcpy(buffer, message);
    proof = send(sock, buffer, strlen(buffer), 0);
    if(proof < 0) {
        perror("ERROR: Senden fehlgeschlagen\n");
    }
    printf("Client: %s\n", buffer);
    receiveAnswer(sock);
}

int performConnection(int sock, char* gameID) {
    
    bzeroNew(buffer, BUFFERSIZE);
    receiveAnswer(sock);
    while(strncmp(buffer, "+", 1) == 0 && prologEnd == 0) {  
        if(strncmp(buffer, "+ MNM Gameserver", 16) == 0) {
               receiveAnswer(sock);
               sendResponse(sock, VERSION);
        } 
            else if(strncmp(buffer, "+ Client", 8) == 0) {
               sendResponse(sock, gameID);
            }

            else if(strncmp(buffer, "+ PLAYING", 9) == 0) {
               sendResponse(sock, "PLAYER\n");
               receiveAnswer(sock);
            }

            else if(strncmp(buffer, "+ YOU", 5) == 0) {
               receiveAnswer(sock);
               printf("\n");
               prologEnd = 1;
            }
            
            else if(strncmp(buffer, "+ TOTAL", 7) == 0) {
               receiveAnswer(sock);
               receiveAnswer(sock);
               prologEnd = 1;
         } 
    }

    bzeroNew(buffer, BUFFERSIZE);

    return 0;
}

This is our main() function, but I think the error is within the file above:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#include <sys/socket.h>
#include <sys/types.h>
#include <sys/wait.h> // für Warten auf Kindprozess
#include <unistd.h>

#include <netdb.h>
#include <netinet/in.h>

#include <fcntl.h>
#include <limits.h>
#include <signal.h>

// für Shared Memory:
#include <sys/ipc.h>
#include <sys/shm.h>

#include "functions.h"
#include "sharedMemory.h"

// dublicat, brauchen wir das?
#define GAMEKINDNAME "NMMorris"
#define HOSTNAME "sysprak.priv.lab.nm.ifi.lmu.de"
#define PORTNUMBER 1357

int main (int argc, char *argv[]) {

    char gamekindname[256] = "NMMorris";
    char hostname[256] = "sysprak.priv.lab.nm.ifi.lmu.de";
    int portnumber = 1357;

    char* gameID = argv[2];
    char playerNumber[256];
    char configFile[256] = "client.conf" ;
    int fd[2]; // TODO: fd und client_fd vereinen
    

    //gameID formatieren
    char bufferGameID[64];
    strcpy(bufferGameID, "ID ");
    strcat(bufferGameID, gameID);
    strcpy(gameID, bufferGameID);
    strcat(gameID, "\n");

    int i;
    char tmp[256];

    //Argumente einlesen und an Variablen übergeben
    for(i = 3; i < 7; i++) {
        strcpy(tmp, argv[i]);
        if (strcmp(tmp, "-p") == 0){
            strcpy(playerNumber, argv[i+1]);
        } else if (strcmp(tmp, "-conf") == 0){
            strcpy(configFile, argv[i+1]);
        } 
    }

    
    config configMain = readConfig(configFile);
    
    strcpy(gamekindname, configMain.gameKind);
    strcpy(hostname, configMain.hostServerName);
    portnumber = configMain.portNmbr;

    printf("\n>>>Config File Data<<<\n");
    printf("HostServerName: %s\n", hostname);
    printf("PortNumber: %d\n", portnumber);
    printf("GameKind: %s\n\n ", gamekindname);
    
    //From here: sockets
    int sock, client_fd;
    struct sockaddr_in serv_addr;
    struct hostent *server;

    sock = socket(AF_INET, SOCK_STREAM, 0);
    if (sock < 0) {
        perror("\nERROR: Socket creation error \n"); 
        return - 1;
    }

    //ipAdresse nachschauen
    server = gethostbyname(hostname);
    if (server == NULL)
    {
        perror("ERROR: no such host\n");
    } 

    memset(&serv_addr,0,sizeof(serv_addr));
    serv_addr.sin_family = AF_INET;
    serv_addr.sin_port = htons(portnumber);
    memcpy(&serv_addr.sin_addr.s_addr,server->h_addr,server->h_length);

    if ((client_fd = connect(sock, (struct sockaddr*) &serv_addr, sizeof(serv_addr))) < 0) {
        perror("ERROR: Connection Failed \n");
        return -1;
    }

    printf(">>> Mit Host : %s verbunden <<<\n", hostname);

    if(performConnection(sock, gameID) != 0) {
        perror("performConnection Failed\n");
    }        // TODO: verlagern

    close(client_fd);
    return 0;


    
    // Shared Memory Segment erstellen
    int shmid_game = shmget(KEY, sizeof(gameInfo), IPC_CREAT | SHM_R | SHM_W);
    if (shmid_game == -1) {
        perror("Error while creating shared memory segment");
        exit(EXIT_FAILURE);
    } else {
        printf("Creation successful\n");
    }

    int shmid_player = shmget(KEY, sizeof(playerInfo), IPC_CREAT | SHM_R | SHM_W);
    if (shmid_player == -1) {
        perror("Error while creating shared memory segment");
        exit(EXIT_FAILURE);
    } else {
            printf("Creation successful\n");
    }

    // Prozess mit SHM verbinden
    void* shm_game = shmat(shmid_game, 0, 0);
    if (shm_game == NULL) {
        perror("Error while attaching shared memory segment");
        exit(EXIT_FAILURE);
    } else {
        printf("Attachment successful\n");
    } 

    void* shm_player = shmat(shmid_player, 0, 0);
        if (shm_player == NULL) {
        perror("Error while attaching shared memory segment");
        exit(EXIT_FAILURE);
        } else {
            printf("Attachment successful\n");
        }
    

    // Kindprozess (Connector) erstellen

    pid_t pid;
    pid = fork();

    if (pid < 0) {
        fprintf(stderr, "Fehler bei Erstellung des Kindprozesses.\n");
    } else if (pid == 0) {  // Kindprozess (Connector)
        close(fd[1]);
        performConnection(sock, gameID);
    } else {    // Elternprozess (Thinker)
        close(fd[0]);
    }

    return 0;
}

Answer 1

recv() returns the number of bytes that it has written into your buffer -- which is to say, it returns the number of bytes that it currently has available to give to you at the time you called it. Importantly, that will often be less than the number of bytes you requested, so it is mandatory that you check the return value of recv() to find out how many bytes you actually received, and not just assume that the value returned by recv() is equal to sizeof(buffer).

OTOH if you want recv() to not return until sizeof(buffer) bytes have been successfully read, you can pass the MSG_WAITALL flag to recv() in the fourth argument.

From the recv man page:

This flag requests that the operation block until the full
request is satisfied.  However, the call may still return
less data than requested if a signal is caught, an error
or disconnect occurs, or the next data to be received is
of a different type than that returned.  This flag has no
effect for datagram sockets.

Answer 2

TCP is a stream-oriented protocol, not a message-oriented one. A message sent as 100 bytes can be received as 1 100-byte read or as 100 1-byte reads, or any combination in between. This means that you must keep looping on the recv() until you have the whole message. That, in turn, means you need to know when a message is completely received. Either prepend the message's length before the message, use a fixed-size message, or have a unique recognizable terminator at the end of the message.

Answer 3

It is not guaranteed that recv() will get all of the bytes sent at once. The convention is to call it in a loop until you've read all the bytes.

NB that recv() returns 0 when the client is stalling or closed the connection, and -1 on a read error.

Handling partial send()s:

int sendall(int s, char *buf, int *len)
{
    int total = 0;        // how many bytes we've sent
    int bytesleft = *len; // how many we have left to send
    int n;

    while(total < *len) {
        n = send(s, buf+total, bytesleft, 0);
        if (n == -1) { break; }
        total += n;
        bytesleft -= n;
    }

    *len = total; // return number actually sent here

    return n==-1?-1:0; // return -1 on failure, 0 on success
} 

— From Beej's guide to Network Programming

---

The above code snippet calls send() in a loop until all the data has been sent.

You can now write a similar recv_all function that calls recv() in a loop until it has read all the data.

Handling partial recv()s:

Perhaps something like this:

/*  Synopsis: Calls recv() in a loop to ensure 
*   len bytes have been read. Stores 
*   the total number of bytes sent in len.
*
*   Returns:  0 on failure, 1 otherwise.
*/
static int recv_all(int sockfd, char *buf, size_t *len)
{
    size_t bytes_left = *len;
    size_t total = 0;
    ssize_t rv = 0;
    errno = 0;

    while (total < *len) {
          rv = recv(sockfd, buf + total, bytes_left, 0);
          if (rv == 0) {         /* Client closed the connection or is stalling */             
               return 0;
          } else if (rv == -1) { /* A read error */
               perror("recv");
               return 0;
          } 
          total += rv;
          bytes_left -= rv;
    }
   *len = total;
    return 1;
}

recv() may also return 0 when 0 characters were read. In that case, len can be compared against the original len to see if the call to recv() was successful.

Note: The above recv_all function has not been tested, and hence is not guaranteed to be bug free. It's meant to be an example.