common/memleak: implement callback arg for dump_memleak.

This makes it easier to use outside simple subds, and now lightningd can
simply dump to log rather than returning JSON.

JSON formatting was a lot of work, and we only did it for lightningd, not for
subdaemons.  Easier to use the logs in all cases.

Signed-off-by: Rusty Russell <[email protected]>

channeld/channeld.c

@@ -5585,7 +5585,7 @@ static void handle_dev_memleak(struct peer *peer, const u8 *msg)
 	/* Now delete peer and things it has pointers to. */
 	memleak_scan_obj(memtable, peer);
 
-	found_leak = dump_memleak(memtable, memleak_status_broken);
+	found_leak = dump_memleak(memtable, memleak_status_broken, NULL);
 	wire_sync_write(MASTER_FD,
 			 take(towire_channeld_dev_memleak_reply(NULL,
 							       found_leak)));

closingd/closingd.c

@@ -559,7 +559,7 @@ static void closing_dev_memleak(const tal_t *ctx,
 	memleak_ptr(memtable, scriptpubkey[REMOTE]);
 	memleak_ptr(memtable, funding_wscript);
 
-	dump_memleak(memtable, memleak_status_broken);
+	dump_memleak(memtable, memleak_status_broken, NULL);
 }
 
 /* Figure out what weight we actually expect for this closing tx (using zero fees

common/memleak.c

@@ -199,7 +199,7 @@ static bool ptr_match(const void *candidate, void *ptr)
 	return candidate == ptr;
 }
 
-const void *memleak_get(struct htable *memtable, const uintptr_t **backtrace)
+static const void *memleak_get(struct htable *memtable, const uintptr_t **backtrace)
 {
 	struct htable_iter it;
 	const tal_t *i, *p;
@@ -330,51 +330,61 @@ void memleak_init(void)
 	}
 }
 
+struct print_and_arg {
+	void PRINTF_FMT(2,3) (*print)(void *arg, const char *fmt, ...);
+	void *arg;
+};
+
 static int dump_syminfo(void *data, uintptr_t pc UNUSED,
 			const char *filename, int lineno,
 			const char *function)
 {
-	void PRINTF_FMT(1,2) (*print)(const char *fmt, ...) = data;
+	struct print_and_arg *pag = data;
 	/* This can happen in backtraces. */
 	if (!filename || !function)
 		return 0;
 
-	print("    %s:%u (%s)", filename, lineno, function);
+	pag->print(pag->arg, "    %s:%u (%s)", filename, lineno, function);
 	return 0;
 }
 
 static void dump_leak_backtrace(const uintptr_t *bt,
-				void PRINTF_FMT(1,2)
-				(*print)(const char *fmt, ...))
+				void (*print)(void *arg, const char *fmt, ...),
+				void *arg)
 {
+	struct print_and_arg pag;
+
 	if (!bt)
 		return;
 
+	pag.print = print;
+	pag.arg = arg;
 	/* First one serves as counter. */
-	print("  backtrace:");
+	print(arg, "  backtrace:");
 	for (size_t i = 1; i < bt[0]; i++) {
 		backtrace_pcinfo(backtrace_state,
 				 bt[i], dump_syminfo,
-				 NULL, print);
+				 NULL, &pag);
 	}
 }
 
-bool dump_memleak(struct htable *memtable,
-		  void PRINTF_FMT(1,2) (*print)(const char *fmt, ...))
+bool dump_memleak_(struct htable *memtable,
+		   void PRINTF_FMT(2,3) (*print)(void *arg, const char *fmt, ...),
+		   void *arg)
 {
 	const tal_t *i;
 	const uintptr_t *backtrace;
 	bool found_leak = false;
 
 	while ((i = memleak_get(memtable, &backtrace)) != NULL) {
-		print("MEMLEAK: %p", i);
+		print(arg, "MEMLEAK: %p", i);
 		if (tal_name(i))
-			print("  label=%s", tal_name(i));
+			print(arg, "  label=%s", tal_name(i));
 
-		dump_leak_backtrace(backtrace, print);
-		print("  parents:");
+		dump_leak_backtrace(backtrace, print, arg);
+		print(arg, "  parents:");
 		for (tal_t *p = tal_parent(i); p; p = tal_parent(p)) {
-			print("    %s", tal_name(p));
+			print(arg, "    %s", tal_name(p));
 			p = tal_parent(p);
 		}
 		found_leak = true;

common/memleak.h

@@ -3,6 +3,7 @@
 #include "config.h"
 #include <ccan/strmap/strmap.h>
 #include <ccan/tal/tal.h>
+#include <ccan/typesafe_cb/typesafe_cb.h>
 #include <inttypes.h>
 
 struct htable;
@@ -131,18 +132,22 @@ void memleak_scan_strmap_(struct htable *memtable, const struct strmap *m);
 void memleak_ignore_children(struct htable *memtable, const void *p);
 
 /**
- * memleak_get: get (and remove) a leak from memtable, or NULL
+ * dump_memleak: use print function to dump memleak details
  * @memtable: the memtable after all known allocations removed.
- * @backtrace: the backtrace to set if there is one.
+ * @print: the printf-style function to use (takes @arg first)
+ * @arg: the arg for @print
  *
- * If this returns NULL, it means the @memtable was empty.  Otherwise
- * it return a pointer to a leak (and removes it from @memtable)
+ * Returns true if there was a leak.
  */
-const void *memleak_get(struct htable *memtable, const uintptr_t **backtrace);
+#define dump_memleak(memtable, print, arg) \
+	dump_memleak_((memtable),					\
+		      typesafe_cb_postargs(void, void *, (print), (arg), const char *, ...), \
+		      (arg))
 
-extern struct backtrace_state *backtrace_state;
+bool dump_memleak_(struct htable *memtable,
+		   void PRINTF_FMT(2,3) (*print)(void *arg, const char *fmt, ...),
+		   void *arg);
 
-bool dump_memleak(struct htable *memtable,
-		  void PRINTF_FMT(1,2) (*print)(const char *fmt, ...));
+extern struct backtrace_state *backtrace_state;
 
 #endif /* LIGHTNING_COMMON_MEMLEAK_H */

common/status.c

@@ -235,7 +235,7 @@ void master_badmsg(u32 type_expected, const u8 *msg)
 }
 
 /* Print BROKEN status: callback for dump_memleak. */
-void memleak_status_broken(const char *fmt, ...)
+void memleak_status_broken(void *unused, const char *fmt, ...)
 {
 	va_list ap;
 	va_start(ap, fmt);

common/status.h

@@ -71,6 +71,6 @@ void status_send_fatal(const u8 *msg TAKES) NORETURN;
 void status_send_fd(int fd);
 
 /* Print BROKEN status: callback for dump_memleak. */
-void memleak_status_broken(const char *fmt, ...);
+void memleak_status_broken(void *unused, const char *fmt, ...);
 
 #endif /* LIGHTNING_COMMON_STATUS_H */

connectd/connectd.c

@@ -1861,7 +1861,7 @@ static void dev_connect_memleak(struct daemon *daemon, const u8 *msg)
 	memleak_scan_obj(memtable, daemon);
 	memleak_scan_htable(memtable, &daemon->peers->raw);
 
-	found_leak = dump_memleak(memtable, memleak_status_broken);
+	found_leak = dump_memleak(memtable, memleak_status_broken, NULL);
 	daemon_conn_send(daemon->master,
 			 take(towire_connectd_dev_memleak_reply(NULL,
 							      found_leak)));

gossipd/gossipd.c

@@ -966,7 +966,7 @@ static void dev_gossip_memleak(struct daemon *daemon, const u8 *msg)
 	memleak_scan_obj(memtable, daemon);
 	memleak_scan_htable(memtable, &daemon->peers->raw);
 
-	found_leak = dump_memleak(memtable, memleak_status_broken);
+	found_leak = dump_memleak(memtable, memleak_status_broken, NULL);
 	daemon_conn_send(daemon->master,
 			 take(towire_gossipd_dev_memleak_reply(NULL,
 							      found_leak)));

hsmd/hsmd.c

@@ -573,7 +573,7 @@ static struct io_plan *handle_memleak(struct io_conn *conn,
 	memleak_ptr(memtable, dev_force_privkey);
 	memleak_ptr(memtable, dev_force_bip32_seed);
 
-	found_leak = dump_memleak(memtable, memleak_status_broken);
+	found_leak = dump_memleak(memtable, memleak_status_broken, NULL);
 	reply = towire_hsmd_dev_memleak_reply(NULL, found_leak);
 	return req_reply(conn, c, take(reply));
 }

lightningd/memdump.c

@@ -88,46 +88,17 @@ static const struct json_command dev_memdump_command = {
 };
 AUTODATA(json_command, &dev_memdump_command);
 
-static int json_add_syminfo(void *data, uintptr_t pc UNUSED,
-			    const char *filename, int lineno,
-			    const char *function)
+static void memleak_log(struct logger *log, const char *fmt, ...)
 {
-	struct json_stream *response = data;
-	char *str;
-
-	/* This can happen in backtraces. */
-	if (!filename || !function)
-		return 0;
-
-	str = tal_fmt(response, "%s:%u (%s)", filename, lineno, function);
-	json_add_string(response, NULL, str);
-	tal_free(str);
-	return 0;
-}
-
-static void json_add_backtrace(struct json_stream *response,
-			       const uintptr_t *bt)
-{
-	size_t i;
-
-	if (!bt)
-		return;
-
-	json_array_start(response, "backtrace");
-	/* First one serves as counter. */
-	for (i = 1; i < bt[0]; i++) {
-		backtrace_pcinfo(backtrace_state,
-				 bt[i], json_add_syminfo,
-				 NULL, response);
-	}
-	json_array_end(response);
+	va_list ap;
+	va_start(ap, fmt);
+	logv(log, LOG_BROKEN, NULL, true, fmt, ap);
+	va_end(ap);
 }
 
 static void finish_report(const struct leak_detect *leaks)
 {
 	struct htable *memtable;
-	const tal_t *i;
-	const uintptr_t *backtrace;
 	struct command *cmd;
 	struct lightningd *ld;
 	struct json_stream *response;
@@ -162,24 +133,11 @@ static void finish_report(const struct leak_detect *leaks)
 	/* Now delete ld and those which it has pointers to. */
 	memleak_scan_obj(memtable, ld);
 
+	if (dump_memleak(memtable, memleak_log, ld->log))
+		tal_arr_expand(&leaks->leakers, "lightningd");
+
 	response = json_stream_success(cmd);
 	json_array_start(response, "leaks");
-	while ((i = memleak_get(memtable, &backtrace)) != NULL) {
-		const tal_t *p;
-
-		json_object_start(response, NULL);
-		json_add_ptr(response, "value", i);
-		if (tal_name(i))
-			json_add_string(response, "label", tal_name(i));
-
-		json_add_backtrace(response, backtrace);
-		json_array_start(response, "parents");
-		for (p = tal_parent(i); p; p = tal_parent(p))
-			json_add_string(response, NULL, tal_name(p));
-		json_array_end(response);
-		json_object_end(response);
-	}
-
 	for (size_t num_leakers = 0;
 	     num_leakers < tal_count(leaks->leakers);
 	     num_leakers++) {

onchaind/onchaind.c

@@ -1606,7 +1606,7 @@ static void handle_dev_memleak(struct tracked_output ***outs, const u8 *msg)
 	memleak_remove_globals(memtable, tal_parent(*outs));
 	memleak_scan_obj(memtable, *outs);
 
-	found_leak = dump_memleak(memtable, memleak_status_broken);
+	found_leak = dump_memleak(memtable, memleak_status_broken, NULL);
 	wire_sync_write(REQ_FD,
 			take(towire_onchaind_dev_memleak_reply(NULL,
 							      found_leak)));

onchaind/test/run-grind_feerate-bug.c

@@ -30,10 +30,11 @@ bool derive_keyset(const struct pubkey *per_commitment_point UNNEEDED,
 		   bool option_static_remotekey UNNEEDED,
 		   struct keyset *keyset UNNEEDED)
 { fprintf(stderr, "derive_keyset called!\n"); abort(); }
-/* Generated stub for dump_memleak */
-bool dump_memleak(struct htable *memtable UNNEEDED,
-		  void  (*print)(const char *fmt UNNEEDED, ...))
-{ fprintf(stderr, "dump_memleak called!\n"); abort(); }
+/* Generated stub for dump_memleak_ */
+bool dump_memleak_(struct htable *memtable UNNEEDED,
+		   void  (*print)(void *arg UNNEEDED, const char *fmt UNNEEDED, ...) UNNEEDED,
+		   void *arg UNNEEDED)
+{ fprintf(stderr, "dump_memleak_ called!\n"); abort(); }
 /* Generated stub for fromwire_basepoints */
 void fromwire_basepoints(const u8 **ptr UNNEEDED, size_t *max UNNEEDED,
 			 struct basepoints *b UNNEEDED)
@@ -101,7 +102,7 @@ void memleak_scan_obj(struct htable *memtable UNNEEDED, const void *obj UNNEEDED
 struct htable *memleak_start(const tal_t *ctx UNNEEDED)
 { fprintf(stderr, "memleak_start called!\n"); abort(); }
 /* Generated stub for memleak_status_broken */
-void memleak_status_broken(const char *fmt UNNEEDED, ...)
+void memleak_status_broken(void *unused UNNEEDED, const char *fmt UNNEEDED, ...)
 { fprintf(stderr, "memleak_status_broken called!\n"); abort(); }
 /* Generated stub for new_coin_channel_close */
 struct chain_coin_mvt *new_coin_channel_close(const tal_t *ctx UNNEEDED,

onchaind/test/run-grind_feerate.c

@@ -29,10 +29,11 @@ bool derive_keyset(const struct pubkey *per_commitment_point UNNEEDED,
 		   bool option_static_remotekey UNNEEDED,
 		   struct keyset *keyset UNNEEDED)
 { fprintf(stderr, "derive_keyset called!\n"); abort(); }
-/* Generated stub for dump_memleak */
-bool dump_memleak(struct htable *memtable UNNEEDED,
-		  void  (*print)(const char *fmt UNNEEDED, ...))
-{ fprintf(stderr, "dump_memleak called!\n"); abort(); }
+/* Generated stub for dump_memleak_ */
+bool dump_memleak_(struct htable *memtable UNNEEDED,
+		   void  (*print)(void *arg UNNEEDED, const char *fmt UNNEEDED, ...) UNNEEDED,
+		   void *arg UNNEEDED)
+{ fprintf(stderr, "dump_memleak_ called!\n"); abort(); }
 /* Generated stub for fromwire */
 const u8 *fromwire(const u8 **cursor UNNEEDED, size_t *max UNNEEDED, void *copy UNNEEDED, size_t n UNNEEDED)
 { fprintf(stderr, "fromwire called!\n"); abort(); }
@@ -148,7 +149,7 @@ void memleak_scan_obj(struct htable *memtable UNNEEDED, const void *obj UNNEEDED
 struct htable *memleak_start(const tal_t *ctx UNNEEDED)
 { fprintf(stderr, "memleak_start called!\n"); abort(); }
 /* Generated stub for memleak_status_broken */
-void memleak_status_broken(const char *fmt UNNEEDED, ...)
+void memleak_status_broken(void *unused UNNEEDED, const char *fmt UNNEEDED, ...)
 { fprintf(stderr, "memleak_status_broken called!\n"); abort(); }
 /* Generated stub for new_coin_channel_close */
 struct chain_coin_mvt *new_coin_channel_close(const tal_t *ctx UNNEEDED,

openingd/dualopend.c

@@ -976,7 +976,7 @@ static void handle_dev_memleak(struct state *state, const u8 *msg)
 	memleak_scan_obj(memtable, state);
 
 	/* If there's anything left, dump it to logs, and return true. */
-	found_leak = dump_memleak(memtable, memleak_status_broken);
+	found_leak = dump_memleak(memtable, memleak_status_broken, NULL);
 	wire_sync_write(REQ_FD,
 			take(towire_dualopend_dev_memleak_reply(NULL,
 							        found_leak)));

openingd/openingd.c

@@ -1419,7 +1419,7 @@ static void handle_dev_memleak(struct state *state, const u8 *msg)
 	memleak_scan_obj(memtable, state);
 
 	/* If there's anything left, dump it to logs, and return true. */
-	found_leak = dump_memleak(memtable, memleak_status_broken);
+	found_leak = dump_memleak(memtable, memleak_status_broken, NULL);
 	wire_sync_write(REQ_FD,
 			take(towire_openingd_dev_memleak_reply(NULL,
 							      found_leak)));

plugins/libplugin.c

@@ -1513,14 +1513,12 @@ void plugin_log(struct plugin *p, enum log_level l, const char *fmt, ...)
 	va_end(ap);
 }
 
-/* Hack since we have no extra ptr to log_memleak */
-static struct plugin *memleak_plugin;
-static void PRINTF_FMT(1,2) log_memleak(const char *fmt, ...)
+static void PRINTF_FMT(2,3) log_memleak(struct plugin *plugin, const char *fmt, ...)
 {
 	va_list ap;
 
 	va_start(ap, fmt);
-	plugin_logv(memleak_plugin, LOG_BROKEN, fmt, ap);
+	plugin_logv(plugin, LOG_BROKEN, fmt, ap);
 	va_end(ap);
 }
 
@@ -1546,8 +1544,7 @@ static void memleak_check(struct plugin *plugin, struct command *cmd)
 	if (plugin->mark_mem)
 		plugin->mark_mem(plugin, memtable);
 
-	memleak_plugin = plugin;
-	dump_memleak(memtable, log_memleak);
+	dump_memleak(memtable, log_memleak, plugin);
 }
 
 void plugin_set_memleak_handler(struct plugin *plugin,