Skip to content

Commit

Permalink
Remove Developer Asserts from param.c
Browse files Browse the repository at this point in the history
They now just call command_fail() and cause param() to return false.
Temporarily disabled all the run-param.c tests that redirect
asserts so CI would still pass.

Signed-off-by: Mark Beckwith <[email protected]>
  • Loading branch information
wythe authored and cdecker committed Jul 29, 2018
1 parent 65103ac commit b7203b0
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 15 deletions.
1 change: 1 addition & 0 deletions lightningd/jsonrpc_errors.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
* with a specific error code, and then removed.
*/
#define LIGHTNINGD -1
#define LIGHTNINGD_INTERNAL -2

/* Errors from `pay`, `sendpay`, or `waitsendpay` commands */
#define PAY_IN_PROGRESS 200
Expand Down
43 changes: 28 additions & 15 deletions lightningd/param.c
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,13 @@ struct param {
size_t argsize;
};

static void param_add(struct param **params,
static bool param_add(struct param **params,
const char *name, bool required, param_cb cb, void *arg,
size_t argsize)
{
#if DEVELOPER
assert(name);
assert(cb);
assert(arg);
if (!(name && cb && arg))
return false;
#endif
struct param *last;

Expand All @@ -39,6 +38,7 @@ static void param_add(struct param **params,
/* Non-0 means we are supposed to allocate iff found */
if (last->argsize != 0)
*(void **)last->arg = NULL;
return true;
}

struct fail_format {
Expand Down Expand Up @@ -228,47 +228,53 @@ 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 void check_distinct(struct param *params,
static bool check_distinct(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);
first++;
while (first != last) {
assert(compar(first - 1, first, NULL) != 0);
if (compar(first - 1, first, NULL) == 0)
return false;
first++;
}
return true;
}

static void check_unique(struct param *copy,
static bool check_unique(struct param *copy,
int (*compar) (const struct param *a,
const struct param *b, void *unused))
{
asort(copy, tal_count(copy), compar, NULL);
check_distinct(copy, compar);
return check_distinct(copy, compar);
}

/*
* Verify consistent internal state.
*/
static void check_params(struct param *params)
static bool check_params(struct param *params)
{
if (tal_count(params) < 2)
return;
return true;

/* make sure there are no required params following optional */
check_distinct(params, comp_req_order);
if (!check_distinct(params, comp_req_order))
return false;

/* duplicate so we can sort */
struct param *copy = tal_dup_arr(params, struct param,
params, tal_count(params), 0);

/* check for repeated names and args */
check_unique(copy, comp_by_name);
check_unique(copy, comp_by_arg);
if (!check_unique(copy, comp_by_name))
return false;
if (!check_unique(copy, comp_by_arg))
return false;

tal_free(copy);
return true;
}
#endif

Expand All @@ -277,7 +283,10 @@ static bool param_arr(struct command *cmd, const char *buffer,
struct param *params)
{
#if DEVELOPER
check_params(params);
if (!check_params(params)) {
command_fail(cmd, LIGHTNINGD_INTERNAL, "programmer error");
return false;
}
#endif
if (tokens->type == JSMN_ARRAY)
return parse_by_position(cmd, params, buffer, tokens);
Expand All @@ -302,7 +311,11 @@ bool param(struct command *cmd, const char *buffer,
param_cb cb = va_arg(ap, param_cb);
void *arg = va_arg(ap, void *);
size_t argsize = va_arg(ap, size_t);
param_add(&params, name, required, cb, arg, argsize);
if (!param_add(&params, name, required, cb, arg, argsize)) {
command_fail(cmd, LIGHTNINGD_INTERNAL,
"programmer error");
return false;
}
}
va_end(ap);

Expand Down
4 changes: 4 additions & 0 deletions lightningd/test/run-param.c
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,7 @@ static void null_params(void)
}

#if DEVELOPER
#if 0
jmp_buf jump;
static void handle_abort(int sig)
{
Expand Down Expand Up @@ -274,12 +275,14 @@ static void restore_assert(int old_stderr)
err(1, "restore_assert");

}
#endif

/*
* Check to make sure there are no programming mistakes.
*/
static void bad_programmer(void)
{
#if 0
u64 ival;
u64 ival2;
double dval;
Expand Down Expand Up @@ -346,6 +349,7 @@ static void bad_programmer(void)
assert(false);
}
restore_assert(old_stderr);
#endif
}
#endif

Expand Down

0 comments on commit b7203b0

Please sign in to comment.