lightningd/subd: support multiple fds sent at once in request reply.

Instead of indicating where to place the fd, you say how many: the
fd array gets passed into the callback.

This is also clearer for the users.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

lightningd/gossip_control.c

@@ -152,6 +152,7 @@ void gossip_init(struct lightningd *ld)
 }
 
 static bool json_getnodes_reply(struct subd *gossip, const u8 *reply,
+				const int *fds,
 				struct command *cmd)
 {
 	struct gossip_getnodes_entry *nodes;
@@ -188,7 +189,7 @@ static void json_getnodes(struct command *cmd, const char *buffer,
 {
 	struct lightningd *ld = ld_from_dstate(cmd->dstate);
 	u8 *req = towire_gossip_getnodes_request(cmd);
-	subd_req(ld->gossip, req, -1, NULL, json_getnodes_reply, cmd);
+	subd_req(ld->gossip, req, -1, 0, json_getnodes_reply, cmd);
 }
 
 static const struct json_command getnodes_command = {

lightningd/hsm_control.c

@@ -10,7 +10,7 @@
 #include <lightningd/hsm/gen_hsm_wire.h>
 #include <wally_bip32.h>
 
-static bool hsm_init_done(struct subd *hsm, const u8 *msg,
+static bool hsm_init_done(struct subd *hsm, const u8 *msg, const int *fds,
 			  struct lightningd *ld)
 {
 	u8 *serialized_extkey;
@@ -98,7 +98,7 @@ void hsm_init(struct lightningd *ld, bool newdir)
 		create = (access("hsm_secret", F_OK) != 0);
 
 	subd_req(ld->hsm, take(towire_hsmctl_init(ld->hsm, create)),
-		 -1, NULL, hsm_init_done, ld);
+		 -1, 0, hsm_init_done, ld);
 
 	if (io_loop(NULL, NULL) != ld->hsm)
 		errx(1, "Unexpected io exit during HSM startup");

lightningd/peer_control.c

@@ -107,11 +107,13 @@ struct peer *peer_by_id(struct lightningd *ld, const struct pubkey *id)
 	return NULL;
 }
 
-static bool handshake_succeeded(struct subd *hs, const u8 *msg,
+static bool handshake_succeeded(struct subd *hs, const u8 *msg, const int *fds,
 				struct peer *peer)
 {
 	struct crypto_state cs;
 
+	assert(tal_count(fds) == 1);
+	peer->fd = fds[0];
 	if (!peer->id) {
 		struct pubkey id;
 
@@ -152,10 +154,12 @@ err:
 }
 
 static bool peer_got_handshake_hsmfd(struct subd *hsm, const u8 *msg,
+				     const int *fds,
 				     struct peer *peer)
 {
 	const u8 *req;
 
+	assert(tal_count(fds) == 1);
 	if (!fromwire_hsmctl_hsmfd_ecdh_fd_reply(msg, NULL)) {
 		log_unusual(peer->ld->log, "Malformed hsmfd response: %s",
 			    tal_hex(peer, msg));
@@ -168,7 +172,7 @@ static bool peer_got_handshake_hsmfd(struct subd *hsm, const u8 *msg,
 			       "lightningd_handshake", peer,
 			       handshake_wire_type_name,
 			       NULL, NULL,
-			       peer->hsmfd, peer->fd, -1);
+			       fds[0], peer->fd, -1);
 	if (!peer->owner) {
 		log_unusual(peer->ld->log, "Could not subdaemon handshake: %s",
 			    strerror(errno));
@@ -190,11 +194,11 @@ static bool peer_got_handshake_hsmfd(struct subd *hsm, const u8 *msg,
 
 	/* Now hand peer request to the handshake daemon: hands it
 	 * back on success */
-	subd_req(peer->owner, take(req), -1, &peer->fd,
-		 handshake_succeeded, peer);
+	subd_req(peer->owner, take(req), -1, 1, handshake_succeeded, peer);
 	return true;
 
 error:
+	close(fds[0]);
 	tal_free(peer);
 	return true;
 }
@@ -210,7 +214,7 @@ static struct io_plan *peer_in(struct io_conn *conn, struct lightningd *ld)
 	/* Get HSM fd for this peer. */
 	subd_req(ld->hsm,
 		 take(towire_hsmctl_hsmfd_ecdh(ld, peer->unique_id)),
-		 -1, &peer->hsmfd, peer_got_handshake_hsmfd, peer);
+		 -1, 1, peer_got_handshake_hsmfd, peer);
 
 	/* We don't need conn, we'll pass fd to handshaked. */
 	return io_close_taken_fd(conn);
@@ -341,7 +345,7 @@ static struct io_plan *peer_out(struct io_conn *conn,
 	/* Get HSM fd for this peer. */
 	subd_req(ld->hsm,
 		 take(towire_hsmctl_hsmfd_ecdh(ld, peer->unique_id)),
-		 -1, &peer->hsmfd, peer_got_handshake_hsmfd, peer);
+		 -1, 1, peer_got_handshake_hsmfd, peer);
 
 	/* We don't need conn, we'll pass fd to handshaked. */
 	return io_close_taken_fd(conn);
@@ -557,6 +561,7 @@ static enum watch_result funding_depth_cb(struct peer *peer,
 }
 
 static bool opening_got_hsm_funding_sig(struct subd *hsm, const u8 *resp,
+					const int *fds,
 					struct funding_channel *fc)
 {
 	secp256k1_ecdsa_signature *sigs;
@@ -681,6 +686,7 @@ static void peer_start_channeld(struct peer *peer, bool am_funder,
 }
 
 static bool opening_release_tx(struct subd *opening, const u8 *resp,
+			       const int *fds,
 			       struct funding_channel *fc)
 {
 	u8 *msg;
@@ -693,6 +699,9 @@ static bool opening_release_tx(struct subd *opening, const u8 *resp,
 	/* FIXME: marshal code wants array, not array of pointers. */
 	struct utxo *utxos = tal_arr(fc, struct utxo, tal_count(fc->utxomap));
 
+	assert(tal_count(fds) == 1);
+	fc->peer->fd = fds[0];
+
 	if (!fromwire_opening_open_funding_reply(resp, NULL,
 						 &their_config,
 						 &commit_sig,
@@ -718,7 +727,7 @@ static bool opening_release_tx(struct subd *opening, const u8 *resp,
 					 &fc->remote_fundingkey,
 					 utxos);
 	tal_free(utxos);
-	subd_req(fc->peer->ld->hsm, take(msg), -1, NULL,
+	subd_req(fc->peer->ld->hsm, take(msg), -1, 0,
 		 opening_got_hsm_funding_sig, fc);
 
 	/* Start normal channel daemon. */
@@ -732,7 +741,7 @@ static bool opening_release_tx(struct subd *opening, const u8 *resp,
 }
 
 static bool opening_gen_funding(struct subd *opening, const u8 *reply,
-				struct funding_channel *fc)
+				const int *fds, struct funding_channel *fc)
 {
 	u8 *msg;
 	struct pubkey changekey;
@@ -763,13 +772,13 @@ static bool opening_gen_funding(struct subd *opening, const u8 *reply,
 
 	msg = towire_opening_open_funding(fc, fc->peer->funding_txid,
 					  fc->peer->funding_outnum);
-	subd_req(fc->peer->owner, take(msg), -1, &fc->peer->fd,
-		 opening_release_tx, fc);
+	subd_req(fc->peer->owner, take(msg), -1, 1, opening_release_tx, fc);
 	return true;
 }
 
 static bool opening_accept_finish_response(struct subd *opening,
 					   const u8 *reply,
+					   const int *fds,
 					   struct peer *peer)
 {
 	struct channel_config their_config;
@@ -779,6 +788,9 @@ static bool opening_accept_finish_response(struct subd *opening,
 	struct pubkey remote_fundingkey, their_per_commit_point;
 
 	log_debug(peer->log, "Got opening_accept_finish_response");
+	assert(tal_count(fds) == 1);
+	peer->fd = fds[0];
+
 	if (!fromwire_opening_accept_finish_reply(reply, NULL,
 						  &peer->funding_outnum,
 						  &their_config,
@@ -806,6 +818,7 @@ static bool opening_accept_finish_response(struct subd *opening,
 }
 
 static bool opening_accept_reply(struct subd *opening, const u8 *reply,
+				 const int *fds,
 				 struct peer *peer)
 {
 	peer->funding_txid = tal(peer, struct sha256_double);
@@ -823,7 +836,7 @@ static bool opening_accept_reply(struct subd *opening, const u8 *reply,
 
 	/* Tell it we're watching. */
 	subd_req(opening, towire_opening_accept_finish(reply),
-		 -1, &peer->fd,
+		 -1, 1,
 		 opening_accept_finish_response, peer);
 	return true;
 }
@@ -932,12 +945,13 @@ void peer_accept_open(struct peer *peer,
 		tal_free(peer);
 		return;
 	}
-	subd_req(peer->owner, take(msg), -1, NULL, opening_accept_reply, peer);
+	subd_req(peer->owner, take(msg), -1, 0, opening_accept_reply, peer);
 }
 
 /* Peer has been released from gossip.  Start opening. */
 static bool gossip_peer_released(struct subd *gossip,
 				 const u8 *resp,
+				 const int *fds,
 				 struct funding_channel *fc)
 {
 	struct lightningd *ld = fc->peer->ld;
@@ -947,6 +961,9 @@ static bool gossip_peer_released(struct subd *gossip,
 	u8 *msg;
 	struct subd *opening;
 
+	assert(tal_count(fds) == 1);
+	fc->peer->fd = fds[0];
+
 	fc->cs = tal(fc, struct crypto_state);
 	if (!fromwire_gossipctl_release_peer_reply(resp, NULL, &id, fc->cs))
 		fatal("Gossup daemon gave invalid reply %s",
@@ -994,7 +1011,7 @@ static bool gossip_peer_released(struct subd *gossip,
 	msg = towire_opening_open(fc, fc->peer->funding_satoshi,
 				  fc->peer->push_msat,
 				  15000, max_minimum_depth);
-	subd_req(opening, take(msg), -1, NULL, opening_gen_funding, fc);
+	subd_req(opening, take(msg), -1, 0, opening_gen_funding, fc);
 	return true;
 }
 
@@ -1044,8 +1061,7 @@ static void json_fund_channel(struct command *cmd,
 	/* Tie this fc lifetime (and hence utxo release) to the peer */
 	tal_steal(fc->peer, fc);
 	tal_add_destructor(fc, fail_fundchannel_command);
-	subd_req(ld->gossip, msg, -1, &fc->peer->fd,
-		 gossip_peer_released, fc);
+	subd_req(ld->gossip, msg, -1, 1, gossip_peer_released, fc);
 }
 
 static const struct json_command fund_channel_command = {

lightningd/peer_control.h

@@ -36,9 +36,6 @@ struct peer {
 	/* Where we connected to, or it connected from. */
 	struct netaddr netaddr;
 
-	/* HSM connection for this peer. */
-	int hsmfd;
-
 	/* Json command which made us connect (if any) */
 	struct command *connect_cmd;
 

lightningd/subd.c

@@ -44,9 +44,11 @@ struct subd_req {
 
 	/* Callback for a reply. */
 	int reply_type;
-	bool (*replycb)(struct subd *, const u8 *msg_in, void *reply_data);
+	bool (*replycb)(struct subd *, const u8 *, const int *, void *);
 	void *replycb_data;
-	int *fd_in;
+
+	size_t num_fds_read;
+	int *fds_in;
 };
 
 static void free_subd_req(struct subd_req *sr)
@@ -54,19 +56,18 @@ static void free_subd_req(struct subd_req *sr)
 	list_del(&sr->list);
 }
 
-static void add_req(struct subd *sd, int type,
-		    bool (*replycb)(struct subd *, const u8 *, void *),
-		    void *replycb_data,
-		    int *reply_fd_in)
+static void add_req(struct subd *sd, int type, size_t num_fds_in,
+		    bool (*replycb)(struct subd *, const u8 *, const int *,
+				    void *),
+		    void *replycb_data)
 {
 	struct subd_req *sr = tal(sd, struct subd_req);
 
 	sr->reply_type = type + SUBD_REPLY_OFFSET;
 	sr->replycb = replycb;
 	sr->replycb_data = replycb_data;
-	sr->fd_in = reply_fd_in;
-	if (sr->fd_in)
-		*sr->fd_in = -1;
+	sr->fds_in = num_fds_in ? tal_arr(sr, int, num_fds_in) : NULL;
+	sr->num_fds_read = 0;
 	assert(strends(sd->msgname(sr->reply_type), "_REPLY"));
 
 	/* Keep in FIFO order: we sent in order, so replies will be too. */
@@ -184,18 +185,18 @@ static struct io_plan *sd_msg_reply(struct io_conn *conn, struct subd *sd,
 {
 	int type = fromwire_peektype(sd->msg_in);
 	bool keep_open;
+	size_t i;
 
-	if (sr->fd_in) {
-		/* Don't trust subd to set it blocking. */
-		set_blocking(*sr->fd_in, true);
-		log_info(sd->log, "REPLY %s with fd %i", sd->msgname(type),
-			 *sr->fd_in);
-	} else
-		log_info(sd->log, "REPLY %s", sd->msgname(type));
+	log_info(sd->log, "REPLY %s with %zu fds",
+		 sd->msgname(type), tal_count(sr->fds_in));
+
+	/* Don't trust subd to set it blocking. */
+	for (i = 0; i < tal_count(sr->fds_in); i++)
+		set_blocking(sr->fds_in[i], true);
 
 	/* If not stolen, we'll free this below. */
 	tal_steal(sr, sd->msg_in);
-	keep_open = sr->replycb(sd, sd->msg_in, sr->replycb_data);
+	keep_open = sr->replycb(sd, sd->msg_in, sr->fds_in, sr->replycb_data);
 	tal_free(sr);
 
 	if (!keep_open)
@@ -220,9 +221,11 @@ static struct io_plan *sd_msg_read(struct io_conn *conn, struct subd *sd)
 	/* First, check for replies. */
 	sr = get_req(sd, type);
 	if (sr) {
-		/* If we need fd, read it and call us again. */
-		if (sr->fd_in && *sr->fd_in == -1)
-			return io_recv_fd(conn, sr->fd_in, sd_msg_read, sd);
+		/* If we need (another) fd, read it and call us again. */
+		if (sr->num_fds_read < tal_count(sr->fds_in)) {
+			return io_recv_fd(conn, &sr->fds_in[sr->num_fds_read++],
+					  sd_msg_read, sd);
+		}
 		return sd_msg_reply(conn, sd, sr);
 	}
 
@@ -371,8 +374,8 @@ void subd_send_fd(struct subd *sd, int fd)
 
 void subd_req_(struct subd *sd,
 	       const u8 *msg_out,
-	       int fd_out, int *fd_in,
-	       bool (*replycb)(struct subd *, const u8 *, void *),
+	       int fd_out, size_t num_fds_in,
+	       bool (*replycb)(struct subd *, const u8 *, const int *, void *),
 	       void *replycb_data)
 {
 	/* Grab type now in case msg_out is taken() */
@@ -382,7 +385,7 @@ void subd_req_(struct subd *sd,
 	if (fd_out >= 0)
 		subd_send_fd(sd, fd_out);
 
-	add_req(sd, type, replycb, replycb_data, fd_in);
+	add_req(sd, type, num_fds_in, replycb, replycb_data);
 }
 
 char *opt_subd_debug(const char *optarg, struct lightningd *ld)

lightningd/subd.h

@@ -96,24 +96,24 @@ void subd_send_fd(struct subd *sd, int fd);
  * @sd: subdaemon to request
  * @msg_out: request message (can be take)
  * @fd_out: if >=0 fd to pass at the end of the message (closed after)
- * @fd_in: if not NULL, where to put fd read in at end of reply.
+ * @num_fds_in: how many fds to read in to hand to @replycb.
  * @replycb: callback when reply comes in, returns false to shutdown daemon.
  * @replycb_data: final arg to hand to @replycb
  *
  * @replycb cannot free @sd, so it returns false to remove it.
  */
-#define subd_req(sd, msg_out, fd_out, fd_in, replycb, replycb_data)	\
-	subd_req_((sd), (msg_out), (fd_out), (fd_in),			\
-		       typesafe_cb_preargs(bool, void *,		\
-					   (replycb), (replycb_data),	\
-					   struct subd *,		\
-					   const u8 *),			\
+#define subd_req(sd, msg_out, fd_out, num_fds_in, replycb, replycb_data) \
+	subd_req_((sd), (msg_out), (fd_out), (num_fds_in),		\
+		  typesafe_cb_preargs(bool, void *,			\
+				      (replycb), (replycb_data),	\
+				      struct subd *,			\
+				      const u8 *, const int *),		\
 		       (replycb_data))
 void subd_req_(struct subd *sd,
-		    const u8 *msg_out,
-		    int fd_out, int *fd_in,
-		    bool (*replycb)(struct subd *, const u8 *, void *),
-		    void *replycb_data);
+	       const u8 *msg_out,
+	       int fd_out, size_t num_fds_in,
+	       bool (*replycb)(struct subd *, const u8 *, const int *, void *),
+	       void *replycb_data);
 
 char *opt_subd_debug(const char *optarg, struct lightningd *ld);