Matthew John Cheetham via GitGitGadget wrote:
> From: Matthew John Cheetham <mjcheetham@xxxxxxxxxxx>
>
> Extract functions for setting up listening sockets and keep-alive options
> from `daemon.c` to new `daemon-utils.{c,h}` files. Remove direct
> dependencies on global state by inlining the behaviour at the callsites
> for all libified functions.
Thanks for making this change, the reduced code duplication should make the
common daemon-related code more maintainable.
For reference, I used
'git blame -s -b -C -C -C master..<this patch> -- daemon-utils.c'
to help identify which lines in 'daemon-utils.c' were changed from their
original implementation in 'daemon.c'. I'll try to rearrange the diff to
show those differences more directly.
The first main change I see is that 'logerror' and 'reuseaddr' are changed
from global references to arguments in 'set_keep_alive()',
'setup_named_sock()' (same for 'NO_IPV6' defined and undefined), and
'socksetup()':
> -static void set_keep_alive(int sockfd)
> +void set_keep_alive(int sockfd, log_fn logerror)
> -static int setup_named_sock(char *listen_addr, int listen_port, struct socketlist *socklist)
> +static int setup_named_sock(char *listen_addr, int listen_port,
> + struct socketlist *socklist, int reuseaddr,
> + log_fn logerror)
> -static void socksetup(struct string_list *listen_addr, int listen_port, struct socketlist *socklist)
> +void socksetup(struct string_list *listen_addr, int listen_port,
> + struct socketlist *socklist, int reuseaddr,
> + log_fn logerror)
The external calls in 'daemon.c' to 'set_keep_alive()' and 'socksetup()' are
updated to pass the 'logerror()' function and global 'reuseaddr' as
arguments, so there isn't any change in behavior.
> @@ -759,7 +748,7 @@ static int execute(void)
> if (addr)
> loginfo("Connection from %s:%s", addr, port);
>
> - set_keep_alive(0);
> + set_keep_alive(0, logerror);
> alarm(init_timeout ? init_timeout : timeout);
> pktlen = packet_read(0, packet_buffer, sizeof(packet_buffer), 0);
> alarm(0);
> @@ -1246,7 +1039,8 @@ static int serve(struct string_list *listen_addr, int listen_port,
> {
> struct socketlist socklist = { NULL, 0, 0 };
>
> - socksetup(listen_addr, listen_port, &socklist);
> + socksetup(listen_addr, listen_port, &socklist, reuseaddr,
> + logerror);
> if (socklist.nr == 0)
> die("unable to allocate any listen sockets on port %u",
> listen_port);
The other notable change is moving the 'if (!reusaddr) return 0' block in
'set_reuse_addr()' to its callers in both 'setup_named_sock()'s:
> +#ifndef NO_IPV6
> +
> +static int setup_named_sock(char *listen_addr, int listen_port,
> + struct socketlist *socklist, int reuseaddr,
> + log_fn logerror)
> +{
...
> + if (reuseaddr && set_reuse_addr(sockfd)) {
> + logerror("Could not set SO_REUSEADDR: %s", strerror(errno));
> + close(sockfd);
> + continue;
> + }
...
> +}
> +
> +#else /* NO_IPV6 */
> +
> +static int setup_named_sock(char *listen_addr, int listen_port,
> + struct socketlist *socklist, int reuseaddr,
> + log_fn logerror)
> +{
...
> + if (reuseaddr && set_reuse_addr(sockfd)) {
> + logerror("Could not set SO_REUSEADDR: %s", strerror(errno));
> + close(sockfd);
> + return 0;
> + }
...
> +}
> +
> +#endif
Where, previously, that region looked like:
> -#ifndef NO_IPV6
> -
> -static int setup_named_sock(char *listen_addr, int listen_port, struct socketlist *socklist)
> -{
...
> - if (set_reuse_addr(sockfd)) {
> - logerror("Could not set SO_REUSEADDR: %s", strerror(errno));
> - close(sockfd);
> - continue;
> - }
...
> -}
> -
> -#else /* NO_IPV6 */
> -
> -static int setup_named_sock(char *listen_addr, int listen_port, struct socketlist *socklist)
> -{
...
> - if (set_reuse_addr(sockfd)) {
> - logerror("Could not set SO_REUSEADDR: %s", strerror(errno));
> - close(sockfd);
> - return 0;
> - }
...
> -}
> -
> -#endif
'reuseaddr' is passed into 'setup_named_sock()' from 'socksetup()' calls in
'daemon.c', so this also won't result in changed behavior.
Otherwise, you only expose functions & types that are called in 'daemon.c'
(the rest are still static), and everything else is a verbatim copy. Looks
good to me!
