Alexander Sulfrian <alexander@xxxxxxxxxxxx> writes:
> @@ -861,11 +862,20 @@ static int setup_named_sock(char *listen_addr, int listen_port, int **socklist_p
>
> #endif
>
> -static int socksetup(char *listen_addr, int listen_port, int **socklist_p)
> +static int socksetup(struct string_list *listen_addr, int listen_port, int **socklist_p)
> {
> int socknum = 0, *socklist = NULL;
>
> - socknum = setup_named_sock(listen_addr, listen_port, &socklist, socknum);
> + if (!listen_addr->nr)
> + socknum = setup_named_sock(NULL, listen_port, &socklist,
> + socknum);
> + else {
> + int i;
> + for (i = 0; i < listen_addr->nr; i++)
> + socknum = setup_named_sock(listen_addr->items[i].string,
> + listen_port, &socklist,
> + socknum);
> + }
>
> *socklist_p = socklist;
> return socknum;
Giving an old number and returning a new number feels a bit awkward as an
API. If you create a structure that consists of a <pointer, nr, alloc>
tuple that is suitable for ALLOC_GROW() API and pass that around instead
of <&socklist, socknum> pair, then the helper can return how many sockets
it prepared, and signal a failure with a negative value, no?
> +static int serve(struct string_list *listen_addr, int listen_port, struct passwd *pass, gid_t gid)
> {
> int socknum, *socklist;
>
> socknum = socksetup(listen_addr, listen_port, &socklist);
> if (socknum == 0)
> - die("unable to allocate any listen sockets on host %s port %u",
> - listen_addr, listen_port);
> + die("unable to allocate any listen sockets on port %u",
> + listen_port);
The old code accepted only one --listen, so it was clear when no socket
can be prepared for a given name (which may expand to multiple addresses),
it is clear what failed (i.e. the failing input could have been only _one_
from the user's point of view). This check diagnoses the case where
socket preparation failed for all names. Don't we want to have a new
check inside the iteration over names in socksetup() to warn when socket
preparation for all addresses for a name fails?
Also doesn't setup_named_sock() as refactored die() when one of the names
given to --listen results no socket under ipv6 build but keeps going under
noipv6 build? Without multi-listen, the distinction did not matter
exactly because it took only one name, but your multi-listen addition
exposes this inconsistency. I think the helper should be modified not to
die but just return "I didn't prepare any socket for the given name", to
allow the outer loop to continue on to the next name (perhaps while
issuing a warning e.g. "No listening socket resulted for '%s'".
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html