subd: automatically detect if callback frees subd.

This involves a tricky callback internally, but far less error-prone.

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

lightningd/dev_ping.c

@@ -9,7 +9,7 @@
 #include <lightningd/peer_control.h>
 #include <lightningd/subd.h>
 
-static bool ping_reply(struct subd *subd, const u8 *msg, const int *fds,
+static void ping_reply(struct subd *subd, const u8 *msg, const int *fds,
 		       struct command *cmd)
 {
 	u16 totlen;
@@ -33,7 +33,6 @@ static bool ping_reply(struct subd *subd, const u8 *msg, const int *fds,
 		json_object_end(response);
 		command_success(cmd, response);
 	}
-	return true;
 }
 
 static void json_dev_ping(struct command *cmd,

lightningd/gossip_control.c

@@ -49,7 +49,7 @@ static void peer_nongossip(struct subd *gossip, const u8 *msg,
 			    peer_fd, gossip_fd, in_pkt);
 }
 
-static int gossip_msg(struct subd *gossip, const u8 *msg, const int *fds)
+static unsigned gossip_msg(struct subd *gossip, const u8 *msg, const int *fds)
 {
 	enum gossip_wire_type t = fromwire_peektype(msg);
 
@@ -124,7 +124,7 @@ void gossip_init(struct lightningd *ld)
 	tal_free(tmpctx);
 }
 
-static bool json_getnodes_reply(struct subd *gossip, const u8 *reply,
+static void json_getnodes_reply(struct subd *gossip, const u8 *reply,
 				const int *fds,
 				struct command *cmd)
 {
@@ -134,7 +134,7 @@ static bool json_getnodes_reply(struct subd *gossip, const u8 *reply,
 
 	if (!fromwire_gossip_getnodes_reply(reply, reply, NULL, &nodes)) {
 		command_fail(cmd, "Malformed gossip_getnodes response");
-		return true;
+		return;
 	}
 
 	json_object_start(response, NULL);
@@ -153,7 +153,6 @@ static bool json_getnodes_reply(struct subd *gossip, const u8 *reply,
 	json_array_end(response);
 	json_object_end(response);
 	command_success(cmd, response);
-	return true;
 }
 
 static void json_getnodes(struct command *cmd, const char *buffer,
@@ -168,7 +167,7 @@ static const struct json_command getnodes_command = {
     "Returns a list of all nodes that we know about"};
 AUTODATA(json_command, &getnodes_command);
 
-static bool json_getroute_reply(struct subd *gossip, const u8 *reply, const int *fds,
+static void json_getroute_reply(struct subd *gossip, const u8 *reply, const int *fds,
 				struct command *cmd)
 {
 	struct json_result *response;
@@ -179,7 +178,7 @@ static bool json_getroute_reply(struct subd *gossip, const u8 *reply, const int
 
 	if (tal_count(hops) == 0) {
 		command_fail(cmd, "Could not find a route");
-		return true;
+		return;
 	}
 
 	response = new_json_result(cmd);
@@ -197,7 +196,6 @@ static bool json_getroute_reply(struct subd *gossip, const u8 *reply, const int
 	json_array_end(response);
 	json_object_end(response);
 	command_success(cmd, response);
-	return true;
 }
 
 static void json_getroute(struct command *cmd, const char *buffer, const jsmntok_t *params)
@@ -247,7 +245,7 @@ static const struct json_command getroute_command = {
 AUTODATA(json_command, &getroute_command);
 
 /* Called upon receiving a getchannels_reply from `gossipd` */
-static bool json_getchannels_reply(struct subd *gossip, const u8 *reply,
+static void json_getchannels_reply(struct subd *gossip, const u8 *reply,
 				   const int *fds, struct command *cmd)
 {
 	size_t i;
@@ -257,7 +255,7 @@ static bool json_getchannels_reply(struct subd *gossip, const u8 *reply,
 
 	if (!fromwire_gossip_getchannels_reply(reply, reply, NULL, &entries)) {
 		command_fail(cmd, "Invalid reply from gossipd");
-		return true;
+		return;
 	}
 
 	json_object_start(response, NULL);
@@ -288,7 +286,6 @@ static bool json_getchannels_reply(struct subd *gossip, const u8 *reply,
 	json_array_end(response);
 	json_object_end(response);
 	command_success(cmd, response);
-	return true;
 }
 
 static void json_getchannels(struct command *cmd, const char *buffer,

lightningd/peer_control.c

@@ -786,13 +786,12 @@ static const struct json_command dev_fail_command = {
 };
 AUTODATA(json_command, &dev_fail_command);
 
-static bool dev_reenable_commit_finished(struct subd *channeld,
+static void dev_reenable_commit_finished(struct subd *channeld,
 					 const u8 *resp,
 					 const int *fds,
 					 struct command *cmd)
 {
 	command_success(cmd, null_response(cmd));
-	return true;
 }
 
 static void json_dev_reenable_commit(struct command *cmd,
@@ -1024,26 +1023,26 @@ static void onchaind_tell_fulfill(struct peer *peer)
 	}
 }
 
-static int handle_onchain_init_reply(struct peer *peer, const u8 *msg)
+static void handle_onchain_init_reply(struct peer *peer, const u8 *msg)
 {
 	u8 state;
 
 	if (!fromwire_onchain_init_reply(msg, NULL, &state)) {
-		log_broken(peer->log, "Invalid onchain_init_reply");
-		return -1;
+		peer_internal_error(peer, "Invalid onchain_init_reply");
+		return;
 	}
 
 	if (!peer_state_on_chain(state)) {
-		log_broken(peer->log, "Invalid onchain_init_reply state %u (%s)",
-			   state, peer_state_name(state));
-		return -1;
+		peer_internal_error(peer,
+				    "Invalid onchain_init_reply state %u (%s)",
+				    state, peer_state_name(state));
+		return;
 	}
 
 	peer_set_condition(peer, FUNDING_SPEND_SEEN, state);
 
 	/* Tell it about any preimages we know. */
 	onchaind_tell_fulfill(peer);
-	return 0;
 }
 
 static enum watch_result onchain_tx_watched(struct peer *peer,
@@ -1112,47 +1111,44 @@ static void watch_tx_and_outputs(struct peer *peer,
 			  onchain_txo_watched, NULL);
 }
 
-static int handle_onchain_broadcast_tx(struct peer *peer, const u8 *msg)
+static void handle_onchain_broadcast_tx(struct peer *peer, const u8 *msg)
 {
 	struct bitcoin_tx *tx;
 
 	tx = tal(msg, struct bitcoin_tx);
 	if (!fromwire_onchain_broadcast_tx(msg, NULL, tx)) {
-		log_broken(peer->log, "Invalid onchain_broadcast_tx");
-		return -1;
+		peer_internal_error(peer, "Invalid onchain_broadcast_tx");
+		return;
 	}
 
 	/* We don't really care if it fails, we'll respond via watch. */
 	broadcast_tx(peer->ld->topology, peer, tx, NULL);
-	return 0;
 }
 
-static int handle_onchain_unwatch_tx(struct peer *peer, const u8 *msg)
+static void handle_onchain_unwatch_tx(struct peer *peer, const u8 *msg)
 {
 	/* FIXME: unwatch tx and children here. */
-	return 0;
 }
 
-static int handle_extracted_preimage(struct peer *peer, const u8 *msg)
+static void handle_extracted_preimage(struct peer *peer, const u8 *msg)
 {
 	struct preimage preimage;
 
 	if (!fromwire_onchain_extracted_preimage(msg, NULL, &preimage)) {
-		log_broken(peer->log, "Invalid extracted_preimage");
-		return -1;
+		peer_internal_error(peer, "Invalid extracted_preimage");
+		return;
 	}
 
 	onchain_fulfilled_htlc(peer, &preimage);
-	return 0;
 }
 
-static int handle_missing_htlc_output(struct peer *peer, const u8 *msg)
+static void handle_missing_htlc_output(struct peer *peer, const u8 *msg)
 {
 	struct htlc_stub htlc;
 
 	if (!fromwire_onchain_missing_htlc_output(msg, NULL, &htlc)) {
-		log_broken(peer->log, "Invalid missing_htlc_output");
-		return -1;
+		peer_internal_error(peer, "Invalid missing_htlc_output");
+		return;
 	}
 
 	/* BOLT #5:
@@ -1164,16 +1160,15 @@ static int handle_missing_htlc_output(struct peer *peer, const u8 *msg)
 	 * transaction contains an output corresponding to the HTLC.
 	 */
 	onchain_failed_our_htlc(peer, &htlc, "missing in commitment tx");
-	return 0;
 }
 
-static int handle_onchain_htlc_timeout(struct peer *peer, const u8 *msg)
+static void handle_onchain_htlc_timeout(struct peer *peer, const u8 *msg)
 {
 	struct htlc_stub htlc;
 
 	if (!fromwire_onchain_htlc_timeout(msg, NULL, &htlc)) {
-		log_broken(peer->log, "Invalid onchain_htlc_timeout");
-		return -1;
+		peer_internal_error(peer, "Invalid onchain_htlc_timeout");
+		return;
 	}
 
 	/* BOLT #5:
@@ -1184,47 +1179,49 @@ static int handle_onchain_htlc_timeout(struct peer *peer, const u8 *msg)
 	 * depth.
 	 */
 	onchain_failed_our_htlc(peer, &htlc, "timed out");
-	return 0;
 }
 
-static int handle_irrevocably_resolved(struct peer *peer, const u8 *msg)
+static void handle_irrevocably_resolved(struct peer *peer, const u8 *msg)
 {
 	/* FIXME: Remove peer from db. */
 	log_info(peer->log, "onchaind complete, forgetting peer");
 
-	/* Peer is gone: don't free sd yet though; it will exit. */
-	peer->owner->peer = NULL;
-	peer->owner = NULL;
-
+	/* This will also free onchaind. */
 	tal_free(peer);
-	return 0;
 }
 
-static int onchain_msg(struct subd *sd, const u8 *msg, const int *fds)
+static unsigned int onchain_msg(struct subd *sd, const u8 *msg, const int *fds)
 {
 	enum onchain_wire_type t = fromwire_peektype(msg);
 
 	switch (t) {
 	case WIRE_ONCHAIN_INIT_REPLY:
-		return handle_onchain_init_reply(sd->peer, msg);
+		handle_onchain_init_reply(sd->peer, msg);
+		break;
 
 	case WIRE_ONCHAIN_BROADCAST_TX:
-		return handle_onchain_broadcast_tx(sd->peer, msg);
+		handle_onchain_broadcast_tx(sd->peer, msg);
+		break;
 
 	case WIRE_ONCHAIN_UNWATCH_TX:
-		return handle_onchain_unwatch_tx(sd->peer, msg);
+		handle_onchain_unwatch_tx(sd->peer, msg);
+		break;
 
  	case WIRE_ONCHAIN_EXTRACTED_PREIMAGE:
-		return handle_extracted_preimage(sd->peer, msg);
+		handle_extracted_preimage(sd->peer, msg);
+		break;
 
 	case WIRE_ONCHAIN_MISSING_HTLC_OUTPUT:
-		return handle_missing_htlc_output(sd->peer, msg);
+		handle_missing_htlc_output(sd->peer, msg);
+		break;
 
 	case WIRE_ONCHAIN_HTLC_TIMEOUT:
-		return handle_onchain_htlc_timeout(sd->peer, msg);
+		handle_onchain_htlc_timeout(sd->peer, msg);
+		break;
 
 	case WIRE_ONCHAIN_ALL_IRREVOCABLY_RESOLVED:
-		return handle_irrevocably_resolved(sd->peer, msg);
+		handle_irrevocably_resolved(sd->peer, msg);
+		break;
 
 	/* We send these, not receive them */
 	case WIRE_ONCHAIN_INIT:
@@ -1539,7 +1536,7 @@ static u8 *create_node_announcement(const tal_t *ctx, struct lightningd *ld,
  * an update, so we can now start sending a node_announcement. The
  * first step is to build the provisional announcement and ask the HSM
  * to sign it. */
-static int peer_channel_announced(struct peer *peer, const u8 *msg)
+static void peer_channel_announced(struct peer *peer, const u8 *msg)
 {
 	struct lightningd *ld = peer->ld;
 	tal_t *tmpctx = tal_tmpctx(peer);
@@ -1550,7 +1547,7 @@ static int peer_channel_announced(struct peer *peer, const u8 *msg)
 	if (!fromwire_channel_announced(msg, NULL)) {
 		peer_internal_error(peer, "bad fromwire_channel_announced %s",
 				    tal_hex(peer, msg));
-		return -1;
+		return;
 	}
 
 	msg = towire_hsmctl_node_announcement_sig_req(
@@ -1570,40 +1567,37 @@ static int peer_channel_announced(struct peer *peer, const u8 *msg)
 	wrappedmsg = towire_gossip_forwarded_msg(tmpctx, announcement);
 	subd_send_msg(ld->gossip, take(wrappedmsg));
 	tal_free(tmpctx);
-
-	return 0;
 }
 
-static int peer_got_funding_locked(struct peer *peer, const u8 *msg)
+static void peer_got_funding_locked(struct peer *peer, const u8 *msg)
 {
 	struct pubkey next_per_commitment_point;
 
 	if (!fromwire_channel_got_funding_locked(msg, NULL,
 						 &next_per_commitment_point)) {
-		log_broken(peer->log, "bad channel_got_funding_locked %s",
-			   tal_hex(peer, msg));
-		return -1;
+		peer_internal_error(peer, "bad channel_got_funding_locked %s",
+				    tal_hex(peer, msg));
+		return;
 	}
 
 	if (peer->remote_funding_locked) {
-		log_broken(peer->log, "channel_got_funding_locked twice");
-		return -1;
+		peer_internal_error(peer, "channel_got_funding_locked twice");
+		return;
 	}
 	update_per_commit_point(peer, &next_per_commitment_point);
 
 	log_debug(peer->log, "Got funding_locked");
 	peer->remote_funding_locked = true;
-	return 0;
 }
 
-static int peer_got_shutdown(struct peer *peer, const u8 *msg)
+static void peer_got_shutdown(struct peer *peer, const u8 *msg)
 {
 	u8 *scriptpubkey;
 
 	if (!fromwire_channel_got_shutdown(peer, msg, NULL, &scriptpubkey)) {
-		log_broken(peer->log, "bad channel_got_shutdown %s",
-			   tal_hex(peer, msg));
-		return -1;
+		peer_internal_error(peer, "bad channel_got_shutdown %s",
+				    tal_hex(peer, msg));
+		return;
 	}
 
 	/* FIXME: Add to spec that we must allow repeated shutdown! */
@@ -1628,7 +1622,7 @@ static int peer_got_shutdown(struct peer *peer, const u8 *msg)
 		char *str = tal_fmt(peer, "Bad shutdown scriptpubkey %s",
 				    tal_hex(peer, scriptpubkey));
 		peer_fail_permanent_str(peer, take(str));
-		return -1;
+		return;
 	}
 
 	if (peer->local_shutdown_idx == -1) {
@@ -1638,7 +1632,7 @@ static int peer_got_shutdown(struct peer *peer, const u8 *msg)
 		if (peer->local_shutdown_idx == -1) {
 			peer_internal_error(peer,
 					    "Can't get local shutdown index");
-			return -1;
+			return;
 		}
 
 		peer_set_condition(peer, CHANNELD_NORMAL, CHANNELD_SHUTTING_DOWN);
@@ -1656,7 +1650,7 @@ static int peer_got_shutdown(struct peer *peer, const u8 *msg)
 			peer_internal_error(peer,
 					    "Can't get shutdown script %"PRIu64,
 					    peer->local_shutdown_idx);
-			return -1;
+			return;
 		}
 
 		/* BOLT #2:
@@ -1675,8 +1669,6 @@ static int peer_got_shutdown(struct peer *peer, const u8 *msg)
 		fatal("Could not save channel to database: %s",
 		      peer->ld->wallet->db->err);
 	}
-
-	return 0;
 }
 
 void peer_last_tx(struct peer *peer, struct bitcoin_tx *tx,
@@ -1770,7 +1762,7 @@ static bool better_closing_fee(struct peer *peer, const struct bitcoin_tx *tx)
 	return (new_diff < old_diff);
 }
 
-static int peer_received_closing_signature(struct peer *peer, const u8 *msg)
+static void peer_received_closing_signature(struct peer *peer, const u8 *msg)
 {
 	secp256k1_ecdsa_signature sig;
 	struct bitcoin_tx *tx = tal(msg, struct bitcoin_tx);
@@ -1778,7 +1770,7 @@ static int peer_received_closing_signature(struct peer *peer, const u8 *msg)
 	if (!fromwire_closing_received_signature(msg, NULL, &sig, tx)) {
 		peer_internal_error(peer, "Bad closing_received_signature %s",
 				    tal_hex(peer, msg));
-		return -1;
+		return;
 	}
 
 	/* FIXME: Make sure signature is correct! */
@@ -1795,36 +1787,36 @@ static int peer_received_closing_signature(struct peer *peer, const u8 *msg)
 	/* OK, you can continue now. */
 	subd_send_msg(peer->owner,
 		      take(towire_closing_received_signature_reply(peer)));
-	return 0;
 }
 
-static int peer_closing_complete(struct peer *peer, const u8 *msg)
+static void peer_closing_complete(struct peer *peer, const u8 *msg)
 {
 	if (!fromwire_closing_complete(msg, NULL)) {
 		peer_internal_error(peer, "Bad closing_complete %s",
 				    tal_hex(peer, msg));
-		return -1;
+		return;
 	}
 
 	/* Retransmission only, ignore closing. */
 	if (peer->state == CLOSINGD_COMPLETE)
-		return -1;
+		return;
 
 	drop_to_chain(peer);
 	peer_set_condition(peer, CLOSINGD_SIGEXCHANGE, CLOSINGD_COMPLETE);
-	return -1;
 }
 
-static int closing_msg(struct subd *sd, const u8 *msg, const int *fds)
+static unsigned closing_msg(struct subd *sd, const u8 *msg, const int *fds)
 {
 	enum closing_wire_type t = fromwire_peektype(msg);
 
 	switch (t) {
 	case WIRE_CLOSING_RECEIVED_SIGNATURE:
-		return peer_received_closing_signature(sd->peer, msg);
+		peer_received_closing_signature(sd->peer, msg);
+		break;
 
 	case WIRE_CLOSING_COMPLETE:
-		return peer_closing_complete(sd->peer, msg);
+		peer_closing_complete(sd->peer, msg);
+		break;
 
 	/* We send these, not receive them */
 	case WIRE_CLOSING_INIT:
@@ -1917,29 +1909,26 @@ static void peer_start_closingd(struct peer *peer,
 	tal_free(tmpctx);
 }
 
-static int peer_start_closingd_after_shutdown(struct peer *peer, const u8 *msg,
-					      const int *fds)
+static void peer_start_closingd_after_shutdown(struct peer *peer, const u8 *msg,
+					       const int *fds)
 {
 	struct crypto_state cs;
 
 	/* We expect 2 fds. */
-	if (!fds)
-		return 2;
+	assert(tal_count(fds) == 2);
 
 	if (!fromwire_channel_shutdown_complete(msg, NULL, &cs)) {
 		peer_internal_error(peer, "bad shutdown_complete: %s",
 				    tal_hex(peer, msg));
-		return -1;
+		return;
 	}
 
+	/* This sets peer->owner, closes down channeld. */
 	peer_start_closingd(peer, &cs, fds[0], fds[1], false);
 	peer_set_condition(peer, CHANNELD_SHUTTING_DOWN, CLOSINGD_SIGEXCHANGE);
-
-	/* Close the channeld */
-	return -1;
 }
 
-static int channel_msg(struct subd *sd, const u8 *msg, const int *fds)
+static unsigned channel_msg(struct subd *sd, const u8 *msg, const int *fds)
 {
 	enum channel_wire_type t = fromwire_peektype(msg);
 
@@ -1949,19 +1938,29 @@ static int channel_msg(struct subd *sd, const u8 *msg, const int *fds)
 				   CHANNELD_AWAITING_LOCKIN, CHANNELD_NORMAL);
 		break;
 	case WIRE_CHANNEL_SENDING_COMMITSIG:
-		return peer_sending_commitsig(sd->peer, msg);
+		peer_sending_commitsig(sd->peer, msg);
+		break;
 	case WIRE_CHANNEL_GOT_COMMITSIG:
-		return peer_got_commitsig(sd->peer, msg);
+		peer_got_commitsig(sd->peer, msg);
+		break;
 	case WIRE_CHANNEL_GOT_REVOKE:
-		return peer_got_revoke(sd->peer, msg);
+		peer_got_revoke(sd->peer, msg);
+		break;
 	case WIRE_CHANNEL_ANNOUNCED:
-		return peer_channel_announced(sd->peer, msg);
+		peer_channel_announced(sd->peer, msg);
+		break;
 	case WIRE_CHANNEL_GOT_FUNDING_LOCKED:
-		return peer_got_funding_locked(sd->peer, msg);
+		peer_got_funding_locked(sd->peer, msg);
+		break;
 	case WIRE_CHANNEL_GOT_SHUTDOWN:
-		return peer_got_shutdown(sd->peer, msg);
+		peer_got_shutdown(sd->peer, msg);
+		break;
 	case WIRE_CHANNEL_SHUTDOWN_COMPLETE:
-		return peer_start_closingd_after_shutdown(sd->peer, msg, fds);
+		/* We expect 2 fds. */
+		if (!fds)
+			return 2;
+		peer_start_closingd_after_shutdown(sd->peer, msg, fds);
+		break;
 
 	/* And we never get these from channeld. */
 	case WIRE_CHANNEL_INIT:
@@ -2123,7 +2122,7 @@ static bool peer_commit_initial(struct peer *peer)
 	return true;
 }
 
-static bool opening_funder_finished(struct subd *opening, const u8 *resp,
+static void opening_funder_finished(struct subd *opening, const u8 *resp,
 				    const int *fds,
 				    struct funding_channel *fc)
 {
@@ -2162,7 +2161,7 @@ static bool opening_funder_finished(struct subd *opening, const u8 *resp,
 					   &channel_info->feerate_per_kw)) {
 		peer_internal_error(fc->peer, "bad funder_reply: %s",
 				    tal_hex(resp, resp));
-		return false;
+		return;
 	}
 
 	/* old_remote_per_commit not valid yet, copy valid one. */
@@ -2211,12 +2210,12 @@ static bool opening_funder_finished(struct subd *opening, const u8 *resp,
 						   &local_fundingkey),
 				    type_to_string(fc, struct pubkey,
 						   &channel_info->remote_fundingkey));
-		return false;
+		return;
 	}
 
 	if (!peer_commit_initial(fc->peer)) {
 		peer_internal_error(fc->peer, "Initial peer to db failed");
-		return false;
+		return;
 	}
 
 	/* Get HSM to sign the funding tx. */
@@ -2238,15 +2237,12 @@ static bool opening_funder_finished(struct subd *opening, const u8 *resp,
 
 	msg = hsm_sync_read(fc, fc->peer->ld);
 	opening_got_hsm_funding_sig(fc, fds[0], fds[1], msg, &cs);
-
-	/* openingd already exited. */
-	return false;
 }
 
-static bool opening_fundee_finished(struct subd *opening,
-					   const u8 *reply,
-					   const int *fds,
-					   struct peer *peer)
+static void opening_fundee_finished(struct subd *opening,
+				    const u8 *reply,
+				    const int *fds,
+				    struct peer *peer)
 {
 	u8 *funding_signed;
 	struct channel_info *channel_info;
@@ -2282,9 +2278,9 @@ static bool opening_fundee_finished(struct subd *opening,
 					   &peer->channel_flags,
 					   &channel_info->feerate_per_kw,
 					   &funding_signed)) {
-		log_broken(peer->log, "bad OPENING_FUNDEE_REPLY %s",
-			   tal_hex(reply, reply));
-		return false;
+		peer_internal_error(peer, "bad OPENING_FUNDEE_REPLY %s",
+				    tal_hex(reply, reply));
+		return;
 	}
 	/* old_remote_per_commit not valid yet, copy valid one. */
 	channel_info->old_remote_per_commit = channel_info->remote_per_commit;
@@ -2293,7 +2289,7 @@ static bool opening_fundee_finished(struct subd *opening,
 	peer_last_tx(peer, remote_commit, &remote_commit_sig);
 
 	if (!peer_commit_initial(peer))
-		return false;
+		return;
 
 	log_debug(peer->log, "Watching funding tx %s",
 		     type_to_string(reply, struct sha256_double,
@@ -2311,9 +2307,6 @@ static bool opening_fundee_finished(struct subd *opening,
 	/* On to normal operation! */
 	peer_start_channeld(peer, &cs, fds[0], fds[1], funding_signed, false);
 	peer_set_condition(peer, OPENINGD, CHANNELD_AWAITING_LOCKIN);
-
-	/* openingd already exited. */
-	return false;
 }
 
 /* Peer has spontaneously exited from gossip due to open msg */
@@ -2496,7 +2489,7 @@ static void peer_offer_channel(struct lightningd *ld,
 }
 
 /* Peer has been released from gossip.  Start opening. */
-static bool gossip_peer_released(struct subd *gossip,
+static void gossip_peer_released(struct subd *gossip,
 				 const u8 *resp,
 				 const int *fds,
 				 struct funding_channel *fc)
@@ -2519,7 +2512,7 @@ static bool gossip_peer_released(struct subd *gossip,
 				     peer_state_name(fc->peer->state));
 		else
 			command_fail(fc->cmd, "Peer not connected");
-		return true;
+		return;
 	}
 	assert(tal_count(fds) == 2);
 
@@ -2530,12 +2523,11 @@ static bool gossip_peer_released(struct subd *gossip,
 			     peer_state_name(fc->peer->state));
 		close(fds[0]);
 		close(fds[1]);
-		return true;
+		return;
 	}
 
 	/* OK, offer peer a channel. */
 	peer_offer_channel(ld, fc, &cs, gfeatures, lfeatures, fds[0], fds[1]);
-	return true;
 }
 
 static void json_fund_channel(struct command *cmd,

lightningd/peer_htlcs.c

@@ -395,7 +395,7 @@ static void hout_subd_died(struct htlc_out *hout)
 
 /* This is where channeld gives us the HTLC id, and also reports if it
  * failed immediately. */
-static bool rcvd_htlc_reply(struct subd *subd, const u8 *msg, const int *fds,
+static void rcvd_htlc_reply(struct subd *subd, const u8 *msg, const int *fds,
 			    struct htlc_out *hout)
 {
 	u16 failure_code;
@@ -407,7 +407,7 @@ static bool rcvd_htlc_reply(struct subd *subd, const u8 *msg, const int *fds,
 					       &failurestr)) {
 		peer_internal_error(subd->peer, "Bad channel_offer_htlc_reply");
 		tal_free(hout);
-		return false;
+		return;
 	}
 
 	if (failure_code) {
@@ -419,7 +419,7 @@ static bool rcvd_htlc_reply(struct subd *subd, const u8 *msg, const int *fds,
 			payment_failed(hout->key.peer->ld, hout, localfail);
 		} else
 			local_fail_htlc(hout->in, failure_code);
-		return true;
+		return;
 	}
 
 	if (find_htlc_out(&subd->ld->htlcs_out, hout->key.peer, hout->key.id)) {
@@ -428,14 +428,13 @@ static bool rcvd_htlc_reply(struct subd *subd, const u8 *msg, const int *fds,
 				    " is a duplicate",
 				    hout->key.id);
 		tal_free(hout);
-		return false;
+		return;
 	}
 
 	/* Add it to lookup table now we know id. */
 	connect_htlc_out(&subd->ld->htlcs_out, hout);
 
 	/* When channeld includes it in commitment, we'll make it persistent. */
-	return true;
 }
 
 enum onion_type send_htlc_out(struct peer *out, u64 amount, u32 cltv,
@@ -558,7 +557,7 @@ struct gossip_resolve {
 
 /* We received a resolver reply, which gives us the node_ids of the
  * channel we want to forward over */
-static bool channel_resolve_reply(struct subd *gossip, const u8 *msg,
+static void channel_resolve_reply(struct subd *gossip, const u8 *msg,
 				  const int *fds, struct gossip_resolve *gr)
 {
 	struct pubkey *nodes, *peer_id;
@@ -567,17 +566,17 @@ static bool channel_resolve_reply(struct subd *gossip, const u8 *msg,
 		log_broken(gossip->log,
 			   "bad fromwire_gossip_resolve_channel_reply %s",
 			   tal_hex(msg, msg));
-		return false;
+		return;
 	}
 
 	if (tal_count(nodes) == 0) {
 		local_fail_htlc(gr->hin, WIRE_UNKNOWN_NEXT_PEER);
-		return true;
+		return;
 	} else if (tal_count(nodes) != 2) {
 		log_broken(gossip->log,
 			   "fromwire_gossip_resolve_channel_reply has %zu nodes",
 			   tal_count(nodes));
-		return false;
+		return;
 	}
 
 	/* Get the other peer matching the id that is not us */
@@ -591,7 +590,6 @@ static bool channel_resolve_reply(struct subd *gossip, const u8 *msg,
 		     gr->amt_to_forward, gr->outgoing_cltv_value, peer_id,
 		     gr->next_onion);
 	tal_free(gr);
-	return true;
 }
 
 /* Everyone is committed to this htlc of theirs */
@@ -970,7 +968,7 @@ static bool peer_save_commitsig_sent(struct peer *peer, u64 commitnum)
 	return true;
 }
 
-int peer_sending_commitsig(struct peer *peer, const u8 *msg)
+void peer_sending_commitsig(struct peer *peer, const u8 *msg)
 {
 	u64 commitnum;
 	struct changed_htlc *changed_htlcs;
@@ -984,14 +982,14 @@ int peer_sending_commitsig(struct peer *peer, const u8 *msg)
 						&commit_sig, &htlc_sigs)) {
 		peer_internal_error(peer, "bad channel_sending_commitsig %s",
 				    tal_hex(peer, msg));
-		return -1;
+		return;
 	}
 
 	for (i = 0; i < tal_count(changed_htlcs); i++) {
 		if (!changed_htlc(peer, changed_htlcs + i)) {
 			peer_internal_error(peer,
 				   "channel_sending_commitsig: update failed");
-			return -1;
+			return;
 		}
 
 		/* While we're here, sanity check added ones are in
@@ -1010,14 +1008,14 @@ int peer_sending_commitsig(struct peer *peer, const u8 *msg)
 				   " Added %"PRIu64", maxid now %"PRIu64
 				   " from %"PRIu64,
 				   num_local_added, maxid, peer->next_htlc_id);
-			return -1;
+			return;
 		}
 		/* FIXME: Save to db */
 		peer->next_htlc_id += num_local_added;
 	}
 
 	if (!peer_save_commitsig_sent(peer, commitnum))
-		return -1;
+		return;
 
 	/* Last was commit. */
 	peer->last_was_revoke = false;
@@ -1027,7 +1025,6 @@ int peer_sending_commitsig(struct peer *peer, const u8 *msg)
 	/* Tell it we've got it, and to go ahead with commitment_signed. */
 	subd_send_msg(peer->owner,
 		      take(towire_channel_sending_commitsig_reply(msg)));
-	return 0;
 }
 
 static void added_their_htlc(struct peer *peer,
@@ -1090,7 +1087,7 @@ static bool peer_sending_revocation(struct peer *peer,
 }
 
 /* This also implies we're sending revocation */
-int peer_got_commitsig(struct peer *peer, const u8 *msg)
+void peer_got_commitsig(struct peer *peer, const u8 *msg)
 {
 	u64 commitnum;
 	secp256k1_ecdsa_signature commit_sig;
@@ -1116,7 +1113,7 @@ int peer_got_commitsig(struct peer *peer, const u8 *msg)
 		peer_internal_error(peer,
 				    "bad fromwire_channel_got_commitsig %s",
 				    tal_hex(peer, msg));
-		return -1;
+		return;
 	}
 
 	log_debug(peer->log,
@@ -1134,28 +1131,28 @@ int peer_got_commitsig(struct peer *peer, const u8 *msg)
 	/* Save information now for fulfilled & failed HTLCs */
 	for (i = 0; i < tal_count(fulfilled); i++) {
 		if (!peer_fulfilled_our_htlc(peer, &fulfilled[i]))
-			return -1;
+			return;
 	}
 
 	for (i = 0; i < tal_count(failed); i++) {
 		if (!peer_failed_our_htlc(peer, &failed[i]))
-			return -1;
+			return;
 	}
 
 	for (i = 0; i < tal_count(changed); i++) {
 		if (!changed_htlc(peer, &changed[i])) {
 			peer_internal_error(peer,
 					    "got_commitsig: update failed");
-			return -1;
+			return;
 		}
 	}
 
 	/* Since we're about to send revoke, bump state again. */
 	if (!peer_sending_revocation(peer, added, fulfilled, failed, changed))
-		return -1;
+		return;
 
 	if (!peer_save_commitsig_received(peer, commitnum))
-		return -1;
+		return;
 
 	peer_last_tx(peer, tx, &commit_sig);
 	/* FIXME: Put these straight in the db! */
@@ -1165,7 +1162,6 @@ int peer_got_commitsig(struct peer *peer, const u8 *msg)
 	/* Tell it we've committed, and to go ahead with revoke. */
 	msg = towire_channel_got_commitsig_reply(msg);
 	subd_send_msg(peer->owner, take(msg));
-	return 0;
 }
 
 /* Shuffle them over, forgetting the ancient one. */
@@ -1177,7 +1173,7 @@ void update_per_commit_point(struct peer *peer,
 	ci->remote_per_commit = *per_commitment_point;
 }
 
-int peer_got_revoke(struct peer *peer, const u8 *msg)
+void peer_got_revoke(struct peer *peer, const u8 *msg)
 {
 	u64 revokenum;
 	struct sha256 per_commitment_secret;
@@ -1192,7 +1188,7 @@ int peer_got_revoke(struct peer *peer, const u8 *msg)
 					 &changed)) {
 		peer_internal_error(peer, "bad fromwire_channel_got_revoke %s",
 				    tal_hex(peer, msg));
-		return -1;
+		return;
 	}
 
 	log_debug(peer->log,
@@ -1206,12 +1202,12 @@ int peer_got_revoke(struct peer *peer, const u8 *msg)
 		if (changed[i].newstate == RCVD_ADD_ACK_REVOCATION) {
 			if (!peer_accepted_htlc(peer, changed[i].id,
 						&failcodes[i]))
-				return -1;
+				return;
 		} else {
 			if (!changed_htlc(peer, &changed[i])) {
 				peer_internal_error(peer,
 						    "got_revoke: update failed");
-				return -1;
+				return;
 			}
 		}
 	}
@@ -1219,14 +1215,14 @@ int peer_got_revoke(struct peer *peer, const u8 *msg)
 	if (revokenum >= (1ULL << 48)) {
 		peer_internal_error(peer, "got_revoke: too many txs %"PRIu64,
 				    revokenum);
-		return -1;
+		return;
 	}
 
 	if (revokenum != revocations_received(&peer->their_shachain.chain)) {
 		peer_internal_error(peer, "got_revoke: expected %"PRIu64
 				    " got %"PRIu64,
 				    revocations_received(&peer->their_shachain.chain), revokenum);
-		return -1;
+		return;
 	}
 
 	/* BOLT #2:
@@ -1243,7 +1239,7 @@ int peer_got_revoke(struct peer *peer, const u8 *msg)
 						   &per_commitment_secret),
 				    revokenum);
 		peer_fail_permanent(peer, take((u8 *)err));
-		return -1;
+		return;
 	}
 
 	/* FIXME: Check per_commitment_secret -> per_commit_point */
@@ -1269,8 +1265,6 @@ int peer_got_revoke(struct peer *peer, const u8 *msg)
 		fatal("Could not save channel to database: %s",
 		      peer->ld->wallet->db->err);
 	}
-
-	return 0;
 }
 
 static void *tal_arr_append_(void **p, size_t size)

lightningd/peer_htlcs.h

@@ -27,9 +27,9 @@ void peer_htlcs(const tal_t *ctx,
 		struct failed_htlc **failed_htlcs,
 		enum side **failed_sides);
 
-int peer_sending_commitsig(struct peer *peer, const u8 *msg);
-int peer_got_commitsig(struct peer *peer, const u8 *msg);
-int peer_got_revoke(struct peer *peer, const u8 *msg);
+void peer_sending_commitsig(struct peer *peer, const u8 *msg);
+void peer_got_commitsig(struct peer *peer, const u8 *msg);
+void peer_got_revoke(struct peer *peer, const u8 *msg);
 
 void update_per_commit_point(struct peer *peer,
 			     const struct pubkey *per_commitment_point);

lightningd/subd.c

@@ -36,7 +36,7 @@ struct subd_req {
 
 	/* Callback for a reply. */
 	int type;
-	bool (*replycb)(struct subd *, const u8 *, const int *, void *);
+	void (*replycb)(struct subd *, const u8 *, const int *, void *);
 	void *replycb_data;
 
 	size_t num_reply_fds;
@@ -53,7 +53,7 @@ static void free_subd_req(struct subd_req *sr)
 }
 
 /* Called when the callback is disabled because caller was freed. */
-static bool ignore_reply(struct subd *sd, const u8 *msg, const int *fds,
+static void ignore_reply(struct subd *sd, const u8 *msg, const int *fds,
 			 void *arg)
 {
 	size_t i;
@@ -61,7 +61,6 @@ static bool ignore_reply(struct subd *sd, const u8 *msg, const int *fds,
 	log_debug(sd->log, "IGNORING REPLY");
 	for (i = 0; i < tal_count(fds); i++)
 		close(fds[i]);
-	return true;
 }
 
 static void disable_cb(void *disabler, struct subd_req *sr)
@@ -72,7 +71,7 @@ static void disable_cb(void *disabler, struct subd_req *sr)
 
 static void add_req(const tal_t *ctx,
 		    struct subd *sd, int type, size_t num_fds_in,
-		    bool (*replycb)(struct subd *, const u8 *, const int *,
+		    void (*replycb)(struct subd *, const u8 *, const int *,
 				    void *),
 		    void *replycb_data)
 {
@@ -251,11 +250,16 @@ int subd_raw(struct lightningd *ld, const char *name)
 
 static struct io_plan *sd_msg_read(struct io_conn *conn, struct subd *sd);
 
+static void mark_freed(struct subd *unused, bool *freed)
+{
+	*freed = true;
+}
+
 static struct io_plan *sd_msg_reply(struct io_conn *conn, struct subd *sd,
 				    struct subd_req *sr)
 {
 	int type = fromwire_peektype(sd->msg_in);
-	bool keep_open;
+	bool freed = false;
 	const tal_t *tmpctx = tal_tmpctx(conn);
 	int *fds_in;
 
@@ -277,12 +281,16 @@ static struct io_plan *sd_msg_reply(struct io_conn *conn, struct subd *sd,
 	fds_in = tal_steal(tmpctx, sd->fds_in);
 	sd->fds_in = NULL;
 
-	keep_open = sr->replycb(sd, sd->msg_in, fds_in, sr->replycb_data);
+	/* Find out if they freed it. */
+	tal_add_destructor2(sd, mark_freed, &freed);
+	sr->replycb(sd, sd->msg_in, fds_in, sr->replycb_data);
 	tal_free(tmpctx);
 
-	if (!keep_open)
+	if (freed)
 		return io_close(conn);
 
+	tal_del_destructor2(sd, mark_freed, &freed);
+
 	/* Restore conn ptr. */
 	sd->conn = conn;
 	return io_read_wire(conn, sd, &sd->msg_in, sd_msg_read, sd);
@@ -449,13 +457,18 @@ static struct io_plan *sd_msg_read(struct io_conn *conn, struct subd *sd)
 
 	log_info(sd->log, "UPDATE %s", sd->msgname(type));
 	if (sd->msgcb) {
-		int i;
+		unsigned int i;
+		bool freed = false;
 
 		/* Might free sd (if returns negative); save/restore sd->conn */
 		sd->conn = NULL;
+		tal_add_destructor2(sd, mark_freed, &freed);
+
 		i = sd->msgcb(sd, sd->msg_in, sd->fds_in);
-		if (i < 0)
+		if (freed)
 			return io_close(conn);
+		tal_del_destructor2(sd, mark_freed, &freed);
+
 		sd->conn = conn;
 
 		if (i != 0) {
@@ -549,7 +562,8 @@ static struct subd *new_subd(struct lightningd *ld,
 			     const char *name,
 			     struct peer *peer,
 			     const char *(*msgname)(int msgtype),
-			     int (*msgcb)(struct subd *, const u8 *, const int *fds),
+			     unsigned int (*msgcb)(struct subd *,
+						   const u8 *, const int *fds),
 			     va_list *ap)
 {
 	struct subd *sd = tal(ld, struct subd);
@@ -586,8 +600,8 @@ static struct subd *new_subd(struct lightningd *ld,
 struct subd *new_global_subd(struct lightningd *ld,
 			     const char *name,
 			     const char *(*msgname)(int msgtype),
-			     int (*msgcb)(struct subd *, const u8 *,
-					  const int *fds),
+			     unsigned int (*msgcb)(struct subd *, const u8 *,
+						   const int *fds),
 			     ...)
 {
 	va_list ap;
@@ -605,8 +619,8 @@ struct subd *new_peer_subd(struct lightningd *ld,
 			   const char *name,
 			   struct peer *peer,
 			   const char *(*msgname)(int msgtype),
-			   int (*msgcb)(struct subd *, const u8 *,
-					const int *fds), ...)
+			   unsigned int (*msgcb)(struct subd *, const u8 *,
+						 const int *fds), ...)
 {
 	va_list ap;
 	struct subd *sd;
@@ -634,7 +648,7 @@ void subd_req_(const tal_t *ctx,
 	       struct subd *sd,
 	       const u8 *msg_out,
 	       int fd_out, size_t num_fds_in,
-	       bool (*replycb)(struct subd *, const u8 *, const int *, void *),
+	       void (*replycb)(struct subd *, const u8 *, const int *, void *),
 	       void *replycb_data)
 {
 	/* Grab type now in case msg_out is taken() */

lightningd/subd.h

@@ -32,7 +32,7 @@ struct subd {
 	struct log *log;
 
 	/* Callback when non-reply message comes in. */
-	int (*msgcb)(struct subd *, const u8 *, const int *);
+	unsigned (*msgcb)(struct subd *, const u8 *, const int *);
 	const char *(*msgname)(int msgtype);
 
 	/* Buffer for input. */
@@ -62,14 +62,14 @@ struct subd {
  *	(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
- * subdaemon is shutdown.
+ * that many @fds are received before calling again.  @msgcb can free subd
+ * to shut it down.
  */
 struct subd *new_global_subd(struct lightningd *ld,
 			     const char *name,
 			     const char *(*msgname)(int msgtype),
-			     int (*msgcb)(struct subd *, const u8 *,
-					  const int *fds),
+			     unsigned int (*msgcb)(struct subd *, const u8 *,
+						   const int *fds),
 			     ...);
 
 /**
@@ -90,8 +90,8 @@ struct subd *new_peer_subd(struct lightningd *ld,
 			   const char *name,
 			   struct peer *peer,
 			   const char *(*msgname)(int msgtype),
-			   int (*msgcb)(struct subd *, const u8 *,
-					const int *fds),
+			   unsigned int (*msgcb)(struct subd *, const u8 *,
+						 const int *fds),
 			   ...);
 
 /**
@@ -122,7 +122,7 @@ void subd_send_fd(struct subd *sd, int fd);
  * @msg_out: request message (can be take)
  * @fd_out: if >=0 fd to pass at the end of the message (closed after)
  * @num_fds_in: how many fds to read in to hand to @replycb if it's a reply.
- * @replycb: callback when reply comes in, returns false to shutdown daemon.
+ * @replycb: callback when reply comes in (can free subd)
  * @replycb_data: final arg to hand to @replycb
  *
  * @replycb cannot free @sd, so it returns false to remove it.
@@ -131,7 +131,7 @@ void subd_send_fd(struct subd *sd, int fd);
  */
 #define subd_req(ctx, sd, msg_out, fd_out, num_fds_in, replycb, replycb_data) \
 	subd_req_((ctx), (sd), (msg_out), (fd_out), (num_fds_in),	\
-		  typesafe_cb_preargs(bool, void *,			\
+		  typesafe_cb_preargs(void, void *,			\
 				      (replycb), (replycb_data),	\
 				      struct subd *,			\
 				      const u8 *, const int *),		\
@@ -140,7 +140,7 @@ void subd_req_(const tal_t *ctx,
 	       struct subd *sd,
 	       const u8 *msg_out,
 	       int fd_out, size_t num_fds_in,
-	       bool (*replycb)(struct subd *, const u8 *, const int *, void *),
+	       void (*replycb)(struct subd *, const u8 *, const int *, void *),
 	       void *replycb_data);
 
 /**