0

Here's my simple server and client.

server.cc

#include <sys/types.h>
#include <sys/socket.h>
#include <netdb.h>
#include <arpa/inet.h>

#include <iostream>

struct TcpConnection {
    int client_id;
    struct sockaddr_in client_addr;
    socklen_t  addr_len;

    ~TcpConnection() {
        shutdown(client_id, SHUT_RDWR);
    }
};

int main()
{
    // Creates an endpoint for communication
    int server_id = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
    if (server_id < 0) {
        perror("socket");
        exit(errno);
    }

    // Assigning a name to a socket
    struct sockaddr_in addr;
    addr.sin_family = AF_INET;
    // address to accept any incoming messages
    addr.sin_addr.s_addr = INADDR_ANY;
    addr.sin_port = htons(9219);

    int yes = 1;
    setsockopt(server_id, SOL_SOCKET, SO_REUSEADDR, &yes, sizeof(yes));

    if (bind(server_id, (struct sockaddr *)(&addr), sizeof(addr)) < 0) {
        perror("bind");
        exit(errno);
    }

    if (listen(server_id, 5) < 0) {
        perror("listen");
        exit(errno);
    }

    while (1) {
        // Accept a connection on a socket
        TcpConnection * conn = new TcpConnection;
        conn->client_id = accept(server_id, 
                (struct sockaddr *)(&conn->client_addr), &(conn->addr_len));

        if (conn->client_id < 0) {
                perror("accept");
                exit(errno);
        } else {
            char ip[16];
            inet_ntop(AF_INET, &(conn->client_addr.sin_addr), ip, 16);
            std::cout << " Received a connection from " << ip << ", port " << conn->client_addr.sin_port << std::endl;

        }

        delete conn;
    }
    return 0;
}

client.cc

#include <sys/types.h>
#include <sys/socket.h>
#include <netdb.h>
#include <arpa/inet.h>

#include <iostream>

std::string get_ip_addr(struct addrinfo* host)
{
    struct sockaddr_in* addr = (struct sockaddr_in *)host->ai_addr;
    char ip[16];
    inet_ntop(AF_INET, &addr->sin_addr, ip, 16);
    return std::string(ip);
}


int main(int argc, char* argv[])
{
    if (argc != 3) {
        std::cout << "Usage: "<< argv[0] << " <server name> <port>\n";
        exit(errno);
    }

    // Create an endpoint for communication
    int client_id = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);

    // Get the information of server to be connected
    struct addrinfo * server_ai;

    int rv = getaddrinfo(argv[1], argv[2], 0, &server_ai);
    if ( rv < 0 ) {
        std::cout << gai_strerror(rv) << std::endl;
        exit(errno);
    }

    std::cout << "Connecting to " << argv[1] << 
        "|" << get_ip_addr(server_ai) << "|:" << argv[2] << "... ";
    std::cout.flush();

    // Connect to server
    if ( connect(client_id, server_ai->ai_addr, sizeof(struct sockaddr)) < 0 ) { 
        perror("connect");
        exit(errno);
    }

    std::cout << "connected" << std::endl;

    freeaddrinfo(server_ai);
}

After lauching the server program, I fire up a client to connect to this server. The server outputs:

Received a connection from 0.0.0.0, port 0

So confusing! And then I fire up a client again, the server outputs:

Received a connection from 127.0.0.1, port 15566

This time I get the expected result. So how to explain the first output?

wintr
  • 53
  • 12

2 Answers2

3

You do not initialize conn->addr_len before calling accept. As your struct TcpConnection contains a destructor, I suppose its members are set to 0 at initialization.

The third parameter of accept is an in/out parameter : according to the man page: the addrlen argument is a value-result argument; it should initially contain the amount of space pointed to by addr; on return it will contain the actual length (in bytes) of the address returned.

If it is initialized to 0, it means that you reserved no memory and nothing is written to conn->client_addr. You should use a constructor to initialize it explicitely:

TcpConnection(): client_id(0) {
    addr_len = sizeof(client_addr);
}
Serge Ballesta
  • 143,923
  • 11
  • 122
  • 252
1

You never initialize TcpConnection::addr_len, so its value will be random, when it shouldn't be. If its value is less than the length of sockaddr_in, you won't get all the information.

In your case, addr_len is set to 0 at first allocation by new so accept() will not fill the sockaddr_in. The call will set addr_len, and the next new inherit this correct value by chance (because you delete, remove delete and this won't work at all), so the next accept() will fill the address as expected. This is totally undefined behavior, it's just an explanation of what actually happens.

ElderBug
  • 5,926
  • 16
  • 25