TCP echo server / client in C, recv_all, send_all - implemented by me, recv does not work

Question

I want to write a simple client/server application, in which I will have fixed length of messages (BUFFER_SIZE). I want to be sure, that I send/recv ALL data.

Since send_all works great, I have a problem with recv_all. When I tested the program, when client sent a message to server, it got no response. What is wrong? Thank you.

client.c

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

#define BUFFER_SIZE 1024

void hostname_to_ip(char *hostname, char *ip)
{
    int sockfd;
    struct addrinfo hints, *servinfo;
    struct sockaddr_in *h;
    int rv;

    memset(&hints, 0, sizeof hints);
    hints.ai_family = AF_UNSPEC;
    hints.ai_socktype = SOCK_STREAM;

    if ( (rv = getaddrinfo(hostname, NULL, &hints, &servinfo)) != 0)
    {
        fprintf(stderr, "getaddrinfo: %s\n", gai_strerror(rv));
        exit(1);
    }

    h = (struct sockaddr_in *) servinfo->ai_addr;
    strcpy(ip, inet_ntoa( h->sin_addr ));

    freeaddrinfo(servinfo);
}

ssize_t send_all(int sockfd, const char *buf)
{
    ssize_t total_bytes = 0;
    ssize_t bytes = 0;
    size_t len = BUFFER_SIZE;

    while (len > 0)
    {
        bytes = send(sockfd, buf + total_bytes, len, 0);
        if (bytes == -1)
            break;
        total_bytes += bytes;
        len -= bytes;
    }

    return total_bytes;
}

int recv_all(int sockfd, char *buf)
{
    size_t len = BUFFER_SIZE;
    char *p = buf;
    ssize_t n, total_bytes = 0;

    while (len > 0 && (n = recv(sockfd, p, len, 0)) > 0)
    {
        p += n;
        len =- (size_t)n;
        total_bytes += n;
    }
    if ( len > 0 || n < 0 )
    {
        return -1;
    }
    return total_bytes;
}

int main(int argc, char **argv)
{
    char *hostname = "127.0.0.1";
    char ip_addr[BUFFER_SIZE];
    char buffer[BUFFER_SIZE+1];
    int port = 6666;
    char msg[BUFFER_SIZE];

    int sockfd, recv_size, err;
    struct sockaddr_in server_addr;

    if ((sockfd = socket(AF_INET, SOCK_STREAM, 0)) == -1)
    {
        perror("socket");
        exit(1);
    }

    hostname_to_ip(hostname, ip_addr);

    server_addr.sin_family = AF_INET;
    server_addr.sin_port = htons(port);
    inet_aton(ip_addr, &server_addr.sin_addr);

    if (connect(sockfd, (struct sockaddr *)&server_addr, sizeof(struct sockaddr)) == -1)
    {
        perror("connect");
        exit(1);
    }

    while (1)
    {
        memset(buffer, BUFFER_SIZE+1, '\0');

        printf("> ");
        fgets (msg, BUFFER_SIZE-1, stdin);


        err = send_all(sockfd, msg);

        if (err == 0)
        {
            perror("send");
            return 1;
        }

        if ((recv_size = recv(sockfd, buffer, BUFFER_SIZE, 0)) == 0)
        {
            close(sockfd);
            if (errno != 0)
            {
                perror("recv");
                exit(1);
            }
        }

        buffer[recv_size] = '\0';
        printf("FROM SERVER: %s\n", buffer);

    }

    close(sockfd);

    return 0;
}

server.c

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

#define BUFFER_SIZE 1024

ssize_t send_all(int sockfd, const char *buf)
{
    ssize_t total_bytes = 0;
    ssize_t bytes = 0;
    size_t len = BUFFER_SIZE;

    while (len > 0)
    {
        bytes = send(sockfd, buf + total_bytes, len, 0);
        if (bytes == -1)
            break;
        total_bytes += bytes;
        len -= bytes;
    }

    return total_bytes;
}

ssize_t recv_all(int sockfd, char * buf)
{
    size_t len = BUFFER_SIZE;
    char *p = buf;
    ssize_t total_bytes = 0, bytes = 0;

    while (len > 0 && (bytes = recv(sockfd, p, len, 0)) > 0)
    {
        p +=  bytes;
        len =- (size_t)bytes;
        total_bytes += bytes;
    }
    if ( len > 0 || bytes < 0 )
    {
        return -1;
    }
    return total_bytes;
}


int main()
{
    int port = 6666;
    int server_fd, client_fd, err, res;
    struct sockaddr_in server, client;
    char buffer[BUFFER_SIZE];
    memset(buffer, BUFFER_SIZE, '\0');

    server_fd = socket(AF_INET, SOCK_STREAM, 0);
    if (server_fd < 0)
    {
        printf("Could not create socket\n");
        perror("socket");
        return 1;
    }

    int optval = 1;
    setsockopt(server_fd, SOL_SOCKET, SO_REUSEADDR, (const void *)&optval, sizeof(int));

    server.sin_family = AF_INET;
    server.sin_port = htons(port);
    server.sin_addr.s_addr = htonl(INADDR_ANY);

    err = bind(server_fd, (struct sockaddr *) &server, sizeof(server));
    if (err < 0)
    {
        printf("Could not bind socket\n");
        perror("socket");
        return 1;
    }

    err = listen(server_fd, 128);
    if (err < 0)
    {
        printf("Could not listen on socket\n");
        perror("socket");
        return 1;
    }

    printf("Server TCP is listening on port %d ... \n", port);
    socklen_t client_len = sizeof(client);

    client_fd = accept(server_fd, (struct sockaddr *) &client, &client_len);

    if (client_fd < 0)
    {
        printf("Could not establish new connection\n");
        perror("socket");
        return 1;
    }

    while(1)
    {
        char IP[255];
        int remote_port;

        struct sockaddr_in *s = (struct sockaddr_in *)&client;
        remote_port = ntohs(s->sin_port);
        inet_ntop(AF_INET, &s->sin_addr, IP, sizeof(IP));

        printf("Client IP address: %s, port %d\n", IP, remote_port);

        int read;
        memset(buffer, BUFFER_SIZE, '\0');
        read = recv_all(client_fd, buffer);

        if (read < 0)
        {
            printf("Client read failed\n");
            perror("socket");
            return 1;
        }

        err = send_all(client_fd, buffer);

        if (err == 0)
        {
            printf("Client write failed\n");
            perror("socket");
            return 1;
        }

    }

    close(server_fd);
    close(client_fd);

    return 0;
}

Answer 1

There is a typo in recv_all() on both sides, you have the -= operator typed in backwards:

• client: len =- (size_t)n; needs to be len -= (size_t)n;

• server: len =- (size_t)bytes; needs to be len -= (size_t)bytes;

You are setting len to the negative of n/bytes, and since size_t is an unsigned type, that causes len to become a very large number, so if (len > 0) is always true, making recv() wait for more data that never arrives.

---

I also see a number of other issues.

On both sides:

• it would be better to make send_all() and recv_all() be ignorant of BUFFER_SIZE. Pass the desired len as an input parameter instead, and let the caller decide how many bytes to send/read. That will also allow you to simplify their logic.

• the return value of send_all() is not being validated correctly. The code assumes that only a return value of 0 is an error, but send_all() can return any value < BUFFER_SIZE on failure.

On the client side:

• hostname_to_ip() assumes that the returned IP (and it is ignoring the fact that there may be multiple output IPs) is always an AF_INET address, but it is using AF_UNSPEC for the query so the returned IP can be an AF_INET6 address instead, which won't work with an AF_INET socket.

• there is no point in converting a sockaddr_in to a char[] just to convert it back to a sockaddr_in. Use the original sockaddr_in data that getaddrinfo() returned.

• recv_all() is not being used at all. The client doesn't handle the case where recv() may fail with a return value < 0.

With that said, try something more like this instead:

client.c

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

#define BUFFER_SIZE 1024

socklen_t hostname_to_ip_port(char *hostname, int port, struct sockaddr_storage *addr)
{
    int sockfd;
    struct addrinfo hints, *servinfo;
    int rv;

    char service[20];
    sprintf(service, "%d", port);

    memset(&hints, 0, sizeof hints);
    hints.ai_family = AF_UNSPEC; // <-- if you really want only IPv4, this should be AF_INET !
    hints.ai_socktype = SOCK_STREAM;
    hints.ai_protocol = IPPROTO_TCP;

    if ( (rv = getaddrinfo(hostname, service, &hints, &servinfo)) != 0)
    {
        fprintf(stderr, "getaddrinfo: %s\n", gai_strerror(rv));
        return 0;
    }

    socklen_t addrlen = servinfo->ai_addrlen;
    memcpy(addr, servinfo->ai_addr, addrlen);

    freeaddrinfo(servinfo);

    return addrlen;
}

int send_all(int sockfd, const char *buf, size_t len)
{
    ssize_t n;

    while (len > 0)
    {
        n = send(sockfd, buf, len, 0);
        if (n < 0)
            return -1;
        buf += n;
        len -= n;
    }

    return 0;
}

int recv_all(int sockfd, char *buf, int len)
{
    ssize_t n;

    while (len > 0)
    {
        n = recv(sockfd, buf, len, 0);
        if (n <= 0)
            return n;
        buf += n;
        len -= n;
    }

    return 1;
}

int main(int argc, char **argv)
{
    char *hostname = "127.0.0.1";
    int port = 6666;
    char buffer[BUFFER_SIZE];

    int sockfd, err;
    struct sockaddr_storage server_addr;
    socklen_t server_addr_len;

    server_addr_len = hostname_to_ip_port(hostname, port, &server_addr);
    if (server_addr_len == 0)
        return 1;

    sockfd = socket(server_addr.ss_family, SOCK_STREAM, IPPROTO_TCP);
    if (sockfd < 0)
    {
        perror("Could not create socket");
        return 1;
    }

    if (connect(sockfd, (struct sockaddr *)&server_addr, server_addr_len) < 0)
    {
        perror("Could not connect socket");
        return 1;
    }

    while (1)
    {
        printf("> ");
        if (!fgets(buffer, BUFFER_SIZE, stdin))
            break;

        if (send_all(sockfd, buffer, BUFFER_SIZE) < 0)
        {
            perror("Could not send message");
            close(sockfd);
            return 1;
        }

        err = recv_all(sockfd, buffer, BUFFER_SIZE);
        if (err <= 0)
        {
            if (err < 0)
                perror("Could not read message");
            else
                printf("Server disconnected\n");
            break;
        }

        printf("FROM SERVER: %.*s\n", BUFFER_SIZE, buffer);
    }

    close(sockfd);

    return 0;
}

server.c

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

#define BUFFER_SIZE 1024

int send_all(int sockfd, const char *buf, int len)
{
    ssize_t n;

    while (len > 0)
    {
        n = send(sockfd, buf, len, 0);
        if (n < 0)
            return -1;
        buf += n;
        len -= n;
    }

    return 0;
}

int recv_all(int sockfd, char * buf, int len)
{
    ssize_t n;

    while (len > 0)
    {
        n = recv(sockfd, buf, len, 0);
        if (n <= 0)
            return n;
        buf += n;
        len -= n;
    }

    return 1;
}

int main()
{
    int port = 6666;
    int server_fd, client_fd, read;
    struct sockaddr_in server, client;
    char buffer[BUFFER_SIZE];
    char remote_ip[16];
    int remote_port;

    server_fd = socket(AF_INET, SOCK_STREAM, 0);
    if (server_fd < 0)
    {
        perror("Could not create socket");
        return 1;
    }

    int optval = 1;
    setsockopt(server_fd, SOL_SOCKET, SO_REUSEADDR, (const void *)&optval, sizeof(int));

    memset(&server, '\0', sizeof(server));
    server.sin_family = AF_INET;
    server.sin_port = htons(port);
    server.sin_addr.s_addr = htonl(INADDR_ANY);

    if (bind(server_fd, (struct sockaddr *) &server, sizeof(server)) < 0)
    {
        perror("Could not bind socket");
        close(server_fd);
        return 1;
    }

    if (listen(server_fd, 1) < 0)
    {
        perror("Could not listen on socket");
        close(server_fd);
        return 1;
    }

    printf("Server TCP is listening on port %d ... \n", port);

    socklen_t client_len = sizeof(client);
    client_fd = accept(server_fd, (struct sockaddr *) &client, &client_len);

    if (client_fd < 0)
    {
        perror("Could not establish new connection");
        close(server_fd);
        return 1;
    }

    remote_port = ntohs(client.sin_port);
    inet_ntop(AF_INET, &client.sin_addr, remote_ip, sizeof(remote_ip));

    printf("Client IP address: %s, port %d\n", remote_ip, remote_port);

    while (1)
    {
        read = recv_all(client_fd, buffer, BUFFER_SIZE);
        if (read <= 0)
        {
            if (read < 0)
                perror("Client read failed");
            else
                printf("Client disconnected\n");
            break;
        }

        printf("FROM CLIENT: %.*s\n", BUFFER_SIZE, buffer);

        if (send_all(client_fd, buffer, BUFFER_SIZE) < 0)
        {
            perror("Client write failed");
            break;
        }
    }

    close(client_fd);
    close(server_fd);

    return 0;
}