On 10/25/22 20:18, Paul Gofman wrote:
On 10/25/22 19:04, Zebediah Figura (@zfigura) wrote:
Zebediah Figura (@zfigura) commented about server/sock.c:
+static struct rb_tree bound_addresses_tree = { addr_compare };
+static int is_blocking_addr( struct sock *sock, const union unix_sockaddr *addr ) +{ + if (!is_tcp_socket( sock )) return 0;
+ if (sock->family == WS_AF_INET) + { + if (addr->addr.sa_family != AF_INET) return 0; + if (!addr->in.sin_port) return 0; + return 1; + } + if (addr->addr.sa_family != AF_INET6) return 0; + if (!addr->in6.sin6_port) return 0; + return 1;
I think it'd be clearer if you had an "else if (sock->family == WS_AF_INET6)" branch, even though it's more lines of code.
Also, do we really need those sa_family checks? It skips the address lookup, but that doesn't seem necessary, and it's a bit distracting.
The check is here because in theory an app may supply the wrong address type for socket. In this case, if I don't skip the lookup here and happen to find the actually supplied address already bound we will return the wrong error code (that should fail in native bind() with a different error). I don't think such specific and weird condition warrants any code complication, but it also looks weird to search for a non matching address type for socket, so maybe we could keep the check in place?
Oh, right, I can't think. I guess in that case the question is the opposite—can we just check sa_family here and *not* sock->family?