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 possible – fd
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);