common/json_param: use assert() for param correctness checks.

And do them on the first run (where we check parameters), instead
of every time.  Might as well do them in non-developer mode too,
since they're simply programmer correctness.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This commit is contained in:
Rusty Russell
2023-07-25 11:09:43 +09:30
parent 8df29db48f
commit 4037594c81
2 changed files with 75 additions and 79 deletions

View File

@@ -15,6 +15,11 @@
#include <common/json_param.h>
#include <common/route.h>
/* Overridden by run-param.c */
#ifndef paramcheck_assert
#define paramcheck_assert assert
#endif
struct param {
const char *name;
bool is_set;
@@ -23,17 +28,17 @@ struct param {
void *arg;
};
static bool param_add(struct param **params,
static void param_add(struct param **params,
const char *name,
enum param_style style,
param_cbx cbx, void *arg)
{
#if DEVELOPER
if (!(name && cbx && arg))
return false;
#endif
struct param last;
paramcheck_assert(name);
paramcheck_assert(cbx);
paramcheck_assert(arg);
last.is_set = false;
last.name = name;
last.style = style;
@@ -41,7 +46,6 @@ static bool param_add(struct param **params,
last.arg = arg;
tal_arr_expand(params, last);
return true;
}
/* FIXME: To support the deprecated p_req_dup_ok */
@@ -175,7 +179,6 @@ static struct command_result *parse_by_name(struct command *cmd,
return post_check(cmd, params);
}
#if DEVELOPER
static int comp_by_name(const struct param *a, const struct param *b,
void *unused)
{
@@ -208,54 +211,47 @@ static int comp_req_order(const struct param *a, const struct param *b,
* Make sure 2 sequential items in @params are not equal (based on
* provided comparator).
*/
static bool check_distinct(struct param *params,
int (*compar) (const struct param *a,
const struct param *b, void *unused))
static void check_distinct(const struct param *params,
int (*compar)(const struct param *a,
const struct param *b, void *unused))
{
struct param *first = params;
struct param *last = first + tal_count(params);
const struct param *first = params;
const struct param *last = first + tal_count(params);
first++;
while (first != last) {
if (compar(first - 1, first, NULL) == 0)
return false;
paramcheck_assert(compar(first - 1, first, NULL) != 0);
first++;
}
return true;
}
static bool check_unique(struct param *copy,
static void check_unique(struct param *copy,
int (*compar) (const struct param *a,
const struct param *b, void *unused))
{
asort(copy, tal_count(copy), compar, NULL);
return check_distinct(copy, compar);
check_distinct(copy, compar);
}
/*
* Verify consistent internal state.
*/
static bool check_params(struct param *params)
static void check_params(const struct param *params)
{
if (tal_count(params) < 2)
return true;
return;
/* make sure there are no required params following optional */
if (!check_distinct(params, comp_req_order))
return false;
check_distinct(params, comp_req_order);
/* duplicate so we can sort */
struct param *copy = tal_dup_talarr(params, struct param, params);
/* check for repeated names and args */
if (!check_unique(copy, comp_by_name))
return false;
if (!check_unique(copy, comp_by_arg))
return false;
check_unique(copy, comp_by_name);
check_unique(copy, comp_by_arg);
tal_free(copy);
return true;
}
#endif
static char *param_usage(const tal_t *ctx,
const struct param *params)
@@ -279,12 +275,6 @@ static struct command_result *param_arr(struct command *cmd, const char *buffer,
struct param *params,
bool allow_extra)
{
#if DEVELOPER
if (!check_params(params)) {
return command_fail(cmd, PARAM_DEV_ERROR,
"developer error: check_params");
}
#endif
if (tokens->type == JSMN_ARRAY)
return parse_by_position(cmd, params, buffer, tokens, allow_extra);
else if (tokens->type == JSMN_OBJECT)
@@ -315,6 +305,7 @@ const char *param_subcommand(struct command *cmd, const char *buffer,
for (size_t i = 0; i < tal_count(names); i++)
tal_append_fmt(&usage, "%c%s",
i == 0 ? '=' : '|', names[i]);
check_params(params);
command_set_usage(cmd, usage);
return NULL;
}
@@ -354,19 +345,12 @@ bool param(struct command *cmd, const char *buffer,
allow_extra = true;
continue;
}
if (!param_add(&params, name, style, cbx, arg)) {
/* We really do ignore this return! */
struct command_result *ignore;
ignore = command_fail(cmd, PARAM_DEV_ERROR,
"developer error: param_add %s", name);
assert(ignore);
va_end(ap);
return false;
}
param_add(&params, name, style, cbx, arg);
}
va_end(ap);
if (command_usage_only(cmd)) {
check_params(params);
command_set_usage(cmd, param_usage(cmd, params));
return false;
}

View File

@@ -3,12 +3,24 @@
#include "../json_filter.c"
#include "../json_parse.c"
#include "../json_parse_simple.c"
#include "../json_param.c"
#include <assert.h>
#include <ccan/array_size/array_size.h>
#include <common/channel_type.h>
#include <common/setup.h>
#include <stdio.h>
/* We want to catch parameter checs for bad_programmer() */
#define paramcheck_assert save_paramcheck_assert
static bool paramcheck_assert_failed;
static void save_paramcheck_assert(bool cond)
{
if (!cond)
paramcheck_assert_failed = true;
}
#include "../json_param.c"
char *fail_msg = NULL;
bool failed = false;
@@ -299,8 +311,6 @@ static void no_params(void)
assert(!param(cmd, j->buffer, j->toks, NULL));
}
#if DEVELOPER
/*
* Check to make sure there are no programming mistakes.
*/
@@ -311,56 +321,60 @@ static void bad_programmer(void)
u64 *fpval;
struct json *j = json_parse(cmd, "[ '25', '546', '26' ]");
/* Usage mode makes it check parameters are sane */
cmd->mode = CMD_USAGE;
/* check for repeated names */
assert(!param(cmd, j->buffer, j->toks,
p_req("repeat", param_u64, &ival),
p_req("fp", param_millionths, &fpval),
p_req("repeat", param_u64, &ival2), NULL));
assert(check_fail());
assert(strstr(fail_msg, "developer error"));
paramcheck_assert_failed = false;
param(cmd, j->buffer, j->toks,
p_req("repeat", param_u64, &ival),
p_req("fp", param_millionths, &fpval),
p_req("repeat", param_u64, &ival2), NULL);
assert(paramcheck_assert_failed);
assert(!param(cmd, j->buffer, j->toks,
p_req("repeat", param_u64, &ival),
p_req("fp", param_millionths, &fpval),
p_req("repeat", param_u64, &ival), NULL));
assert(check_fail());
assert(strstr(fail_msg, "developer error"));
paramcheck_assert_failed = false;
param(cmd, j->buffer, j->toks,
p_req("repeat", param_u64, &ival),
p_req("fp", param_millionths, &fpval),
p_req("repeat", param_u64, &ival), NULL);
assert(paramcheck_assert_failed);
assert(!param(cmd, j->buffer, j->toks,
p_req("u64", param_u64, &ival),
p_req("repeat", param_millionths, &fpval),
p_req("repeat", param_millionths, &fpval), NULL));
assert(check_fail());
assert(strstr(fail_msg, "developer error"));
paramcheck_assert_failed = false;
param(cmd, j->buffer, j->toks,
p_req("u64", param_u64, &ival),
p_req("repeat", param_millionths, &fpval),
p_req("repeat", param_millionths, &fpval), NULL);
assert(paramcheck_assert_failed);
/* check for repeated arguments */
assert(!param(cmd, j->buffer, j->toks,
p_req("u64", param_u64, &ival),
p_req("repeated-arg", param_u64, &ival), NULL));
assert(check_fail());
assert(strstr(fail_msg, "developer error"));
paramcheck_assert_failed = false;
param(cmd, j->buffer, j->toks,
p_req("u64", param_u64, &ival),
p_req("repeated-arg", param_u64, &ival), NULL);
assert(paramcheck_assert_failed);
assert(!param(cmd, j->buffer, j->toks,
p_req("u64", (param_cbx) NULL, NULL), NULL));
assert(check_fail());
assert(strstr(fail_msg, "developer error"));
paramcheck_assert_failed = false;
param(cmd, j->buffer, j->toks,
p_req("u64", (param_cbx) NULL, NULL), NULL);
assert(paramcheck_assert_failed);
/* Add required param after optional */
j = json_parse(cmd, "[ '25', '546', '26', '1.1' ]");
unsigned int *msatoshi;
u64 *riskfactor_millionths;
assert(!param(
paramcheck_assert_failed = false;
param(
cmd, j->buffer, j->toks, p_req("u64", param_u64, &ival),
p_req("fp", param_millionths, &fpval),
p_opt_def("msatoshi", param_number, &msatoshi, 100),
p_req("riskfactor", param_millionths, &riskfactor_millionths),
NULL));
NULL);
assert(*msatoshi);
assert(*msatoshi == 100);
assert(check_fail());
assert(strstr(fail_msg, "developer error"));
assert(paramcheck_assert_failed);
cmd->mode = CMD_NORMAL;
}
#endif
static void add_members(struct param **params,
char **obj,
@@ -665,9 +679,7 @@ int main(int argc, char *argv[])
tok_tok();
null_params();
no_params();
#if DEVELOPER
bad_programmer();
#endif
dup_names();
five_hundred_params();
sendpay();