pasqualerossi / 42-School-Exam-Rank-06

Last 42 School Exam
42 stars 8 forks source link

Possible select read fd set fix + code passing current grademe.fr #11

Open jwikiera opened 2 months ago

jwikiera commented 2 months ago

Hello, I have been compiling this repo's solution and variations of it, since some time, on various linuxes and compilers. Consistently, I get the situation where after a couple compiles and program starts, netcat disconnects as soon as I try to send a message with it. I heard (but can't find a source for it), that it's bad practice to pass in write ready descriptors to select if we actually don't want to write to them, especially in a loop. In my implementation, I add the current fds to write_set only at the beginning of the send_to_allfunction, before rerunning a select on them. At function exit, I FD_ZEROthe write set.

Here is the full code which currently passes the grademe.fr test exam:

Code ```c #include #include #include #include #include #include int serverfd; typedef struct s_client { int id; char msg[300000]; } t_client; t_client clients[1024]; fd_set read_set, write_set, current; int maxfd = 0, gid = 0; char send_buffer[300000], recv_buffer[300000]; void err(char *msg) { if (msg) write(2, msg, strlen(msg)); else write(2, "Fatal error", 11); write(2, "\n", 1); exit(1); } void send_to_all(int except) { write_set = current; if (select(maxfd + 1, &read_set, &write_set, 0, 0) == -1) { FD_ZERO(&write_set); bzero(send_buffer, sizeof(send_buffer)); return; } for (int fd=0; fd <= maxfd; fd ++) { if (fd != except && FD_ISSET(fd, &write_set) && fd != serverfd) { if (send(fd, send_buffer, strlen(send_buffer), 0) == -1) { err(NULL); } } } FD_ZERO(&write_set); bzero(send_buffer, sizeof(send_buffer)); } int main(int ac, char **av) { if (ac != 2) err("Wrong number of arguments"); struct sockaddr_in serveraddr; socklen_t len; int serverfd = socket(AF_INET, SOCK_STREAM, 0); if (serverfd == -1) err(NULL); maxfd = serverfd; FD_ZERO(¤t); FD_ZERO(&write_set); FD_SET(serverfd, ¤t); bzero(clients, sizeof(clients)); bzero(&serveraddr, sizeof(serveraddr)); bzero(recv_buffer, sizeof(recv_buffer)); bzero(send_buffer, sizeof(send_buffer)); serveraddr.sin_family = AF_INET; serveraddr.sin_addr.s_addr = htonl(INADDR_ANY); serveraddr.sin_port = htons(atoi(av[1])); if (bind(serverfd, (const struct sockaddr *)&serveraddr, sizeof(serveraddr)) == -1 || listen(serverfd, 100) == -1) err(NULL); while (1) { read_set = current; if (select(maxfd + 1, &read_set, &write_set, 0, 0) == -1) continue; for (int fd = 0; fd <= maxfd; fd++) { if (FD_ISSET(fd, &read_set)) { if (fd == serverfd) { int clientfd = accept(serverfd, (struct sockaddr *)&serveraddr, &len); if (clientfd == -1) continue; if (clientfd > maxfd) maxfd = clientfd; clients[clientfd].id = gid++; FD_SET(clientfd, ¤t); sprintf(send_buffer, "server: client %d just arrived\n", clients[clientfd].id); send_to_all(clientfd); } else { int ret = recv(fd, recv_buffer, sizeof(recv_buffer), 0); if (ret <= 0) { sprintf(send_buffer, "server: client %d just left\n", clients[fd].id); send_to_all(fd); FD_CLR(fd, ¤t); close(fd); bzero(clients[fd].msg, strlen(clients[fd].msg)); } else { for (int i = 0, j = strlen(clients[fd].msg); i < ret; i++, j++) { clients[fd].msg[j] = recv_buffer[i]; if (clients[fd].msg[j] == '\n') { clients[fd].msg[j] = '\0'; sprintf(send_buffer, "client %d: %s\n", clients[fd].id, clients[fd].msg); send_to_all(fd); bzero(clients[fd].msg, strlen(clients[fd].msg)); j = -1; } } } } break; } } } return (0); } ```
jwikiera commented 2 months ago

Oh well, it sometimes passes grademe on linux sometimes not. I had the chance to try on mac and select seems to behave differently, the solution of the repo works.

CarloCattano commented 1 month ago

just by changing if (fd != except && FD_ISSET(fd, &write_set) && fd != serverfd) { this if statement with the serverfd global check worked for me , thanks for the idea. Despite the int declared twice global int serverfd, and int serverfd in main, which I removed.

I also didn't add the select on the send_to_all and It passed the grademe, not sure about the actual exam. the current solution in this repo does not pass grademe for me but it does with this changes

pasqualerossi commented 1 month ago

create a pull request and I’ll update the code.