gossipd: clean up getnodes handling.

globalfeatures should not be accessed if we haven't received a
channel_update.  Treat it like the other fields which are only
initialized and marshalled/unmarshalled if the timestamp is positive.

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

gossipd/gossipd.c

@@ -1429,30 +1429,25 @@ static struct io_plan *getchannels_req(struct io_conn *conn, struct daemon *daem
 	return daemon_conn_read_next(conn, &daemon->master);
 }
 
-static void append_node(const struct gossip_getnodes_entry ***nodes,
+/* We keep pointers into n, assuming it won't change! */
-			const struct pubkey *nodeid,
+static void append_node(const struct gossip_getnodes_entry ***entries,
-			const u8 *globalfeatures,
-			/* If non-NULL, contains more information */
 			const struct node *n)
 {
-	struct gossip_getnodes_entry *new;
+	struct gossip_getnodes_entry *e;
 
-	new = tal(*nodes, struct gossip_getnodes_entry);
+	*tal_arr_expand(entries) = e
-	new->nodeid = *nodeid;
+		= tal(*entries, struct gossip_getnodes_entry);
-	new->globalfeatures = tal_dup_arr(*nodes, u8, globalfeatures,
+	e->nodeid = n->id;
-					  tal_count(globalfeatures), 0);
+	e->last_timestamp = n->last_timestamp;
-	if (!n || n->last_timestamp < 0) {
+	if (e->last_timestamp < 0)
-		new->last_timestamp = -1;
+		return;
-		new->addresses = NULL;
+
-	} else {
+	e->globalfeatures = n->globalfeatures;
-		new->last_timestamp = n->last_timestamp;
+	e->addresses = n->addresses;
-		new->addresses = n->addresses;
+	BUILD_ASSERT(ARRAY_SIZE(e->alias) == ARRAY_SIZE(n->alias));
-		BUILD_ASSERT(ARRAY_SIZE(new->alias) == ARRAY_SIZE(n->alias));
+	BUILD_ASSERT(ARRAY_SIZE(e->color) == ARRAY_SIZE(n->rgb_color));
-		BUILD_ASSERT(ARRAY_SIZE(new->color) == ARRAY_SIZE(n->rgb_color));
+	memcpy(e->alias, n->alias, ARRAY_SIZE(e->alias));
-		memcpy(new->alias, n->alias, ARRAY_SIZE(new->alias));
+	memcpy(e->color, n->rgb_color, ARRAY_SIZE(e->color));
-		memcpy(new->color, n->rgb_color, ARRAY_SIZE(new->color));
-	}
-	*tal_arr_expand(nodes) = new;
 }
 
 static struct io_plan *getnodes(struct io_conn *conn, struct daemon *daemon,
@@ -1469,12 +1464,12 @@ static struct io_plan *getnodes(struct io_conn *conn, struct daemon *daemon,
 	if (id) {
 		n = get_node(daemon->rstate, id);
 		if (n)
-			append_node(&nodes, id, n->globalfeatures, n);
+			append_node(&nodes, n);
 	} else {
 		struct node_map_iter i;
 		n = node_map_first(daemon->rstate->nodes, &i);
 		while (n != NULL) {
-			append_node(&nodes, &n->id, n->globalfeatures, n);
+			append_node(&nodes, n);
 			n = node_map_next(daemon->rstate->nodes, &i);
 		}
 	}

lightningd/gossip_msg.c

@@ -14,15 +14,16 @@ struct gossip_getnodes_entry *fromwire_gossip_getnodes_entry(const tal_t *ctx,
 	entry = tal(ctx, struct gossip_getnodes_entry);
 	fromwire_pubkey(pptr, max, &entry->nodeid);
 
-	flen = fromwire_u16(pptr, max);
-	entry->globalfeatures = tal_arr(entry, u8, flen);
-	fromwire_u8_array(pptr, max, entry->globalfeatures, flen);
-
 	entry->last_timestamp = fromwire_u64(pptr, max);
 	if (entry->last_timestamp < 0) {
 		entry->addresses = NULL;
 		return entry;
 	}
+
+	flen = fromwire_u16(pptr, max);
+	entry->globalfeatures = tal_arr(entry, u8, flen);
+	fromwire_u8_array(pptr, max, entry->globalfeatures, flen);
+
 	numaddresses = fromwire_u8(pptr, max);
 
 	entry->addresses = tal_arr(entry, struct wireaddr, numaddresses);
@@ -42,18 +43,17 @@ struct gossip_getnodes_entry *fromwire_gossip_getnodes_entry(const tal_t *ctx,
 void towire_gossip_getnodes_entry(u8 **pptr,
 				  const struct gossip_getnodes_entry *entry)
 {
-	u8 i, numaddresses = tal_count(entry->addresses);
 	towire_pubkey(pptr, &entry->nodeid);
-	towire_u16(pptr, tal_count(entry->globalfeatures));
-	towire_u8_array(pptr, entry->globalfeatures,
-			tal_count(entry->globalfeatures));
 	towire_u64(pptr, entry->last_timestamp);
 
 	if (entry->last_timestamp < 0)
 		return;
 
-	towire_u8(pptr, numaddresses);
+	towire_u16(pptr, tal_count(entry->globalfeatures));
-	for (i=0; i<numaddresses; i++) {
+	towire_u8_array(pptr, entry->globalfeatures,
+			tal_count(entry->globalfeatures));
+	towire_u8(pptr, tal_count(entry->addresses));
+	for (size_t i = 0; i < tal_count(entry->addresses); i++) {
 		towire_wireaddr(pptr, &entry->addresses[i]);
 	}
 	towire(pptr, entry->alias, ARRAY_SIZE(entry->alias));

lightningd/gossip_msg.h

@@ -13,8 +13,8 @@ struct peer_features {
 
 struct gossip_getnodes_entry {
 	struct pubkey nodeid;
-	u8 *globalfeatures;
 	s64 last_timestamp; /* -1 means never: following fields ignored */
+	u8 *globalfeatures;
 	struct wireaddr *addresses;
 	u8 alias[32];
 	u8 color[3];