Enhanced param parsing

[ Squashed into single commit --RR ]

This adds two new macros, `p_req_tal()` and `p_opt_tal()`. These support
callbacks that take a `struct command *` context.  Example:

	static bool json_tok_label_x(struct command *cmd,
                                      const char *name,
				      const char *buffer,
				      const jsmntok_t *tok,
				      struct json_escaped **label)

The above is taken from the run-param unit test (near the bottom of the diff).
The return value is true on success, or false (and it calls command_fail itself).

We can pretty much remove all remaining usage of `json_tok_tok` in the codebase
with this type of callback.

lightningd/param.c

@@ -12,16 +12,18 @@ struct param {
 	bool is_set;
 	bool required;
 	param_cb cb;
+	param_cbx cbx;
 	void *arg;
 	size_t argsize;
 };
 
 static bool param_add(struct param **params,
-		      const char *name, bool required, param_cb cb, void *arg,
+		      const char *name, bool required, param_cb cb,
+		      param_cbx cbx, void *arg,
 		      size_t argsize)
 {
 #if DEVELOPER
-	if (!(name && cb && arg))
+	if (!(name && (cb || cbx) && arg))
 		return false;
 #endif
 	struct param *last;
@@ -33,6 +35,7 @@ static bool param_add(struct param **params,
 	last->name = name;
 	last->required = required;
 	last->cb = cb;
+	last->cbx = cbx;
 	last->arg = arg;
 	last->argsize = argsize;
 	/* Non-0 means we are supposed to allocate iff found */
@@ -72,30 +75,44 @@ static const char *find_fail_format(param_cb cb)
 	return fmt->format;
 }
 
+/* Create a json_result out of a jsmntok_t. */
+static struct json_result *make_result(tal_t *ctx, const char *name, const char *buffer,
+				       const jsmntok_t *tok)
+{
+	struct json_result *data = new_json_result(ctx);
+	const char *val = tal_fmt(ctx, "%.*s", tok->end - tok->start,
+				  buffer + tok->start);
+	json_object_start(data, NULL);
+	json_add_string(data, name, val);
+	json_object_end(data);
+	return data;
+}
+
 static bool make_callback(struct command *cmd,
 			  struct param *def,
-			  const char *buffer, const jsmntok_t * tok)
+			  const char *buffer, const jsmntok_t *tok)
 {
 	void *arg;
 	def->is_set = true;
-	if (def->argsize && def->cb != (param_cb)json_tok_tok) {
-		*(void **)def->arg
-			= arg
-			= tal_arr_label(cmd, char, def->argsize, "param");
-	} else
-		arg = def->arg;
-	if (!def->cb(buffer, tok, arg)) {
-		struct json_result *data = new_json_result(cmd);
-		const char *val = tal_fmt(cmd, "%.*s", tok->end - tok->start,
-					  buffer + tok->start);
-		json_object_start(data, NULL);
-		json_add_string(data, def->name, val);
-		json_object_end(data);
-		command_fail_detailed(cmd, JSONRPC2_INVALID_PARAMS, data,
-				      find_fail_format(def->cb), def->name,
-				      tok->end - tok->start,
-				      buffer + tok->start);
-		return false;
+
+	if (def->cb) {
+		if (def->argsize && def->cb != (param_cb)json_tok_tok) {
+			*(void **)def->arg
+				= arg
+				= tal_arr_label(cmd, char, def->argsize, "param");
+		} else
+			arg = def->arg;
+
+		if (!def->cb(buffer, tok, arg)) {
+			command_fail_detailed(cmd, JSONRPC2_INVALID_PARAMS,
+					      make_result(cmd, def->name, buffer, tok),
+					      find_fail_format(def->cb), def->name,
+					      tok->end - tok->start,
+					      buffer + tok->start);
+			return false;
+		}
+	} else {
+		return def->cbx(cmd, def->name, buffer, tok, def->arg);
 	}
 	return true;
 }
@@ -308,10 +325,12 @@ bool param(struct command *cmd, const char *buffer,
 	va_start(ap, tokens);
 	while ((name = va_arg(ap, const char *)) != NULL) {
 		bool required = va_arg(ap, int);
-		param_cb cb = va_arg(ap, param_cb);
+		bool advanced = va_arg(ap, int);
+		param_cb cb = advanced ? NULL : va_arg(ap, param_cb);
+		param_cbx cbx = advanced ? va_arg(ap, param_cbx) : NULL;
 		void *arg = va_arg(ap, void *);
 		size_t argsize = va_arg(ap, size_t);
-		if  (!param_add(&params, name, required, cb, arg, argsize)) {
+		if  (!param_add(&params, name, required, cb, cbx, arg, argsize)) {
 			command_fail(cmd, PARAM_DEV_ERROR, "developer error");
 			va_end(ap);
 			return false;

lightningd/param.h

@@ -54,6 +54,15 @@ bool param(struct command *cmd, const char *buffer,
  */
 typedef bool(*param_cb)(const char *buffer, const jsmntok_t *tok, void *arg);
 
+/*
+ * Advanced callback. Returns NULL on success, error message on failure.
+ */
+typedef bool(*param_cbx)(struct command *cmd,
+			   const char *name,
+			   const char *buffer,
+			   const jsmntok_t *tok,
+			   void **arg);
+
 /*
  * Add a handler to unmarshal a required json token into @arg. The handler must
  * return true on success and false on failure.  Upon failure, command_fail will be
@@ -65,12 +74,25 @@ typedef bool(*param_cb)(const char *buffer, const jsmntok_t *tok, void *arg);
 #define p_req(name, cb, arg)					\
 	      name"",						\
 	      true,						\
+	      false,						\
 	      (cb),				 		\
 	      (arg) + 0*sizeof((cb)((const char *)NULL,		\
 				    (const jsmntok_t *)NULL,	\
 				    (arg)) == true),		\
 	      (size_t)0
 
+#define p_req_tal(name, cbx, arg)				\
+		  name"",					\
+		  true,						\
+		  true,						\
+		  (cbx),				 	\
+		  (arg) + 0*sizeof((cbx)((struct command *)NULL,\
+				   (const char *)NULL,		\
+				   (const char *)NULL,		\
+				   (const jsmntok_t *)NULL,	\
+				   (arg)) == true),		\
+		  (size_t)0
+
 /*
  * Similar to above but for optional parameters.
  * @arg must be the address of a pointer. If found during parsing, it will be
@@ -79,12 +101,25 @@ typedef bool(*param_cb)(const char *buffer, const jsmntok_t *tok, void *arg);
 #define p_opt(name, cb, arg)					\
 	      name"",						\
 	      false,						\
+	      false,						\
 	      (cb),				 		\
 	      (arg) + 0*sizeof((cb)((const char *)NULL,		\
 				    (const jsmntok_t *)NULL,	\
 				    *(arg)) == true),		\
 	      sizeof(**(arg))
 
+#define p_opt_tal(name, cbx, arg)				\
+		  name"",					\
+		  false,					\
+		  true,						\
+		  (cbx),				 	\
+		  (arg) + 0*sizeof((cbx)((struct command *)NULL,\
+				   (const char *)NULL,		\
+				   (const char *)NULL,		\
+				   (const jsmntok_t *)NULL,	\
+				   (arg)) == true),		\
+		  sizeof(**(arg))
+
 /*
  * Similar to p_req but for optional parameters with defaults.
  * @arg will be set to @def if it isn't found during parsing.
@@ -92,12 +127,25 @@ typedef bool(*param_cb)(const char *buffer, const jsmntok_t *tok, void *arg);
 #define p_opt_def(name, cb, arg, def)					\
 		  name"",						\
 		  false,						\
+		  false,						\
 		  (cb),							\
 		  (arg) + 0*sizeof((cb)((const char *)NULL,		\
 					(const jsmntok_t *)NULL,	\
 					(arg)) == true),		\
 		  ((void)((*arg) = (def)), (size_t)0)
 
+#define p_opt_def_tal(name, cbx, arg, def)				\
+		  name"",					\
+		  false,					\
+		  true,						\
+		  (cbx),				 	\
+		  (arg) + 0*sizeof((cbx)((struct command *)NULL,\
+				   (const char *)NULL,		\
+				   (const char *)NULL,		\
+				   (const jsmntok_t *)NULL,	\
+				   (arg)) == true),		\
+		  ({ (*arg) = tal((cmd), typeof(**arg)); (**arg) = (def); (size_t)0;})
+
 /*
  * For when you want an optional raw token.
  *
@@ -106,6 +154,7 @@ typedef bool(*param_cb)(const char *buffer, const jsmntok_t *tok, void *arg);
 #define p_opt_tok(name, arg)						\
 		  name"",						\
 		  false,						\
+		  false,						\
 		  json_tok_tok,						\
 		  (arg) + 0*sizeof(*(arg) == (jsmntok_t *)NULL),	\
 		  sizeof(const jsmntok_t *)

lightningd/test/run-param.c

@@ -17,10 +17,6 @@ static bool check_fail(void) {
 	if (!failed)
 		return false;
 	failed = false;
-	if (taken(fail_msg)) {
-		tal_free(fail_msg);
-		fail_msg = NULL;
-	}
 	return true;
 }
 
@@ -324,6 +320,7 @@ static void add_members(struct param **params,
 					      &ints[i],
 					      const char *,
 					      const jsmntok_t *),
+			  NULL,
 			  &ints[i], 0);
 	}
 }
@@ -406,6 +403,136 @@ static void sendpay_nulltok(void)
 	assert(msatoshi == NULL);
 }
 
+static bool json_tok_msat(struct command *cmd,
+			  const char *name,
+			  const char *buffer,
+			  const jsmntok_t * tok,
+			  u64 **msatoshi_val)
+{
+	if (json_tok_streq(buffer, tok, "any")) {
+		*msatoshi_val = NULL;
+		return true;
+	}
+	*msatoshi_val = tal(cmd, u64);
+
+	if (json_tok_u64(buffer, tok, *msatoshi_val) && *msatoshi_val != 0)
+		return true;
+
+	command_fail(cmd, JSONRPC2_INVALID_PARAMS,
+		     "'%s' should be a positive number or 'any', not '%.*s'",
+		     name,
+		     tok->end - tok->start,
+		     buffer + tok->start);
+	return false;
+}
+
+/* This can eventually replace json_tok_tok and we can remove the special p_opt_tok()
+ * macro. */
+static bool json_tok_tok_x(struct command *cmd,
+			   const char *name,
+			   const char *buffer,
+			   const jsmntok_t *tok,
+			   const jsmntok_t **arg)
+{
+	return (*arg = tok);
+}
+
+/*
+ * New version of json_tok_label conforming to advanced style. This can eventually
+ * replace the existing json_tok_label.
+ */
+static bool json_tok_label_x(struct command *cmd,
+			     const char *name,
+			     const char *buffer,
+			     const jsmntok_t *tok,
+			     struct json_escaped **label)
+{
+	if ((*label = json_tok_escaped_string(cmd, buffer, tok)))
+		return true;
+
+	/* Allow literal numbers */
+	if (tok->type != JSMN_PRIMITIVE)
+		goto fail;
+
+	for (int i = tok->start; i < tok->end; i++)
+		if (!cisdigit(buffer[i]))
+			goto fail;
+
+	if ((*label = json_escaped_string_(cmd, buffer + tok->start,
+					   tok->end - tok->start)))
+		return true;
+
+fail:
+	command_fail(cmd, JSONRPC2_INVALID_PARAMS,
+		     "'%s' should be a string or number, not '%.*s'",
+		     name, tok->end - tok->start, buffer + tok->start);
+	return false;
+}
+
+static void advanced(void)
+{
+	{
+		struct json *j = json_parse(cmd, "[ 'lightning', 24, 'tok', 543 ]");
+
+		struct json_escaped *label;
+		u64 *msat;
+		u64 *msat_opt1, *msat_opt2;
+		const jsmntok_t *tok;
+
+		assert(param(cmd, j->buffer, j->toks,
+			     p_req_tal("description", json_tok_label_x, &label),
+			     p_req_tal("msat", json_tok_msat, &msat),
+			     p_req_tal("tok", json_tok_tok_x, &tok),
+			     p_opt_tal("msat_opt1", json_tok_msat, &msat_opt1),
+			     p_opt_tal("msat_opt2", json_tok_msat, &msat_opt2),
+			     NULL));
+		assert(label != NULL);
+		assert(!strcmp(label->s, "lightning"));
+		assert(*msat == 24);
+		assert(tok);
+		assert(msat_opt1);
+		assert(*msat_opt1 == 543);
+		assert(msat_opt2 == NULL);
+	}
+	{
+		struct json *j = json_parse(cmd, "[ 3 'foo' ]");
+		struct json_escaped *label, *foo;
+		assert(param(cmd, j->buffer, j->toks,
+			      p_req_tal("label", json_tok_label_x, &label),
+			      p_opt_tal("foo", json_tok_label_x, &foo),
+			      NULL));
+		assert(!strcmp(label->s, "3"));
+		assert(!strcmp(foo->s, "foo"));
+	}
+	{
+		u64 *msat;
+		u64 *msat2;
+		struct json *j = json_parse(cmd, "[ 3 ]");
+		assert(param(cmd, j->buffer, j->toks,
+			      p_opt_def_tal("msat", json_tok_msat, &msat, 23),
+			      p_opt_def_tal("msat2", json_tok_msat, &msat2, 53),
+			      NULL));
+		assert(*msat == 3);
+		assert(msat2);
+		assert(*msat2 == 53);
+	}
+}
+
+static void advanced_fail(void)
+{
+	{
+		struct json *j = json_parse(cmd, "[ 'anyx' ]");
+		u64 *msat;
+		assert(!param(cmd, j->buffer, j->toks,
+			      p_req_tal("msat", json_tok_msat, &msat),
+			      NULL));
+		assert(check_fail());
+		assert(strstr(fail_msg, "'msat' should be a positive"
+			      " number or 'any', not 'anyx'"));
+	}
+}
+
+
 int main(void)
 {
 	setup_locale();
@@ -424,6 +551,8 @@ int main(void)
 	five_hundred_params();
 	sendpay();
 	sendpay_nulltok();
+	advanced();
+	advanced_fail();
 	tal_free(tmpctx);
 	printf("run-params ok\n");
 }