common/gossmap: remove open-by-fd.

We only use it in one place, and that was simply to share an fd between
gossipd writing and gossipd reading, which may be causing our zfs problem
anyway.

In fact, it fixes a race if we don't have HAVE_PWRITEV.

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

common/gossmap.c

@@ -55,7 +55,6 @@ struct gossmap {
 
 	/* The file descriptor and filename to monitor */
 	int fd;
-	/* NULL means we don't own fd */
 	const char *fname;
 
 	/* The memory map of the file: u8 for arithmetic portability */
@@ -661,9 +660,6 @@ static bool reopen_store(struct gossmap *map, u64 ended_off)
 {
 	int fd;
 
-	if (!map->fname)
-		errx(1, "Need to reopen, but not fname!");
-
 	fd = open(map->fname, O_RDONLY);
 	if (fd < 0)
 		err(1, "Failed to reopen %s", map->fname);
@@ -717,7 +713,7 @@ static bool map_catchup(struct gossmap *map, bool *changed)
 			remove_channel_by_deletemsg(map, off);
 		else if (type == WIRE_NODE_ANNOUNCEMENT)
 			node_announcement(map, off);
-		else if (type == WIRE_GOSSIP_STORE_ENDED && map->fname) {
+		else if (type == WIRE_GOSSIP_STORE_ENDED) {
 			/* This can recurse! */
 			if (!reopen_store(map, off))
 				return false;
@@ -788,8 +784,7 @@ static void destroy_map(struct gossmap *map)
 	for (size_t i = 0; i < tal_count(map->node_arr); i++)
 		free(map->node_arr[i].chan_idxs);
 
-	if (map->fname)
-		close(map->fd);
+	close(map->fd);
 }
 
 /* Local modifications.  We only expect a few, so we use a simple
@@ -1245,33 +1240,6 @@ struct gossmap *gossmap_load(const tal_t *ctx, const char *filename,
 	return map;
 }
 
-struct gossmap *gossmap_load_fd_(const tal_t *ctx, int fd,
-				 void (*cupdate_fail)(struct gossmap *map,
-						      const struct short_channel_id_dir *scidd,
-						      u16 cltv_expiry_delta,
-						      u32 fee_base_msat,
-						      u32 fee_proportional_millionths,
-						      void *cb_arg),
-				 bool (*unknown_record)(struct gossmap *map,
-							int type,
-							u64 off,
-							size_t msglen,
-							void *cb_arg),
-				 void *cb_arg)
-{
-	map = tal(ctx, struct gossmap);
-	map->fname = NULL;
-	map->fd = fd;
-	map->cupdate_fail = cupdate_fail;
-	map->unknown_record = unknown_record;
-	map->cb_arg = cb_arg;
-	tal_add_destructor(map, destroy_map);
-
-	if (!load_gossip_store(map))
-		return tal_free(map);
-	return map;
-}
-
 void gossmap_node_get_id(const struct gossmap *map,
 			 const struct gossmap_node *node,
 			 struct node_id *id)

common/gossmap.h

@@ -45,37 +45,6 @@ struct gossmap_chan {
 struct gossmap *gossmap_load(const tal_t *ctx, const char *filename,
 			     size_t *num_channel_updates_rejected);
 
-/* Version which uses existing fd */
-#define gossmap_load_fd(ctx, fd, cupdate_fail, unknown_record, cbarg)	\
-	gossmap_load_fd_((ctx), (fd),					\
-			 typesafe_cb_preargs(void, void *, (cupdate_fail), (cbarg), \
-					     struct gossmap *,		\
-					     const struct short_channel_id_dir *, \
-					     u16 cltv_expiry_delta,	\
-					     u32 fee_base_msat,		\
-					     u32 fee_proportional_millionths), \
-			 typesafe_cb_preargs(bool, void *, (unknown_record), (cbarg), \
-					     struct gossmap *,		\
-					     int type,			\
-					     u64 off,			\
-					     size_t msglen),		\
-			 (cbarg))
-
-struct gossmap *gossmap_load_fd_(const tal_t *ctx, int fd,
-				 void (*cupdate_fail)(struct gossmap *map,
-						      const struct short_channel_id_dir *scidd,
-						      u16 cltv_expiry_delta,
-						      u32 fee_base_msat,
-						      u32 fee_proportional_millionths,
-						      void *cb_arg),
-				 bool (*unknown_record)(struct gossmap *map,
-							int type,
-							u64 off,
-							size_t msglen,
-							void *cb_arg),
-				 void *cb_arg);
-
-
 /* Call this before using to ensure it's up-to-date.  Returns true if something
  * was updated. Note: this can scramble node and chan indexes! */
 bool gossmap_refresh(struct gossmap *map, size_t *num_channel_updates_rejected);

devtools/gossmap-compress.c

@@ -516,21 +516,9 @@ static const char *get_alias(const tal_t *ctx,
 	return tal_strndup(ctx, (const char *)alias, 32);
 }
 
-static void cupdate_bad(struct gossmap *map,
-			const struct short_channel_id_dir *scidd,
-			u16 cltv_expiry_delta,
-			u32 fee_base_msat,
-			u32 fee_proportional_millionths,
-			void *unused)
-{
-	warnx("Bad cupdate for %s, ignoring (delta=%u, fee=%u/%u)",
-	      fmt_short_channel_id_dir(tmpctx, scidd),
-	      cltv_expiry_delta, fee_base_msat, fee_proportional_millionths);
-}
-
 int main(int argc, char *argv[])
 {
-	int infd, outfd;
+	int outfd;
 	const struct pubkey **node_ids;
 	bool print_nodes = false;
 
@@ -553,10 +541,6 @@ int main(int argc, char *argv[])
 	if (argc != 4)
 		opt_usage_exit_fail("Needs 4 arguments");
 
-	infd = open(argv[2], O_RDONLY);
-	if (infd < 0)
-		opt_usage_exit_fail(tal_fmt(tmpctx, "Cannot open %s for reading: %s",
-					    argv[2], strerror(errno)));
 	outfd = open(argv[3], O_WRONLY|O_CREAT|O_TRUNC, 0666);
 	if (outfd < 0)
 		opt_usage_exit_fail(tal_fmt(tmpctx, "Cannot open %s for writing: %s",
@@ -570,9 +554,10 @@ int main(int argc, char *argv[])
 		bool *dirs;
 		gzFile outf = gzdopen(outfd, "wb9");
 
-		struct gossmap *gossmap = gossmap_load_fd(tmpctx, infd, cupdate_bad, NULL, NULL);
+		struct gossmap *gossmap = gossmap_load(tmpctx, argv[2], NULL);
 		if (!gossmap)
-			opt_usage_and_exit("Cannot read gossmap");
+			opt_usage_exit_fail(tal_fmt(tmpctx, "Cannot open %s for reading: %s",
+						    argv[2], strerror(errno)));
 
 		nodes = tal_arr(gossmap, struct gossmap_node *, gossmap_max_node_idx(gossmap));
 		for (node_count = 0, n = gossmap_first_node(gossmap);
@@ -713,7 +698,11 @@ int main(int argc, char *argv[])
 		} *chans;
 		const u8 version = GOSSIP_STORE_VER;
 		size_t disabled_count, node_limit;
-		gzFile inf = gzdopen(infd, "rb");
+		gzFile inf = gzopen(argv[2], "rb");
+
+		if (!inf)
+			opt_usage_exit_fail(tal_fmt(tmpctx, "Cannot open %s for reading: %s",
+						    argv[2], strerror(errno)));
 
 		if (gzread(inf, hdr, sizeof(hdr)) != sizeof(hdr)
 		    || !memeq(hdr, sizeof(hdr), GC_HEADER, GC_HEADERLEN))

gossipd/gossip_store.c

@@ -427,11 +427,6 @@ struct gossip_store *gossip_store_new(const tal_t *ctx,
 	return gs;
 }
 
-int gossip_store_get_fd(const struct gossip_store *gs)
-{
-	return gs->fd;
-}
-
 u64 gossip_store_add(struct gossip_store *gs, const u8 *gossip_msg, u32 timestamp)
 {
 	u64 off = gs->len;

gossipd/gossip_store.h

@@ -37,14 +37,6 @@ struct gossip_store *gossip_store_new(const tal_t *ctx,
 				      bool *populated,
 				      struct chan_dying **dying);
 
-/**
- * Get the fd from the gossip_store.
- * @gs: the gossip_store.
- *
- * Used by gossmap_manage to create a gossmap.
- */
-int gossip_store_get_fd(const struct gossip_store *gs);
-
 /**
  * Get the (refreshed!) gossmap from the gossip_store.
  * @gs: gossip store

gossipd/gossmap_manage.c

@@ -407,26 +407,13 @@ static void start_prune_timer(struct gossmap_manage *gm)
 
 static void reprocess_queued_msgs(struct gossmap_manage *gm);
 
-static void report_bad_update(struct gossmap *map,
-			      const struct short_channel_id_dir *scidd,
-			      u16 cltv_expiry_delta,
-			      u32 fee_base_msat,
-			      u32 fee_proportional_millionths,
-			      struct gossmap_manage *gm)
-{
-	status_debug("Update for %s has silly values, disabling (cltv=%u, fee=%u+%u)",
-		     fmt_short_channel_id_dir(tmpctx, scidd),
-		     cltv_expiry_delta, fee_base_msat, fee_proportional_millionths);
-}
-
 struct gossmap_manage *gossmap_manage_new(const tal_t *ctx,
 					  struct daemon *daemon,
 					  struct chan_dying *dying_channels TAKES)
 {
 	struct gossmap_manage *gm = tal(ctx, struct gossmap_manage);
 
-	gm->fd = gossip_store_get_fd(daemon->gs);
-	gm->raw_gossmap = gossmap_load_fd(gm, gm->fd, report_bad_update, NULL, gm);
+	gm->raw_gossmap = gossmap_load(gm, GOSSIP_STORE_FILENAME, NULL);
 	assert(gm->raw_gossmap);
 	gm->daemon = daemon;