From 82ff580a6649fc8751f6cb32b6ea169d224c0809 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 17 Jan 2019 12:42:13 +1030 Subject: [PATCH] json: add more efficient iterators for objects and arrays. Christian points out that we can iterate by ->size rather than calling json_next() to find the end (which traverses the entire object!). Now ->size is reliable (since previous patch), this is OK. Reported-by: @cdecker Signed-off-by: Rusty Russell --- common/json.c | 12 ++++----- common/json.h | 11 ++++++++- common/param.c | 49 ++++++++++++++++++------------------- lightningd/gossip_control.c | 11 +++------ lightningd/invoice.c | 46 +++++++++++++++++----------------- lightningd/pay.c | 35 +++++++++++--------------- plugins/pay.c | 34 ++++++++++++------------- 7 files changed, 97 insertions(+), 101 deletions(-) diff --git a/common/json.c b/common/json.c index 4324aef9b..2062e6baa 100644 --- a/common/json.c +++ b/common/json.c @@ -180,13 +180,13 @@ const jsmntok_t *json_next(const jsmntok_t *tok) const jsmntok_t *json_get_member(const char *buffer, const jsmntok_t tok[], const char *label) { - const jsmntok_t *t, *end; + const jsmntok_t *t; + size_t i; if (tok->type != JSMN_OBJECT) return NULL; - end = json_next(tok); - for (t = tok + 1; t < end; t = json_next(t+1)) + json_for_each_obj(i, t, tok) if (json_tok_streq(buffer, t, label)) return t + 1; @@ -195,13 +195,13 @@ const jsmntok_t *json_get_member(const char *buffer, const jsmntok_t tok[], const jsmntok_t *json_get_arr(const jsmntok_t tok[], size_t index) { - const jsmntok_t *t, *end; + const jsmntok_t *t; + size_t i; if (tok->type != JSMN_ARRAY) return NULL; - end = json_next(tok); - for (t = tok + 1; t < end; t = json_next(t)) { + json_for_each_arr(i, t, tok) { if (index == 0) return t; index--; diff --git a/common/json.h b/common/json.h index e024e9e5f..38c10e2b2 100644 --- a/common/json.h +++ b/common/json.h @@ -51,7 +51,7 @@ bool json_tok_is_num(const char *buffer, const jsmntok_t *tok); /* Is this the null primitive? */ bool json_tok_is_null(const char *buffer, const jsmntok_t *tok); -/* Returns next token with same parent. */ +/* Returns next token with same parent (WARNING: slow!). */ const jsmntok_t *json_next(const jsmntok_t *tok); /* Get top-level member. */ @@ -85,4 +85,13 @@ const jsmntok_t *json_delve(const char *buffer, const jsmntok_t *tok, const char *guide); +/* Iterator macro for array: i is counter, t is token ptr, arr is JSMN_ARRAY */ +#define json_for_each_arr(i, t, arr) \ + for (i = 0, t = (arr) + 1; i < (arr)->size; t = json_next(t), i++) + +/* Iterator macro for object: i is counter, t is token ptr (t+1 is + * contents of obj member), obj is JSMN_OBJECT */ +#define json_for_each_obj(i, t, obj) \ + for (i = 0, t = (obj) + 1; i < (obj)->size; t = json_next(t+1), i++) + #endif /* LIGHTNING_COMMON_JSON_H */ diff --git a/common/param.c b/common/param.c index adabdd1f1..39e5f41d2 100644 --- a/common/param.c +++ b/common/param.c @@ -68,27 +68,27 @@ static struct command_result *parse_by_position(struct command *cmd, bool allow_extra) { struct command_result *res; - const jsmntok_t *tok = tokens + 1; - const jsmntok_t *end = json_next(tokens); - struct param *first = params; - struct param *last = first + tal_count(params); + const jsmntok_t *tok; + size_t i; + + json_for_each_arr(i, tok, tokens) { + /* check for unexpected trailing params */ + if (i == tal_count(params)) { + if (!allow_extra) { + return command_fail(cmd, JSONRPC2_INVALID_PARAMS, + "too many parameters:" + " got %u, expected %zu", + tokens->size, + tal_count(params)); + } + break; + } - while (first != last && tok != end) { if (!json_tok_is_null(buffer, tok)) { - res = make_callback(cmd, first, buffer, tok); + res = make_callback(cmd, params+i, buffer, tok); if (res) return res; } - tok = json_next(tok); - first++; - } - - /* check for unexpected trailing params */ - if (!allow_extra && tok != end) { - return command_fail(cmd, JSONRPC2_INVALID_PARAMS, - "too many parameters:" - " got %u, expected %zu", - tokens->size, tal_count(params)); } return post_check(cmd, params); @@ -115,18 +115,18 @@ static struct command_result *parse_by_name(struct command *cmd, const jsmntok_t tokens[], bool allow_extra) { - const jsmntok_t *first = tokens + 1; - const jsmntok_t *last = json_next(tokens); + size_t i; + const jsmntok_t *t; - while (first != last) { - struct param *p = find_param(params, buffer + first->start, - first->end - first->start); + json_for_each_obj(i, t, tokens) { + struct param *p = find_param(params, buffer + t->start, + t->end - t->start); if (!p) { if (!allow_extra) { return command_fail(cmd, JSONRPC2_INVALID_PARAMS, "unknown parameter: '%.*s'", - first->end - first->start, - buffer + first->start); + t->end - t->start, + buffer + t->start); } } else { struct command_result *res; @@ -137,11 +137,10 @@ static struct command_result *parse_by_name(struct command *cmd, p->name); } - res = make_callback(cmd, p, buffer, first + 1); + res = make_callback(cmd, p, buffer, t + 1); if (res) return res; } - first = json_next(first + 1); } return post_check(cmd, params); } diff --git a/lightningd/gossip_control.c b/lightningd/gossip_control.c index f9319774b..1ff7af914 100644 --- a/lightningd/gossip_control.c +++ b/lightningd/gossip_control.c @@ -331,15 +331,13 @@ static struct command_result *json_getroute(struct command *cmd, *fuzz = *fuzz / 100.0; if (excludetok) { - const jsmntok_t *t, *end = json_next(excludetok); + const jsmntok_t *t; size_t i; excluded = tal_arr(cmd, struct short_channel_id_dir, excludetok->size); - for (i = 0, t = excludetok + 1; - t < end; - t = json_next(t), i++) { + json_for_each_arr(i, t, excludetok) { if (!short_channel_id_dir_from_str(buffer + t->start, t->end - t->start, &excluded[i])) { @@ -486,7 +484,7 @@ static struct command_result *json_dev_query_scids(struct command *cmd, { u8 *msg; const jsmntok_t *scidstok; - const jsmntok_t *t, *end; + const jsmntok_t *t; struct pubkey *id; struct short_channel_id *scids; size_t i; @@ -498,8 +496,7 @@ static struct command_result *json_dev_query_scids(struct command *cmd, return command_param_failed(); scids = tal_arr(cmd, struct short_channel_id, scidstok->size); - end = json_next(scidstok); - for (i = 0, t = scidstok + 1; t < end; t = json_next(t), i++) { + json_for_each_arr(i, t, scidstok) { if (!json_to_short_channel_id(buffer, t, &scids[i])) { return command_fail(cmd, JSONRPC2_INVALID_PARAMS, "scid %zu '%.*s' is not an scid", diff --git a/lightningd/invoice.c b/lightningd/invoice.c index 9f894e8f5..22fd96748 100644 --- a/lightningd/invoice.c +++ b/lightningd/invoice.c @@ -307,13 +307,13 @@ static struct route_info *unpack_route(const tal_t *ctx, const char *buffer, const jsmntok_t *routetok) { - const jsmntok_t *t, *end; - struct route_info *route = tal_arr(ctx, struct route_info, 0); + const jsmntok_t *t; + size_t i; + struct route_info *route = tal_arr(ctx, struct route_info, routetok->size); - end = json_next(routetok); - for (t = routetok + 1; t < end; t = json_next(t)) { + json_for_each_arr(i, t, routetok) { const jsmntok_t *pubkey, *fee_base, *fee_prop, *scid, *cltv; - struct route_info r; + struct route_info *r = &route[i]; u32 cltv_u32; pubkey = json_get_member(buffer, t, "id"); @@ -323,17 +323,16 @@ static struct route_info *unpack_route(const tal_t *ctx, "fee_proportional_millionths"); cltv = json_get_member(buffer, t, "cltv_expiry_delta"); - if (!json_to_pubkey(buffer, pubkey, &r.pubkey) + if (!json_to_pubkey(buffer, pubkey, &r->pubkey) || !json_to_short_channel_id(buffer, scid, - &r.short_channel_id) - || !json_to_number(buffer, fee_base, &r.fee_base_msat) + &r->short_channel_id) + || !json_to_number(buffer, fee_base, &r->fee_base_msat) || !json_to_number(buffer, fee_prop, - &r.fee_proportional_millionths) + &r->fee_proportional_millionths) || !json_to_number(buffer, cltv, &cltv_u32)) abort(); /* We don't have a json_to_u16 */ - r.cltv_expiry_delta = cltv_u32; - tal_arr_expand(&route, r); + r->cltv_expiry_delta = cltv_u32; } return route; } @@ -343,17 +342,16 @@ static struct route_info **unpack_routes(const tal_t *ctx, const jsmntok_t *routestok) { struct route_info **routes; - const jsmntok_t *t, *end; + const jsmntok_t *t; + size_t i; if (!routestok) return NULL; - routes = tal_arr(ctx, struct route_info *, 0); - end = json_next(routestok); - for (t = routestok + 1; t < end; t = json_next(t)) { - struct route_info *r = unpack_route(routes, buffer, t); - tal_arr_expand(&routes, r); - } + routes = tal_arr(ctx, struct route_info *, routestok->size); + json_for_each_arr(i, t, routestok) + routes[i] = unpack_route(routes, buffer, t); + return routes; } #endif /* DEVELOPER */ @@ -409,16 +407,16 @@ static struct command_result *json_invoice(struct command *cmd, } if (fallbacks) { - const jsmntok_t *i, *end = json_next(fallbacks); + size_t i; + const jsmntok_t *t; - fallback_scripts = tal_arr(cmd, const u8 *, 0); - for (i = fallbacks + 1; i < end; i = json_next(i)) { + fallback_scripts = tal_arr(cmd, const u8 *, fallbacks->size); + json_for_each_arr(i, t, fallbacks) { struct command_result *r; - const u8 *fs; - r = parse_fallback(cmd, buffer, i, &fs); + + r = parse_fallback(cmd, buffer, t, &fallback_scripts[i]); if (r) return r; - tal_arr_expand(&fallback_scripts, fs); } } diff --git a/lightningd/pay.c b/lightningd/pay.c index 53e5decec..9d07a2c5e 100644 --- a/lightningd/pay.c +++ b/lightningd/pay.c @@ -981,8 +981,8 @@ static struct command_result *json_sendpay(struct command *cmd, const jsmntok_t *params) { const jsmntok_t *routetok; - const jsmntok_t *t, *end; - size_t n_hops; + const jsmntok_t *t; + size_t i; struct sha256 *rhash; struct route_hop *route; u64 *msatoshi; @@ -996,11 +996,11 @@ static struct command_result *json_sendpay(struct command *cmd, NULL)) return command_param_failed(); - end = json_next(routetok); - n_hops = 0; - route = tal_arr(cmd, struct route_hop, n_hops); + if (routetok->size == 0) + return command_fail(cmd, JSONRPC2_INVALID_PARAMS, "Empty route"); - for (t = routetok + 1; t < end; t = json_next(t)) { + route = tal_arr(cmd, struct route_hop, routetok->size); + json_for_each_arr(i, t, routetok) { u64 *amount; struct pubkey *id; struct short_channel_id *channel; @@ -1015,19 +1015,12 @@ static struct command_result *json_sendpay(struct command *cmd, NULL)) return command_param_failed(); - tal_resize(&route, n_hops + 1); - - route[n_hops].amount = *amount; - route[n_hops].nodeid = *id; - route[n_hops].delay = *delay; - route[n_hops].channel_id = *channel; + route[i].amount = *amount; + route[i].nodeid = *id; + route[i].delay = *delay; + route[i].channel_id = *channel; /* FIXME: Actually ignored by sending code! */ - route[n_hops].direction = direction ? *direction : 0; - n_hops++; - } - - if (n_hops == 0) { - return command_fail(cmd, JSONRPC2_INVALID_PARAMS, "Empty route"); + route[i].direction = direction ? *direction : 0; } /* The given msatoshi is the actual payment that the payee is @@ -1036,8 +1029,8 @@ static struct command_result *json_sendpay(struct command *cmd, /* if not: msatoshi <= finalhop.amount <= 2 * msatoshi, fail. */ if (msatoshi) { - if (!(*msatoshi <= route[n_hops-1].amount && - route[n_hops-1].amount <= 2 * *msatoshi)) { + if (!(*msatoshi <= route[routetok->size-1].amount && + route[routetok->size-1].amount <= 2 * *msatoshi)) { return command_fail(cmd, JSONRPC2_INVALID_PARAMS, "msatoshi %"PRIu64" out of range", *msatoshi); @@ -1045,7 +1038,7 @@ static struct command_result *json_sendpay(struct command *cmd, } if (send_payment(cmd, cmd->ld, rhash, route, - msatoshi ? *msatoshi : route[n_hops-1].amount, + msatoshi ? *msatoshi : route[routetok->size-1].amount, description, &json_sendpay_on_resolve, cmd)) return command_still_pending(cmd); diff --git a/plugins/pay.c b/plugins/pay.c index f39ec20ac..19905d921 100644 --- a/plugins/pay.c +++ b/plugins/pay.c @@ -345,15 +345,15 @@ static const jsmntok_t *find_worst_channel(const char *buf, u64 final) { u64 prev = final, worstval = 0; - const jsmntok_t *worst = NULL, *end; + const jsmntok_t *worst = NULL, *t; + size_t i; - end = json_next(route); - for (route = route + 1; route < end; route = json_next(route)) { + json_for_each_arr(i, t, route) { u64 val; - json_to_u64(buf, json_get_member(buf, route, fieldname), &val); + json_to_u64(buf, json_get_member(buf, t, fieldname), &val); if (worst == NULL || val - prev > worstval) { - worst = route; + worst = t; worstval = val - prev; } prev = val; @@ -616,12 +616,12 @@ static struct command_result *add_shadow_route(struct command *cmd, { /* Use reservoir sampling across the capable channels. */ const jsmntok_t *channels = json_get_member(buf, result, "channels"); - const jsmntok_t *chan, *end, *best = NULL; + const jsmntok_t *chan, *best = NULL; + size_t i; u64 sample; u32 cltv, best_cltv; - end = json_next(channels); - for (chan = channels + 1; chan < end; chan = json_next(chan)) { + json_for_each_arr(i, chan, channels) { u64 sats, v; json_to_u64(buf, json_get_member(buf, chan, "satoshis"), &sats); @@ -676,24 +676,24 @@ static struct command_result *listpeers_done(struct command *cmd, const jsmntok_t *result, struct pay_command *pc) { - const jsmntok_t *peer, *peers_end; + const jsmntok_t *peers, *peer; + size_t i; char *mods = tal_strdup(tmpctx, ""); - peer = json_get_member(buf, result, "peers"); - if (!peer) + peers = json_get_member(buf, result, "peers"); + if (!peers) plugin_err("listpeers gave no 'peers'? '%.*s'", result->end - result->start, buf); - peers_end = json_next(peer); - for (peer = peer + 1; peer < peers_end; peer = json_next(peer)) { - const jsmntok_t *chan, *chans_end; + json_for_each_arr(i, peer, peers) { + const jsmntok_t *chans, *chan; bool connected; + size_t j; json_to_bool(buf, json_get_member(buf, peer, "connected"), &connected); - chan = json_get_member(buf, peer, "channels"); - chans_end = json_next(chan); - for (chan = chan + 1; chan < chans_end; chan = json_next(chan)) { + chans = json_get_member(buf, peer, "channels"); + json_for_each_arr(j, chan, chans) { const jsmntok_t *state, *scid, *dir; u64 spendable;