From 11bfbf5debee445e1a1ee9ac86f24aa891b09023 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 7 Aug 2024 11:19:52 +0930 Subject: [PATCH] libplugin: add data pointer for plugin convenience. This avoids globals (and means memleak traverses the variables!): we only change over the test plugin though, to avoid unnecessary churn. Signed-off-by: Rusty Russell --- plugins/autoclean.c | 3 +- plugins/bcli.c | 2 +- plugins/bkpr/bookkeeper.c | 2 +- plugins/chanbackup.c | 2 +- plugins/commando.c | 2 +- plugins/funder.c | 2 +- plugins/keysend.c | 2 +- plugins/libplugin.c | 17 +++++++++++ plugins/libplugin.h | 6 ++++ plugins/offers.c | 2 +- plugins/pay.c | 2 +- plugins/recover.c | 2 +- plugins/renepay/main.c | 2 +- plugins/spender/main.c | 2 +- plugins/sql.c | 2 +- plugins/topology.c | 2 +- plugins/txprepare.c | 2 +- tests/plugins/test_libplugin.c | 53 ++++++++++++++++++++++++---------- 18 files changed, 77 insertions(+), 30 deletions(-) diff --git a/plugins/autoclean.c b/plugins/autoclean.c index 92c86fe2a..f1e2e5198 100644 --- a/plugins/autoclean.c +++ b/plugins/autoclean.c @@ -805,7 +805,8 @@ int main(int argc, char *argv[]) setup_locale(); timer_cinfo = new_clean_info(NULL, NULL); - plugin_main(argv, init, PLUGIN_STATIC, true, NULL, commands, ARRAY_SIZE(commands), + plugin_main(argv, init, NULL, PLUGIN_STATIC, true, NULL, + commands, ARRAY_SIZE(commands), NULL, 0, NULL, 0, NULL, 0, plugin_option_dynamic("autoclean-cycle", "int", diff --git a/plugins/bcli.c b/plugins/bcli.c index 27cfb6a4f..b169a74df 100644 --- a/plugins/bcli.c +++ b/plugins/bcli.c @@ -1171,7 +1171,7 @@ int main(int argc, char *argv[]) /* Initialize our global context object here to handle startup options. */ bitcoind = new_bitcoind(NULL); - plugin_main(argv, init, PLUGIN_STATIC, false /* Do not init RPC on startup*/, + plugin_main(argv, init, NULL, PLUGIN_STATIC, false /* Do not init RPC on startup*/, NULL, commands, ARRAY_SIZE(commands), NULL, 0, NULL, 0, NULL, 0, plugin_option("bitcoin-datadir", diff --git a/plugins/bkpr/bookkeeper.c b/plugins/bkpr/bookkeeper.c index 0bb4b9cc3..4735e3359 100644 --- a/plugins/bkpr/bookkeeper.c +++ b/plugins/bkpr/bookkeeper.c @@ -1809,7 +1809,7 @@ int main(int argc, char *argv[]) datadir = NULL; db_dsn = NULL; - plugin_main(argv, init, PLUGIN_STATIC, true, NULL, + plugin_main(argv, init, NULL, PLUGIN_STATIC, true, NULL, commands, ARRAY_SIZE(commands), notifs, ARRAY_SIZE(notifs), NULL, 0, diff --git a/plugins/chanbackup.c b/plugins/chanbackup.c index cd094e41e..ca8365495 100644 --- a/plugins/chanbackup.c +++ b/plugins/chanbackup.c @@ -827,7 +827,7 @@ int main(int argc, char *argv[]) { setup_locale(); - plugin_main(argv, init, PLUGIN_STATIC, true, NULL, + plugin_main(argv, init, NULL, PLUGIN_STATIC, true, NULL, commands, ARRAY_SIZE(commands), notifs, ARRAY_SIZE(notifs), hooks, ARRAY_SIZE(hooks), NULL, 0, /* Notification topics we publish */ diff --git a/plugins/commando.c b/plugins/commando.c index 0f20ad927..aede4c35b 100644 --- a/plugins/commando.c +++ b/plugins/commando.c @@ -825,7 +825,7 @@ static const struct plugin_command commands[] = { { int main(int argc, char *argv[]) { setup_locale(); - plugin_main(argv, init, PLUGIN_STATIC, true, NULL, + plugin_main(argv, init, NULL, PLUGIN_STATIC, true, NULL, commands, ARRAY_SIZE(commands), NULL, 0, hooks, ARRAY_SIZE(hooks), diff --git a/plugins/funder.c b/plugins/funder.c index eb292bd6d..66063b363 100644 --- a/plugins/funder.c +++ b/plugins/funder.c @@ -1695,7 +1695,7 @@ int main(int argc, char **argv) /* Our default funding policy is fixed (0msat) */ current_policy = default_funder_policy(NULL, FIXED, 0); - plugin_main(argv, init, PLUGIN_RESTARTABLE, true, + plugin_main(argv, init, NULL, PLUGIN_RESTARTABLE, true, NULL, commands, ARRAY_SIZE(commands), notifs, ARRAY_SIZE(notifs), diff --git a/plugins/keysend.c b/plugins/keysend.c index 486a203b6..b97bd8932 100644 --- a/plugins/keysend.c +++ b/plugins/keysend.c @@ -594,7 +594,7 @@ int main(int argc, char *argv[]) features->bits[i] = tal_arr(features, u8, 0); set_feature_bit(&features->bits[NODE_ANNOUNCE_FEATURE], KEYSEND_FEATUREBIT); - plugin_main(argv, init, PLUGIN_STATIC, true, features, commands, + plugin_main(argv, init, NULL, PLUGIN_STATIC, true, features, commands, ARRAY_SIZE(commands), NULL, 0, hooks, ARRAY_SIZE(hooks), notification_topics, ARRAY_SIZE(notification_topics), NULL); } diff --git a/plugins/libplugin.c b/plugins/libplugin.c index fffcbe5aa..3ba3b85ab 100644 --- a/plugins/libplugin.c +++ b/plugins/libplugin.c @@ -79,6 +79,9 @@ struct plugin { /* to append to all our command ids */ const char *id; + /* Data for the plugin user */ + void *data; + /* options to i-promise-to-fix-broken-api-user */ const char **beglist; @@ -2179,6 +2182,7 @@ static struct plugin *new_plugin(const tal_t *ctx, void plugin_main(char *argv[], const char *(*init)(struct plugin *p, const char *buf, const jsmntok_t *), + void *data, const enum plugin_restartability restartability, bool init_rpc, struct feature_set *features STEALS, @@ -2208,6 +2212,7 @@ void plugin_main(char *argv[], init, restartability, init_rpc, features, commands, num_commands, notif_subs, num_notif_subs, hook_subs, num_hook_subs, notif_topics, num_notif_topics, ap); + plugin_set_data(plugin, data); va_end(ap); setup_command_usage(plugin); @@ -2414,3 +2419,15 @@ bool plugin_developer_mode(const struct plugin *plugin) { return plugin->developer; } + +void plugin_set_data(struct plugin *plugin, void *data TAKES) +{ + if (taken(data)) + tal_steal(plugin, data); + plugin->data = data; +} + +void *plugin_get_data_(struct plugin *plugin) +{ + return plugin->data; +} diff --git a/plugins/libplugin.h b/plugins/libplugin.h index 130127d2b..6a3be12c0 100644 --- a/plugins/libplugin.h +++ b/plugins/libplugin.h @@ -440,6 +440,11 @@ static inline void *plugin_option_jsonfmt_check(bool (*jsonfmt)(struct plugin *, /* Is --developer enabled? */ bool plugin_developer_mode(const struct plugin *plugin); +/* Store a single pointer for our state in the plugin */ +void plugin_set_data(struct plugin *plugin, void *data TAKES); +void *plugin_get_data_(struct plugin *plugin); +#define plugin_get_data(plugin, type) ((type *)(plugin_get_data_(plugin))) + /* Macro to define arguments */ #define plugin_option_(name, type, description, set, jsonfmt, arg, dev_only, depr_start, depr_end, dynamic) \ (name), \ @@ -504,6 +509,7 @@ void NORETURN LAST_ARG_NULL plugin_main(char *argv[], const char *(*init)(struct plugin *p, const char *buf, const jsmntok_t *), + void *data TAKES, const enum plugin_restartability restartability, bool init_rpc, struct feature_set *features STEALS, diff --git a/plugins/offers.c b/plugins/offers.c index be543a5ea..ef75d6c46 100644 --- a/plugins/offers.c +++ b/plugins/offers.c @@ -1473,7 +1473,7 @@ int main(int argc, char *argv[]) /* We deal in UTC; mktime() uses local time */ setenv("TZ", "", 1); - plugin_main(argv, init, PLUGIN_RESTARTABLE, true, NULL, + plugin_main(argv, init, NULL, PLUGIN_RESTARTABLE, true, NULL, commands, ARRAY_SIZE(commands), notifications, ARRAY_SIZE(notifications), hooks, ARRAY_SIZE(hooks), diff --git a/plugins/pay.c b/plugins/pay.c index 3a33f3d88..41f1d63dd 100644 --- a/plugins/pay.c +++ b/plugins/pay.c @@ -1527,7 +1527,7 @@ static const char *notification_topics[] = { int main(int argc, char *argv[]) { setup_locale(); - plugin_main(argv, init, PLUGIN_RESTARTABLE, true, NULL, commands, + plugin_main(argv, init, NULL, PLUGIN_RESTARTABLE, true, NULL, commands, ARRAY_SIZE(commands), NULL, 0, NULL, 0, notification_topics, ARRAY_SIZE(notification_topics), plugin_option("disable-mpp", "flag", diff --git a/plugins/recover.c b/plugins/recover.c index 07149f773..b46d55dc7 100644 --- a/plugins/recover.c +++ b/plugins/recover.c @@ -288,7 +288,7 @@ int main(int argc, char *argv[]) { setup_locale(); - plugin_main(argv, init, PLUGIN_STATIC, true, NULL, + plugin_main(argv, init, NULL, PLUGIN_STATIC, true, NULL, NULL, 0, NULL, 0, NULL, 0, NULL, 0, /* Notification topics we publish */ diff --git a/plugins/renepay/main.c b/plugins/renepay/main.c index 2aa13dace..5ee7de946 100644 --- a/plugins/renepay/main.c +++ b/plugins/renepay/main.c @@ -411,7 +411,7 @@ int main(int argc, char *argv[]) plugin_main( argv, - init, + init, NULL, PLUGIN_RESTARTABLE, /* init_rpc */ true, /* features */ NULL, diff --git a/plugins/spender/main.c b/plugins/spender/main.c index 23698567a..ce0e84d10 100644 --- a/plugins/spender/main.c +++ b/plugins/spender/main.c @@ -32,7 +32,7 @@ int main(int argc, char **argv) notifs = tal_arr(NULL, struct plugin_notification, 0); tal_expand(¬ifs, openchannel_notifs, num_openchannel_notifs); - plugin_main(argv, &spender_init, PLUGIN_STATIC, true, + plugin_main(argv, &spender_init, NULL, PLUGIN_STATIC, true, NULL, take(commands), tal_count(commands), take(notifs), tal_count(notifs), diff --git a/plugins/sql.c b/plugins/sql.c index dd5d722af..1490b5063 100644 --- a/plugins/sql.c +++ b/plugins/sql.c @@ -1650,7 +1650,7 @@ int main(int argc, char *argv[]) common_shutdown(); return 0; } - plugin_main(argv, init, PLUGIN_RESTARTABLE, true, NULL, commands, ARRAY_SIZE(commands), + plugin_main(argv, init, NULL, PLUGIN_RESTARTABLE, true, NULL, commands, ARRAY_SIZE(commands), NULL, 0, NULL, 0, NULL, 0, plugin_option_dev("dev-sqlfilename", "string", diff --git a/plugins/topology.c b/plugins/topology.c index d2368bdbf..8b9ad3d0a 100644 --- a/plugins/topology.c +++ b/plugins/topology.c @@ -747,6 +747,6 @@ static const struct plugin_command commands[] = { int main(int argc, char *argv[]) { setup_locale(); - plugin_main(argv, init, PLUGIN_STATIC, true, NULL, commands, ARRAY_SIZE(commands), + plugin_main(argv, init, NULL, PLUGIN_STATIC, true, NULL, commands, ARRAY_SIZE(commands), NULL, 0, NULL, 0, NULL, 0, NULL); } diff --git a/plugins/txprepare.c b/plugins/txprepare.c index 360f4277c..3024f305b 100644 --- a/plugins/txprepare.c +++ b/plugins/txprepare.c @@ -668,6 +668,6 @@ static const char *init(struct plugin *p, int main(int argc, char *argv[]) { setup_locale(); - plugin_main(argv, init, PLUGIN_RESTARTABLE, true, NULL, commands, + plugin_main(argv, init, NULL, PLUGIN_RESTARTABLE, true, NULL, commands, ARRAY_SIZE(commands), NULL, 0, NULL, 0, NULL, 0, NULL); } diff --git a/tests/plugins/test_libplugin.c b/tests/plugins/test_libplugin.c index 6a2580ef6..71a306b51 100644 --- a/tests/plugins/test_libplugin.c +++ b/tests/plugins/test_libplugin.c @@ -6,10 +6,18 @@ #include #include -static char *somearg; -static bool self_disable = false; -static bool dont_shutdown = false; -static u32 dynamic_opt = 7; +/* Stash this in plugin's data */ +struct test_libplugin { + char *somearg; + bool self_disable; + bool dont_shutdown; + u32 dynamic_opt; +}; + +static struct test_libplugin *get_test_libplugin(struct plugin *plugin) +{ + return plugin_get_data(plugin, struct test_libplugin); +} static struct command_result *get_ds_done(struct command *cmd, const char *val, @@ -94,9 +102,10 @@ static struct command_result *json_shutdown(struct command *cmd, const char *buf, const jsmntok_t *params) { + struct test_libplugin *tlp = get_test_libplugin(cmd->plugin); plugin_log(cmd->plugin, LOG_DBG, "shutdown called"); - if (dont_shutdown) + if (tlp->dont_shutdown) return notification_handled(cmd); plugin_exit(cmd->plugin, 0); @@ -202,13 +211,14 @@ static const char *init(struct plugin *p, { const char *name, *err_str, *err_hex; const u8 *binname; + struct test_libplugin *tlp = get_test_libplugin(p); plugin_log(p, LOG_DBG, "test_libplugin initialised!"); - if (somearg) - plugin_log(p, LOG_DBG, "somearg = %s", somearg); - somearg = tal_free(somearg); + if (tlp->somearg) + plugin_log(p, LOG_DBG, "somearg = %s", tlp->somearg); + tlp->somearg = tal_free(tlp->somearg); - if (self_disable) + if (tlp->self_disable) return "Disabled via selfdisable option"; /* Test rpc_scan_datastore funcs */ @@ -277,30 +287,43 @@ static const struct plugin_notification notifs[] = { { int main(int argc, char *argv[]) { setup_locale(); - plugin_main(argv, init, PLUGIN_RESTARTABLE, true, NULL, + /* We allocate now, so we can hand pointers for plugin options, + * but by specifying take() to plugin_main, it reparents it to + * the plugin */ + struct test_libplugin *tlp = tal(NULL, struct test_libplugin); + tlp->somearg = NULL; + tlp->self_disable = false; + tlp->dont_shutdown = false; + tlp->dynamic_opt = 7; + + plugin_main(argv, init, take(tlp), PLUGIN_RESTARTABLE, true, NULL, commands, ARRAY_SIZE(commands), notifs, ARRAY_SIZE(notifs), hooks, ARRAY_SIZE(hooks), NULL, 0, /* Notification topics we publish */ plugin_option("somearg", "string", "Argument to print at init.", - charp_option, charp_jsonfmt, &somearg), + charp_option, charp_jsonfmt, &tlp->somearg), plugin_option_deprecated("somearg-deprecated", "string", "Deprecated arg for init.", CLN_NEXT_VERSION, NULL, - charp_option, charp_jsonfmt, &somearg), + charp_option, charp_jsonfmt, + &tlp->somearg), plugin_option("selfdisable", "flag", "Whether to disable.", - flag_option, flag_jsonfmt, &self_disable), + flag_option, flag_jsonfmt, + &tlp->self_disable), plugin_option("dont_shutdown", "flag", "Whether to timeout when asked to shutdown.", - flag_option, flag_jsonfmt, &dont_shutdown), + flag_option, flag_jsonfmt, + &tlp->dont_shutdown), plugin_option_dynamic("dynamicopt", "int", "Set me!", - set_dynamic, u32_jsonfmt, &dynamic_opt), + set_dynamic, u32_jsonfmt, + &tlp->dynamic_opt), NULL); }