Skip to content

Commit

Permalink
params: simplify lifetimes of params.
Browse files Browse the repository at this point in the history 
@wythe points out we don't need to keep the around now param_is_set()
is removed.  We can in fact go further and avoid marshalling them into
temporary objects at the caller altogether.

This means internally we have an array of struct param, rather than an
array of 'struct param *', which causes most of the noise in this
patch.

Signed-off-by: Rusty Russell <[email protected]>
  • Loading branch information
@rustyrussell
rustyrussell committed Jul 5, 2018
1 parent 3f6f9e6 commit 6ff901d
Show file tree
Hide file tree
Showing 3 changed files with 105 additions and 111 deletions.
147 changes: 69 additions & 78 deletions lightningd/params.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,20 @@ struct param {
size_t argsize;
};

struct param *param_add_(const tal_t *ctx,
const char *name, param_cb cb, void *arg,
size_t argsize)
static void param_add(struct param **params,
const char *name, param_cb cb, void *arg,
const tal_t *ctx, size_t argsize)
{
#if DEVELOPER
assert(name);
assert(cb);
assert(arg);
#endif
struct param *last = tal(tmpctx, struct param);
struct param *last;

tal_resize(params, tal_count(*params) + 1);
last = &(*params)[tal_count(*params) - 1];

last->ctx = ctx;
last->is_set = false;
last->name = name;
Expand All @@ -35,22 +39,6 @@ struct param *param_add_(const tal_t *ctx,
/* Non-NULL means we are supposed to allocate iff found */
if (last->ctx)
*(void **)last->arg = NULL;
return last;
}

struct param *param_opt_add_(const tal_t *ctx, const char *name,
const jsmntok_t **tok)
{
struct param *last = tal(tmpctx, struct param);
assert(ctx);
last->ctx = ctx;
last->is_set = false;
last->name = name;
last->cb = (param_cb)json_tok_tok;
last->arg = tok;
last->argsize = sizeof(*tok);
*tok = NULL;
return last;
}

struct fail_format {
Expand Down Expand Up @@ -109,37 +97,37 @@ static bool make_callback(struct command *cmd,
return true;
}

static struct param **post_check(struct command *cmd, struct param **params)
static struct param *post_check(struct command *cmd, struct param *params)
{
struct param **first = params;
struct param **last = first + tal_count(params);
struct param *first = params;
struct param *last = first + tal_count(params);

/* Make sure required params were provided. */
while (first != last && (*first)->argsize == 0) {
if (!(*first)->is_set) {
while (first != last && first->argsize == 0) {
if (!first->is_set) {
command_fail(cmd, JSONRPC2_INVALID_PARAMS,
"missing required parameter: '%s'",
(*first)->name);
first->name);
return NULL;
}
first++;
}
return params;
}

static struct param **parse_by_position(struct command *cmd,
struct param **params,
const char *buffer,
const jsmntok_t tokens[])
static bool parse_by_position(struct command *cmd,
struct param *params,
const char *buffer,
const jsmntok_t tokens[])
{
const jsmntok_t *tok = tokens + 1;
const jsmntok_t *end = json_next(tokens);
struct param **first = params;
struct param **last = first + tal_count(params);
struct param *first = params;
struct param *last = first + tal_count(params);

while (first != last && tok != end) {
if (!json_tok_is_null(buffer, tok))
if (!make_callback(cmd, *first, buffer, tok))
if (!make_callback(cmd, first, buffer, tok))
return NULL;
tok = json_next(tok);
first++;
Expand All @@ -151,31 +139,31 @@ static struct param **parse_by_position(struct command *cmd,
"too many parameters:"
" got %u, expected %zu",
tokens->size, tal_count(params));
return NULL;
return false;
}

return post_check(cmd, params);
}

static struct param *find_param(struct param **params, const char *start,
static struct param *find_param(struct param *params, const char *start,
size_t n)
{
struct param **first = params;
struct param **last = first + tal_count(params);
struct param *first = params;
struct param *last = first + tal_count(params);

while (first != last) {
if (strncmp((*first)->name, start, n) == 0)
if (strlen((*first)->name) == n)
return *first;
if (strncmp(first->name, start, n) == 0)
if (strlen(first->name) == n)
return first;
first++;
}
return NULL;
}

static struct param **parse_by_name(struct command *cmd,
struct param **params,
const char *buffer,
const jsmntok_t tokens[])
static bool parse_by_name(struct command *cmd,
struct param *params,
const char *buffer,
const jsmntok_t tokens[])
{
const jsmntok_t *first = tokens + 1;
const jsmntok_t *last = json_next(tokens);
Expand All @@ -188,47 +176,47 @@ static struct param **parse_by_name(struct command *cmd,
"unknown parameter: '%.*s'",
first->end - first->start,
buffer + first->start);
return NULL;
return false;
}

if (p->is_set) {
command_fail(cmd, JSONRPC2_INVALID_PARAMS,
"duplicate json names: '%s'", p->name);
return NULL;
return false;
}

if (!make_callback(cmd, p, buffer, first + 1))
return NULL;
return false;
first = json_next(first + 1);
}
return post_check(cmd, params);
}

#if DEVELOPER
static int comp_by_name(struct param *const *a, struct param *const *b,
static int comp_by_name(const struct param *a, const struct param *b,
void *unused)
{
return strcmp((*a)->name, (*b)->name);
return strcmp(a->name, b->name);
}

static int comp_by_arg(struct param *const *a, struct param *const *b,
static int comp_by_arg(const struct param *a, const struct param *b,
void *unused)
{
/* size_t could be larger than int: don't turn a 4bn difference into 0 */
if ((*a)->arg > (*b)->arg)
if (a->arg > b->arg)
return 1;
else if ((*a)->arg < (*b)->arg)
else if (a->arg < b->arg)
return -1;
return 0;
}

/* This comparator is a bit different, but works well.
* Return 0 if @a is optional and @b is required. Otherwise return 1.
*/
static int comp_req_order(struct param *const *a, struct param *const *b,
static int comp_req_order(const struct param *a, const struct param *b,
void *unused)
{
if ((*a)->argsize != 0 && (*b)->argsize == 0)
if (a->argsize != 0 && b->argsize == 0)
return 0;
return 1;
}
Expand All @@ -237,22 +225,22 @@ static int comp_req_order(struct param *const *a, struct param *const *b,
* Make sure 2 sequential items in @params are not equal (based on
* provided comparator).
*/
static void check_distinct(struct param **params,
int (*compar) (struct param *const *a,
struct param *const *b, void *unused))
static void 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);
struct param *first = params;
struct param *last = first + tal_count(params);
first++;
while (first != last) {
assert(compar(first - 1, first, NULL) != 0);
first++;
}
}

static void check_unique(struct param **copy,
int (*compar) (struct param *const *a,
struct param *const *b, void *unused))
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);
check_distinct(copy, compar);
Expand All @@ -261,7 +249,7 @@ static void check_unique(struct param **copy,
/*
* Verify consistent internal state.
*/
static void check_params(struct param **params)
static void check_params(struct param *params)
{
if (tal_count(params) < 2)
return;
Expand All @@ -270,8 +258,8 @@ static void check_params(struct param **params)
check_distinct(params, comp_req_order);

/* duplicate so we can sort */
struct param **copy = tal_dup_arr(params, struct param *,
params, tal_count(params), 0);
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);
Expand All @@ -281,10 +269,10 @@ static void check_params(struct param **params)
}
#endif

static struct param **param_parse_arr(struct command *cmd,
const char *buffer,
const jsmntok_t tokens[],
struct param **params)
static bool param_parse_arr(struct command *cmd,
const char *buffer,
const jsmntok_t tokens[],
struct param *params)
{
#if DEVELOPER
check_params(params);
Expand All @@ -296,20 +284,23 @@ static struct param **param_parse_arr(struct command *cmd,

command_fail(cmd, JSONRPC2_INVALID_PARAMS,
"Expected array or object for params");
return NULL;
return false;
}

struct param **param_parse(struct command *cmd, const char *buffer,
const jsmntok_t tokens[], ...)
bool param_parse(struct command *cmd, const char *buffer,
const jsmntok_t tokens[], ...)
{
struct param *def;
struct param **params = tal_arr(cmd, struct param *, 0);
struct param *params = tal_arr(tmpctx, struct param, 0);
const char *name;
va_list ap;

va_start(ap, tokens);
while ((def = va_arg(ap, struct param *)) != NULL) {
tal_steal(params, def);
tal_resize(&params, tal_count(params) + 1);
params[tal_count(params) - 1] = def;
while ((name = va_arg(ap, const char *)) != NULL) {
param_cb cb = va_arg(ap, param_cb);
void *arg = va_arg(ap, void *);
const tal_t *ctx = va_arg(ap, const tal_t *);
size_t argsize = va_arg(ap, size_t);
param_add(&params, name, cb, arg, ctx, argsize);
}
va_end(ap);

Expand Down
49 changes: 27 additions & 22 deletions lightningd/params.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ struct param;
return;
At this point in the code you can be assured the json tokens were successfully
parsed. If not, param_parse() returned NULL, having already called
parsed. If not, param_parse() returns false, having already called
command_fail() with a descriptive error message. The data section of the json
result contains the offending parameter and its value.
Expand All @@ -40,8 +40,8 @@ struct param;
Otherwise a generic message is provided.
*/
struct param **param_parse(struct command *cmd, const char *buffer,
const jsmntok_t params[], ...);
bool param_parse(struct command *cmd, const char *buffer,
const jsmntok_t params[], ...);

/*
* This callback provided must follow this signature; e.g.,
Expand All @@ -59,28 +59,33 @@ typedef bool(*param_cb)(const char *buffer, const jsmntok_t *tok, void *arg);
*
* Returns an opaque pointer that can be later used in param_is_set().
*/
#define param_req(name, cb, arg) \
param_add_(NULL, name"", \
typesafe_cb_preargs(bool, void *, \
(cb), (arg), \
const char *, \
const jsmntok_t *), \
(arg), 0)
#define param_req(name, cb, arg) \
name"", \
typesafe_cb_preargs(bool, void *, \
(cb), (arg), \
const char *, \
const jsmntok_t *), \
(arg), NULL, 0
/*
* Same as above but for optional parameters.
*/
#define param_opt(ctx, name, cb, arg) \
param_add_(ctx, name"", \
typesafe_cb_preargs(bool, void *, \
(cb), *(arg), \
const char *, \
const jsmntok_t *), \
(arg), sizeof(**arg))
struct param * param_add_(const tal_t *ctx, const char *name, param_cb cb, void *arg, size_t argsize);
#define param_opt(ctx, name, cb, arg) \
name"", \
typesafe_cb_preargs(bool, void *, \
(cb), *(arg), \
const char *, \
const jsmntok_t *), \
(arg), (ctx), sizeof(**arg)

#define param_opt_tok(ctx, name, arg) \
param_opt_add_(ctx, name"", arg)

struct param *param_opt_add_(const tal_t *ctx, const char *name, const jsmntok_t **tok);
/*
* For when you want an optional raw token.
*
* Note: weird sizeof() does type check that arg really is a (const) jsmntok_t **.
*/
#define param_opt_tok(ctx, name, arg) \
name"", \
json_tok_tok, \
(arg) + 0*sizeof(*(arg) == (jsmntok_t *)NULL), \
(ctx), sizeof(const jsmntok_t *)

#endif /* LIGHTNING_LIGHTNINGD_PARAMS_H */
Loading

0 comments on commit 6ff901d

Please sign in to comment.