Skip to content
Advertisement

accept(…) seems to be modifying the file descriptor parameter I give it

Here’s a section of my code, for a simple echo server I’m attempting to write using the linux sockets syscalls:

while (true) {
    std::cout << "Fd before accept: " << fd << std::endl;
    if ((current_socket = accept(fd, (struct sockaddr*)&addr, (socklen_t*)&addrlen)) < 0) {
        std::perror("accept");
        exit(EXIT_FAILURE);
    }
    std::cout << "Accepted (fd) = " << fd << std::endl;

    amount_read = read(current_socket, in_buf, 1024);
    std::cout << "Amount read: " << amount_read << std::endl;
    std::cout << "In from client: " << in_buf << std::endl;
    std::cout << "What would you like to say back: ";
    std::getline(std::cin, out_buf);
    send(current_socket, out_buf.c_str(), out_buf.length(), 0);
    std::cout << "Sent." << std::endl;

    std::cout << "fd before looping = " << fd << std::endl;
}

Just before this piece of code, I set up a socket descriptor fd, and then, immediately before the while loop begins:

listen(fd, 4);

I compile this and then run it, and then run a client program which I’ve written to send a message ("Hello from the client") to the server.

My problem

Anyway, my problem with this is that, as the title suggests, the call to accept(...) is modifying the value of fd. First of all, I can’t understand how this is possiblefd isn’t being passed as a pointer, so how could it possibly be modified by a function call? But this point is mainly just of interest – the main implication of this problem is described in the next section.

I’m certain the value of fd has been changed because here’s some sample output from the server:

Attempting to listen
Listening
Fd before accept: 3
Accepted (fd) = 0
Amount read: 17
In from client: Hello from client
What would you like to say back: ?
Sent.
fd before looping = 0
Fd before accept: 0
accept: Socket operation on non-socket

So as you can see, fd before the call to accept is 3, but then after the call it becomes 0.

Why this is a problem

Referring back to the output from the server, as shown above:

accept: Socket operation on non-socket

Which clearly is as a result of the new value of fd not being a valid socket.

What I’ve tried

Enveloping also the listen(...) call in the while(true) {...} loop, just in case I need to listen again for each client. I doubted this would work, and it didn’t. I’m all out of ideas.

Other questions which haven’t helped

I found a question somewhere asking why accept(...) was returning (not changing the fd parameter) a value of 0. I understand from that that 0 is a valid socket descriptor, but clearly here it isn’t. Also, this just shouldn’t be happening, right?

My question

Just to sum up: why is accept(...) modifying one of its non-pointer parameters, and how can I fix this?

Full code

#include <unistd.h>
#include <cstdio>
#include <cstdlib>
#include <string>
#include <cstring>
#include <cstdint>
#include <iostream>

#include <sys/socket.h>
#include <netinet/in.h>

int create_socket(uint16_t port, struct sockaddr_in* addr) {
    int server_fd;
    int opt = 1;

    if ((server_fd = socket(AF_INET, SOCK_STREAM, 0)) == 0) {
        std::perror("Socket create failed.");
        exit(EXIT_FAILURE);
    }

    if (setsockopt(server_fd, SOL_SOCKET, SO_REUSEADDR | SO_REUSEPORT, &opt, sizeof(opt))) {
        std::perror("setsockopt failed");
        exit(EXIT_FAILURE);
    }

    addr->sin_family = AF_INET;
    addr->sin_addr.s_addr = INADDR_ANY;
    addr->sin_port = htons(port);

    // now bind it to a port
    if (bind(server_fd, (struct sockaddr*)addr, sizeof(*addr)) < 0) {
        std::perror("bind failed");
    }

    return server_fd;
}

int serve(int fd, struct sockaddr_in* addr, int backlog=4) {
    std::string to_send;
    int addrlen = sizeof(*addr), amount_read;
    int current_socket;
    char in_buf[1024];
    std::string out_buf;

    std::cout << "Attempting to listen" << std::endl;
    if (listen(fd, backlog) < 0) {
        std::perror("listen failed");
        exit(EXIT_FAILURE);
    }
    std::cout << "Listening" << std::endl;

    while (true) {
        std::cout << "Fd before accept: " << fd << std::endl;
        if ((current_socket = accept(fd, (struct sockaddr*)&addr, (socklen_t*)&addrlen)) < 0) {
            std::perror("accept");
            exit(EXIT_FAILURE);
        }
        std::cout << "Accepted (fd) = " << fd << std::endl;

        amount_read = read(current_socket, in_buf, 1024);
        std::cout << "Amount read: " << amount_read << std::endl;
        std::cout << "In from client: " << in_buf << std::endl;
        std::cout << "What would you like to say back: ";
        std::getline(std::cin, out_buf);
        send(current_socket, out_buf.c_str(), out_buf.length(), 0);
        std::cout << "Sent." << std::endl;

        std::cout << "fd before looping = " << fd << std::endl;
    }

    return 0;
}

int main(int argc, char* argv[]) {
    struct sockaddr_in addr;
    int fd = create_socket(5555, &addr);

    serve(fd, &addr);

    close(fd);
}

Advertisement

Answer

Your code has undefined behavior, as expected.

int addrlen = sizeof(*addr), amount_read;
...
accept(fd, (struct sockaddr*)&addr, (socklen_t*)&addrlen));

First off, you are taking address of addr, which is already a pointer. You should not do this.

Also, my little crystal ball tells me you are on 64 bit platform, and as such, your socklen_t is a 64bit integer.

To fix your problems, do not take address of the pointer and do away with nasty cast and use the proper type.

socklen_t addrlen = sizeof(*addr);
...
accept(fd, (struct sockaddr*)addr, &addrlen);
User contributions licensed under: CC BY-SA
5 People found this is helpful
Advertisement