From d1fcc434c8afbc7c90696bd148a421f359c0d9b9 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 23 May 2017 21:47:34 +0930 Subject: [PATCH] subd: use array of fd pointers, not fds, and use take(). This lets us specify that we want to keep some fds. Signed-off-by: Rusty Russell --- lightningd/gossip_control.c | 2 +- lightningd/hsm_control.c | 2 +- lightningd/new_connection.c | 4 +--- lightningd/peer_control.c | 18 +++++++----------- lightningd/subd.c | 22 +++++++++++++--------- lightningd/subd.h | 2 +- 6 files changed, 24 insertions(+), 26 deletions(-) diff --git a/lightningd/gossip_control.c b/lightningd/gossip_control.c index f0e8524eb..3362c88bc 100644 --- a/lightningd/gossip_control.c +++ b/lightningd/gossip_control.c @@ -152,7 +152,7 @@ void gossip_init(struct lightningd *ld) u8 *init; ld->gossip = new_subd(ld, ld, "lightningd_gossip", NULL, gossip_wire_type_name, - gossip_msg, gossip_finished, -1); + gossip_msg, gossip_finished, NULL); if (!ld->gossip) err(1, "Could not subdaemon gossip"); diff --git a/lightningd/hsm_control.c b/lightningd/hsm_control.c index 5d021383b..67a914599 100644 --- a/lightningd/hsm_control.c +++ b/lightningd/hsm_control.c @@ -92,7 +92,7 @@ void hsm_init(struct lightningd *ld, bool newdir) ld->hsm = new_subd(ld, ld, "lightningd_hsm", NULL, hsm_wire_type_name, - hsm_msg, hsm_finished, -1); + hsm_msg, hsm_finished, NULL); if (!ld->hsm) err(1, "Could not subd hsm"); diff --git a/lightningd/new_connection.c b/lightningd/new_connection.c index afffa74ad..65d6401f5 100644 --- a/lightningd/new_connection.c +++ b/lightningd/new_connection.c @@ -181,7 +181,7 @@ static bool got_handshake_hsmfd(struct subd *hsm, const u8 *msg, "lightningd_handshake", NULL, handshake_wire_type_name, NULL, NULL, - fds[0], c->fd, -1); + take(&fds[0]), take(&c->fd), NULL); if (!handshaked) { log_unusual(ld->log, "Could not subdaemon handshake: %s", strerror(errno)); @@ -191,8 +191,6 @@ static bool got_handshake_hsmfd(struct subd *hsm, const u8 *msg, /* If handshake daemon fails, we just drop connection. */ tal_steal(handshaked, c); - /* We no longer own fd (closed; handshaked has copy). */ - c->fd = -1; if (c->known_id) { req = towire_handshake_initiator(c, &ld->dstate.id, c->known_id); diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index 5f981fbe0..ab57d2345 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -1502,8 +1502,9 @@ static bool peer_start_channeld_hsmfd(struct subd *hsm, const u8 *resp, channel_wire_type_name, channel_msg, peer_owner_finished, - peer->fd, - peer->gossip_client_fd, fds[0], -1); + take(&peer->fd), + take(&peer->gossip_client_fd), + take(&fds[0]), NULL); if (!peer->owner) { log_unusual(peer->log, "Could not subdaemon channel: %s", strerror(errno)); @@ -1777,15 +1778,13 @@ void peer_fundee_open(struct peer *peer, const u8 *from_peer) peer->owner = new_subd(ld, ld, "lightningd_opening", peer, opening_wire_type_name, NULL, peer_owner_finished, - peer->fd, peer->gossip_client_fd, -1); + take(&peer->fd), take(&peer->gossip_client_fd), + NULL); if (!peer->owner) { peer_fail(peer, "Failed to subdaemon opening: %s", strerror(errno)); return; } - /* We handed off peer fd and gossip fd */ - peer->fd = -1; - peer->gossip_client_fd = -1; /* They will open channel. */ peer->funder = REMOTE; @@ -1861,7 +1860,8 @@ static bool gossip_peer_released(struct subd *gossip, "lightningd_opening", fc->peer, opening_wire_type_name, NULL, peer_owner_finished, - fc->peer->fd, fc->peer->gossip_client_fd, -1); + take(&fc->peer->fd), + take(&fc->peer->gossip_client_fd), NULL); if (!opening) { peer_fail(fc->peer, "Failed to subdaemon opening: %s", strerror(errno)); @@ -1869,10 +1869,6 @@ static bool gossip_peer_released(struct subd *gossip, } fc->peer->owner = opening; - /* They took our fds. */ - fc->peer->fd = -1; - fc->peer->gossip_client_fd = -1; - /* We will fund channel */ fc->peer->funder = LOCAL; channel_config(ld, &fc->peer->our_config, diff --git a/lightningd/subd.c b/lightningd/subd.c index 7e474ed96..b3264cc09 100644 --- a/lightningd/subd.c +++ b/lightningd/subd.c @@ -124,7 +124,7 @@ static int subd(const char *dir, const char *name, bool debug, { int childmsg[2], execfail[2]; pid_t childpid; - int err, fd; + int err, *fd; if (socketpair(AF_LOCAL, SOCK_STREAM, 0, childmsg) != 0) goto fail; @@ -141,7 +141,7 @@ static int subd(const char *dir, const char *name, bool debug, goto close_execfail_fail; if (childpid == 0) { - int fdnum = 3; + int fdnum = 3, i; long max; const char *debug_arg = NULL; @@ -155,18 +155,18 @@ static int subd(const char *dir, const char *name, bool debug, } /* Dup any extra fds up first. */ - while ((fd = va_arg(ap, int)) != -1) { + while ((fd = va_arg(ap, int *)) != NULL) { /* If this were stdin, dup2 closed! */ - assert(fd != STDIN_FILENO); - if (!move_fd(fd, fdnum)) + assert(*fd != STDIN_FILENO); + if (!move_fd(*fd, fdnum)) goto child_errno_fail; fdnum++; } /* Make (fairly!) sure all other fds are closed. */ max = sysconf(_SC_OPEN_MAX); - for (fd = fdnum; fd < max; fd++) - close(fd); + for (i = fdnum; i < max; i++) + close(i); if (debug) debug_arg = "--debugger"; @@ -184,8 +184,12 @@ static int subd(const char *dir, const char *name, bool debug, close(childmsg[1]); close(execfail[1]); - while ((fd = va_arg(ap, int)) != -1) - close(fd); + while ((fd = va_arg(ap, int *)) != NULL) { + if (taken(fd)) { + close(*fd); + *fd = -1; + } + } /* Child will close this without writing on successful exec. */ if (read(execfail[0], &err, sizeof(err)) == sizeof(err)) { diff --git a/lightningd/subd.h b/lightningd/subd.h index 93adc75e6..695e08add 100644 --- a/lightningd/subd.h +++ b/lightningd/subd.h @@ -57,7 +57,7 @@ struct subd { * @msgname: function to get name from messages * @msgcb: function to call when non-fatal message received (or NULL) * @finished: function to call when it's finished (with exit status). - * @...: the fds to hand as fd 3, 4... terminated with -1. + * @...: NULL-terminated list of pointers to fds to hand as fd 3, 4... (can be take, if so, set to -1) * * @msgcb gets called with @fds set to NULL: if it returns a positive number, * that many @fds are received before calling again. If it returns -1, the