Simple echo program using sockets in C echoing incorrect message after the first run

Question

I am trying to learn the basic of network communication using sockets in C. My client program takes in a message from the user, echoes it server side and back, and prints out the received message. When I fire both of them up for the first time, they both work exactly as expected. However, if I quit the client side and then fire it up again while keeping the server program running, my echoed messages become off by one.

I assumed it was because the last message is getting caught in the pipe or something, and after poking around, I saw that someone suggested to use shutdown() to flush out the pipe, but that doesn't seem to be working. I also tried to zero out the buffers wherever I thought they may be lingering, but that didn't seem to help, either.

server.c

#include <stdio.h>
#include <errno.h>
#include <sys/socket.h>
#include <resolv.h>
#include <arpa/inet.h>
#include <errno.h>
#include <stdlib.h>
#include <strings.h>
#include <unistd.h>

#define PORT            12403
#define BUFFER_MAX      1024
#define BACKLOG_MAX     1024

int clientSocket;
int serverSocket;

void listening()
{
    while (1)
    {
        struct sockaddr_in clientAddress;
        socklen_t addressLength = sizeof(clientAddress);

        /*---accept a connection (creating a data pipe)---*/
        clientSocket = accept(serverSocket, (struct sockaddr*)&clientAddress, &addressLength);
        if (clientSocket > -1)
        {
            printf("%s:%d connected\n", inet_ntoa(clientAddress.sin_addr), ntohs(clientAddress.sin_port));
            break;
        }
    }
}

int main(int Count, char *Strings[])
{   
    struct sockaddr_in socketInfo;
    char buffer[BUFFER_MAX];

    //Create socket
    if ((serverSocket = socket(AF_INET, SOCK_STREAM, 0)) < 0)
    {
        perror("Error creating socket");
        exit(errno);
    }

    //Setting the linger option to off and resuse address option to on for testing
    int option = 0;
    setsockopt(serverSocket, SOL_SOCKET, SO_LINGER, &option, sizeof(option));
    option = 1;
    setsockopt(serverSocket, SOL_SOCKET, SO_REUSEADDR, &option, sizeof(option));

    //Initialize socket information
    bzero(&socketInfo, sizeof(socketInfo));
    socketInfo.sin_family = AF_INET;
    socketInfo.sin_port = htons(PORT);
    socketInfo.sin_addr.s_addr = INADDR_ANY;

    //Assign a port number to the socket
    if (bind(serverSocket, (struct sockaddr*)&socketInfo, sizeof(socketInfo)) != 0)
    {
        perror("Error binding socket");
        exit(errno);
    }

    //Set socket to listen
    if (listen(serverSocket, BACKLOG_MAX) != 0)
    {
        perror("Error setting socket to listen");
        exit(errno);
    }

    listening();

    //Once first socket has been connected, begin echoing process
    int i = 0;
    while (1)
    {
        //Clear the buffer
        bzero(buffer, BUFFER_MAX);

        //Echo back anything sent
        //Close connection and begin listening process again if the client disconnects
        int sendCheck;
        int readCheck;
        readCheck = recv(clientSocket, buffer, BUFFER_MAX, 0);
        if (readCheck <= 0)
        {
            shutdown(clientSocket, SHUT_WR);
            close(clientSocket);
            sleep(1);
            listening();
        }
        sendCheck = send(clientSocket, buffer, BUFFER_MAX, 0);

        if (sendCheck <= 0)
        {
            shutdown(clientSocket, SHUT_WR);
            close(clientSocket);
            sleep(1);
            listening();
        }
        i++;
    }

    close(serverSocket);
    return 0;
}

client.c

#include <unistd.h>
#include <stdio.h>
#include <string.h>
#include <errno.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <arpa/inet.h>
#include <signal.h>
#include <stdlib.h>
#include <resolv.h>
#include <netdb.h>

#define PORT            12403
#define LOCALHOST       "127.0.0.1"
#define BUFFER_MAX      1024

int socketStatus = 0;

void sigpipeHandler()
{
    perror("Connection to server terminated\n");
    socketStatus = 0;
}

int main()
{
    int mySocket;
    struct sockaddr_in socketInfo;
    char buffer[BUFFER_MAX];

    int count = 0;

    //Create socket
    if ((mySocket = socket(AF_INET, SOCK_STREAM, 0)) < 0)
    {
        perror("Error creating socket");
        exit(errno);
    }

    //Get IP address of required host machine
    char* hostName = "<host name removed>";
    int portNumber = PORT;
    char* ipAddr = NULL;
    struct hostent* host = NULL;
    host = gethostbyname(hostName);
    ipAddr = inet_ntoa(*((struct in_addr*) host->h_addr_list[0]));

    //Initialize server information
    bzero(&socketInfo, sizeof(socketInfo));
    socketInfo.sin_family = AF_INET;
    socketInfo.sin_port = htons(portNumber);
    if (inet_aton(ipAddr, (struct in_addr *)&socketInfo.sin_addr.s_addr) == 0)
    {
        perror("Error assigning IP address");
        exit(errno);
    }

    //Set up sigpipe handler
    signal(SIGPIPE, sigpipeHandler);

    //Connect to server
    if (connect(mySocket, (struct sockaddr*)&socketInfo, sizeof(socketInfo)) != 0)
    {
        perror("Error connecting");
        exit(errno);
    }

    //Indicate that socket is OK
    socketStatus = 1;

    while(1)
    {
        if(!socketStatus) {shutdown(mySocket, SHUT_WR); break;}
        printf("Please enter a command.\n");
        char command[BUFFER_MAX];
        bzero(command, BUFFER_MAX);
        fgets(command, sizeof(command), stdin);

        send(mySocket, command, BUFFER_MAX, 0);

        //Get echoed message
        bzero(buffer, BUFFER_MAX);
        recv(mySocket, buffer, sizeof(buffer), 0);
        printf("Echo [%d]:%s\n", ++count, buffer);
    }

    //Close socket
    close(mySocket);
    return 0;
}

Answer 1

I did some cleanup on your server code and this seems to work.

For my testing, the client code is unchanged. But, as others have suggested, you should check the error codes from send and recv. Also, note that if you ctrl-c the server, the client will hang in the fgets, so it won't detect the server abort until you hit return after the prompt. Not a big deal, but I thought I'd mention it.

I also added a fork so you can have multiple clients talking to the same server instance simultaneously.

I tested this with two clients [in two xterm windows] talking with the single server instance.

I moved your echo code into a new function docomm. A small difference from your code is that any error from either recv or send breaks out of the loop and closes the connection. All connections from new clients are guaranteed to start with a recv call.

In your code, you would not always break out of the loop, but close the connection and call listening again. This would happen for either send or recv. If it happened on the wrong one, this might be the source of the problem you were having because you could do a send before a recv to a new client initially.

---

#include <stdio.h>
#include <errno.h>
#include <sys/socket.h>
#include <resolv.h>
#include <arpa/inet.h>
#include <errno.h>
#include <stdlib.h>
#include <strings.h>
#include <unistd.h>
#include <sys/wait.h>

#define PORT            12403
#define BUFFER_MAX      1024
#define BACKLOG_MAX     1024

int clientSocket;
int serverSocket;
int forkflg = 1;

void listening()
{
    while (1)
    {
        struct sockaddr_in clientAddress;
        socklen_t addressLength = sizeof(clientAddress);

        /*---accept a connection (creating a data pipe)---*/
        clientSocket = accept(serverSocket, (struct sockaddr*)&clientAddress, &addressLength);
        if (clientSocket > -1)
        {
            printf("%s:%d connected\n", inet_ntoa(clientAddress.sin_addr), ntohs(clientAddress.sin_port));
            break;
        }
    }
}

void
docomm(void)
{
    char buffer[BUFFER_MAX];

    //Once first socket has been connected, begin echoing process
    int i = 0;
    while (1) {
        //Clear the buffer
        bzero(buffer, BUFFER_MAX);

        //Echo back anything sent
        //Close connection and begin listening process again if the client disconnects
        int sendCheck;
        int readCheck;

        readCheck = recv(clientSocket, buffer, BUFFER_MAX, 0);
        if (readCheck <= 0)
            break;

        sendCheck = send(clientSocket, buffer, BUFFER_MAX, 0);
        if (sendCheck <= 0)
            break;

        i++;
    }

    printf("close\n");
    shutdown(clientSocket, SHUT_WR);
    close(clientSocket);
}

int main(int Count, char *Strings[])
{
    struct sockaddr_in socketInfo;

    //Create socket
    if ((serverSocket = socket(AF_INET, SOCK_STREAM, 0)) < 0)
    {
        perror("Error creating socket");
        exit(errno);
    }

    //Setting the linger option to off and resuse address option to on for testing
    int option = 0;
    setsockopt(serverSocket, SOL_SOCKET, SO_LINGER, &option, sizeof(option));
    option = 1;
    setsockopt(serverSocket, SOL_SOCKET, SO_REUSEADDR, &option, sizeof(option));

    //Initialize socket information
    bzero(&socketInfo, sizeof(socketInfo));
    socketInfo.sin_family = AF_INET;
    socketInfo.sin_port = htons(PORT);
    socketInfo.sin_addr.s_addr = INADDR_ANY;

    //Assign a port number to the socket
    if (bind(serverSocket, (struct sockaddr*)&socketInfo, sizeof(socketInfo)) != 0)
    {
        perror("Error binding socket");
        exit(errno);
    }

    //Set socket to listen
    if (listen(serverSocket, BACKLOG_MAX) != 0)
    {
        perror("Error setting socket to listen");
        exit(errno);
    }

    while (1) {
        listening();

        if (! forkflg) {
            docomm();
            continue;
        }

        pid_t pid = fork();
        if (pid == 0) {
            docomm();
            exit(0);
        }

        while (waitpid(0,NULL,WNOHANG) > 0);
    }

    close(serverSocket);
    return 0;
}

---

UPDATE:

Just from a glance: 1) Can I ask why you created a fork flag if you never change the value of it? Should it be changed somewhere?

I used forkflg so you can set it to zero (e.g. int forkflg = 0;) to run sequentially. Or, you could add some code and parse argv looking for an option (e.g. -f) to set/clear it [for testing/debug purposes]. For production code, you'd want forkflg to be set and could remove the flag and just do the fork case always [adjusting the code to match].

Just tracing through the program mentally, it seems like the forking section will never be executed. Correct me where I'm wrong: after initially setting the socket to listen, the while loop will enter, and listening() will be called. Execution will halt in listening() until a connection is accepted.

Yes, that's true.

Control will return to main, where docomm() gets called. Control stays in docomm() until the connection breaks, at which point it returns to main and continue gets called, skipping the fork stuff and starting the process over again. So does the fork stuff ever get executed?

What you're describing is the behavior if forkflg is zero.

The fork is called if forkflg is set. Note that, in that case, docomm is called in the child and not the parent (because fork returned 0). So, the parent will not be blocked while the child does the echoing.

Thus, the parent returns immediately and is free to do the waitpid loop to reap any old children and restart the main/outer loop.

The waitpid loop only happens when a new connection comes in, so several children may have already terminated and will stay in zombie state until the waitpid loop gets executed [which will reap any/multiple pending children].

A cleaner way to reap the children might be to set up a signal handler for SIGCHLD and have it do the waitpid loop. This would reap all spent children immediately, without having to wait for a new connection to roll in.

Or, with the signal handler, add the waitpid loop to listening [inside the current loop] because if a SIGCHLD signal comes in, accept will return immediately with errno set to EINTR