From 93bf7c4839d6c7199cdc93f57166aeb06ec3dc61 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Sun, 16 Dec 2018 15:19:06 +1030 Subject: [PATCH] param: make command sinks (fail/success) return a special type. These routines free the 'struct command': a common coding error is not to return immediately. To catch this, we make them return a non-NULL 'struct command_result *', and we're going to make the command handlers return the same (to encourage 'return command_fail(...)'-style usage). We also provide two sources for external use: 1. command_param_failed() when param() fails. 2. command_its_complicated() for some complex cases. Signed-off-by: Rusty Russell --- common/json_command.h | 6 ++-- common/test/run-param.c | 4 ++- lightningd/jsonrpc.c | 40 ++++++++++++++++----- lightningd/jsonrpc.h | 36 ++++++++++++++++--- lightningd/test/run-invoice-select-inchan.c | 13 ++++--- wallet/test/run-wallet.c | 16 ++++++--- 6 files changed, 90 insertions(+), 25 deletions(-) diff --git a/common/json_command.h b/common/json_command.h index 27140ae71..1b05ee9b3 100644 --- a/common/json_command.h +++ b/common/json_command.h @@ -7,10 +7,12 @@ #include struct command; +struct command_result; /* Caller supplied this: param assumes it can call it. */ -void PRINTF_FMT(3, 4) command_fail(struct command *cmd, int code, - const char *fmt, ...); +struct command_result *command_fail(struct command *cmd, int code, + const char *fmt, ...) + PRINTF_FMT(3, 4); /* Also caller supplied: is this invoked simply to get usage? */ bool command_usage_only(const struct command *cmd); diff --git a/common/test/run-param.c b/common/test/run-param.c index cf86444ec..80ea498a3 100644 --- a/common/test/run-param.c +++ b/common/test/run-param.c @@ -22,13 +22,15 @@ static bool check_fail(void) { struct command *cmd; -void command_fail(struct command *cmd, int code, const char *fmt, ...) +struct command_result *command_fail(struct command *cmd, + int code, const char *fmt, ...) { failed = true; va_list ap; va_start(ap, fmt); fail_msg = tal_vfmt(cmd, fmt, ap); va_end(ap); + return NULL; } /* AUTOGENERATED MOCKS START */ diff --git a/lightningd/jsonrpc.c b/lightningd/jsonrpc.c index ed6ea02f5..0962c386b 100644 --- a/lightningd/jsonrpc.c +++ b/lightningd/jsonrpc.c @@ -45,6 +45,23 @@ #include #include +/* Dummy structure. */ +struct command_result { + char c; +}; + +static struct command_result param_failed, complete, pending, unknown; + +struct command_result *command_param_failed(void) +{ + return ¶m_failed; +} + +struct command_result *command_its_complicated(void) +{ + return &unknown; +} + /* This represents a JSON RPC connection. It can invoke multiple commands, but * a command can outlive the connection, which could close any time. */ struct json_connection { @@ -383,7 +400,8 @@ static void destroy_command(struct command *cmd) list_del_from(&cmd->jcon->commands, &cmd->list); } -void command_raw_complete(struct command *cmd, struct json_stream *result) +struct command_result *command_raw_complete(struct command *cmd, + struct json_stream *result) { json_stream_close(result, cmd); @@ -392,28 +410,31 @@ void command_raw_complete(struct command *cmd, struct json_stream *result) tal_steal(cmd->jcon, result); tal_free(cmd); + return &complete; } -void command_success(struct command *cmd, struct json_stream *result) +struct command_result *command_success(struct command *cmd, + struct json_stream *result) { assert(cmd); assert(cmd->have_json_stream); json_stream_append(result, " }\n\n"); - command_raw_complete(cmd, result); + return command_raw_complete(cmd, result); } -void command_failed(struct command *cmd, struct json_stream *result) +struct command_result *command_failed(struct command *cmd, + struct json_stream *result) { assert(cmd->have_json_stream); /* Have to close error */ json_stream_append(result, " } }\n\n"); - command_raw_complete(cmd, result); + return command_raw_complete(cmd, result); } -void PRINTF_FMT(3, 4) command_fail(struct command *cmd, int code, - const char *fmt, ...) +struct command_result *command_fail(struct command *cmd, int code, + const char *fmt, ...) { const char *errmsg; struct json_stream *r; @@ -424,13 +445,14 @@ void PRINTF_FMT(3, 4) command_fail(struct command *cmd, int code, va_end(ap); r = json_stream_fail_nodata(cmd, code, errmsg); - command_failed(cmd, r); + return command_failed(cmd, r); } -void command_still_pending(struct command *cmd) +struct command_result *command_still_pending(struct command *cmd) { notleak_with_children(cmd); cmd->pending = true; + return &pending; } static void json_command_malformed(struct json_connection *jcon, diff --git a/lightningd/jsonrpc.h b/lightningd/jsonrpc.h index ffc98a63b..0296d4985 100644 --- a/lightningd/jsonrpc.h +++ b/lightningd/jsonrpc.h @@ -41,6 +41,12 @@ struct command { bool have_json_stream; }; +/** + * Dummy structure to make sure you call one of + * command_success / command_failed / command_still_pending. + */ +struct command_result; + struct json_command { const char *name; void (*dispatch)(struct command *, @@ -87,15 +93,37 @@ struct json_stream *json_stream_fail_nodata(struct command *cmd, const char *errmsg); struct json_stream *null_response(struct command *cmd); -void command_success(struct command *cmd, struct json_stream *response); -void command_failed(struct command *cmd, struct json_stream *result); + +/* These returned values are never NULL. */ +struct command_result *command_success(struct command *cmd, + struct json_stream *response); +struct command_result *command_failed(struct command *cmd, + struct json_stream *result); /* Mainly for documentation, that we plan to close this later. */ -void command_still_pending(struct command *cmd); +struct command_result *command_still_pending(struct command *cmd); /* For low-level JSON stream access: */ struct json_stream *json_stream_raw_for_cmd(struct command *cmd); -void command_raw_complete(struct command *cmd, struct json_stream *result); +struct command_result *command_raw_complete(struct command *cmd, + struct json_stream *result); + +/* To return if param() fails. */ +extern struct command_result *command_param_failed(void); + +/* Wrapper for pending commands (ignores return) */ +static inline void was_pending(const struct command_result *res) +{ + assert(res); +} + +/* Transition for ignoring command */ +static inline void fixme_ignore(const struct command_result *res) +{ +} + +/* FIXME: For the few cases where return value is indeterminate */ +struct command_result *command_its_complicated(void); /** * Create a new jsonrpc to wrap all related information. diff --git a/lightningd/test/run-invoice-select-inchan.c b/lightningd/test/run-invoice-select-inchan.c index 58205b817..10de66cf1 100644 --- a/lightningd/test/run-invoice-select-inchan.c +++ b/lightningd/test/run-invoice-select-inchan.c @@ -41,17 +41,20 @@ bool channel_tell_funding_locked(struct lightningd *ld UNNEEDED, u32 depth UNNEEDED) { fprintf(stderr, "channel_tell_funding_locked called!\n"); abort(); } /* Generated stub for command_fail */ -void command_fail(struct command *cmd UNNEEDED, int code UNNEEDED, - const char *fmt UNNEEDED, ...) +struct command_result *command_fail(struct command *cmd UNNEEDED, int code UNNEEDED, + const char *fmt UNNEEDED, ...) + { fprintf(stderr, "command_fail called!\n"); abort(); } /* Generated stub for command_failed */ -void command_failed(struct command *cmd UNNEEDED, struct json_stream *result UNNEEDED) +struct command_result *command_failed(struct command *cmd UNNEEDED, + struct json_stream *result UNNEEDED) { fprintf(stderr, "command_failed called!\n"); abort(); } /* Generated stub for command_still_pending */ -void command_still_pending(struct command *cmd UNNEEDED) +struct command_result *command_still_pending(struct command *cmd UNNEEDED) { fprintf(stderr, "command_still_pending called!\n"); abort(); } /* Generated stub for command_success */ -void command_success(struct command *cmd UNNEEDED, struct json_stream *response UNNEEDED) +struct command_result *command_success(struct command *cmd UNNEEDED, + struct json_stream *response UNNEEDED) { fprintf(stderr, "command_success called!\n"); abort(); } /* Generated stub for connect_succeeded */ void connect_succeeded(struct lightningd *ld UNNEEDED, const struct pubkey *id UNNEEDED) diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c index 19c1ba0d0..4cc7d5cd3 100644 --- a/wallet/test/run-wallet.c +++ b/wallet/test/run-wallet.c @@ -51,14 +51,16 @@ bool channel_tell_funding_locked(struct lightningd *ld UNNEEDED, u32 depth UNNEEDED) { fprintf(stderr, "channel_tell_funding_locked called!\n"); abort(); } /* Generated stub for command_fail */ -void command_fail(struct command *cmd UNNEEDED, int code UNNEEDED, - const char *fmt UNNEEDED, ...) +struct command_result *command_fail(struct command *cmd UNNEEDED, int code UNNEEDED, + const char *fmt UNNEEDED, ...) + { fprintf(stderr, "command_fail called!\n"); abort(); } /* Generated stub for command_still_pending */ -void command_still_pending(struct command *cmd UNNEEDED) +struct command_result *command_still_pending(struct command *cmd UNNEEDED) { fprintf(stderr, "command_still_pending called!\n"); abort(); } /* Generated stub for command_success */ -void command_success(struct command *cmd UNNEEDED, struct json_stream *response UNNEEDED) +struct command_result *command_success(struct command *cmd UNNEEDED, + struct json_stream *response UNNEEDED) { fprintf(stderr, "command_success called!\n"); abort(); } /* Generated stub for connect_succeeded */ void connect_succeeded(struct lightningd *ld UNNEEDED, const struct pubkey *id UNNEEDED) @@ -264,6 +266,12 @@ bool json_tok_bool(struct command *cmd UNNEEDED, const char *name UNNEEDED, bool json_tok_channel_id(const char *buffer UNNEEDED, const jsmntok_t *tok UNNEEDED, struct channel_id *cid UNNEEDED) { fprintf(stderr, "json_tok_channel_id called!\n"); abort(); } +/* Generated stub for json_tok_full */ +const char *json_tok_full(const char *buffer UNNEEDED, const jsmntok_t *t UNNEEDED) +{ fprintf(stderr, "json_tok_full called!\n"); abort(); } +/* Generated stub for json_tok_full_len */ +int json_tok_full_len(const jsmntok_t *t UNNEEDED) +{ fprintf(stderr, "json_tok_full_len called!\n"); abort(); } /* Generated stub for json_tok_loglevel */ bool json_tok_loglevel(struct command *cmd UNNEEDED, const char *name UNNEEDED, const char *buffer UNNEEDED, const jsmntok_t *tok UNNEEDED,