params: make optional args do allocation for you.

This is a bit more natural, IMHO.  The only issue is that json_tok_tok is
special, so we end up with param_opt_tok() if you really want an optional
generic token.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

lightningd/params.c

@@ -7,19 +7,16 @@
 #include <lightningd/params.h>
 
 struct param {
+	const tal_t *ctx;
 	char *name;
-	bool required;
 	bool is_set;
 	param_cb cb;
 	void *arg;
+	size_t argsize;
 };
 
-void *param_is_set(struct param *def)
-{
-	return def->is_set ? def->arg : NULL;
-}
-
-struct param *param_add_(bool required, char *name, param_cb cb, void *arg)
+struct param *param_add_(const tal_t *ctx,
+			 char *name, param_cb cb, void *arg, size_t argsize)
 {
 #if DEVELOPER
 	assert(name);
@@ -27,11 +24,29 @@ struct param *param_add_(bool required, char *name, param_cb cb, void *arg)
 	assert(arg);
 #endif
 	struct param *last = tal(tmpctx, struct param);
+	last->ctx = ctx;
 	last->is_set = false;
 	last->name = tal_strdup(last, name);
 	last->cb = cb;
 	last->arg = arg;
-	last->required = required;
+	last->argsize = argsize;
+	/* 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, char *name, const jsmntok_t **tok)
+{
+	struct param *last = tal(tmpctx, struct param);
+	assert(ctx);
+	last->ctx = ctx;
+	last->is_set = false;
+	last->name = tal_strdup(last, name);
+	last->cb = (param_cb)json_tok_tok;
+	last->arg = tok;
+	last->argsize = sizeof(*tok);
+	*tok = NULL;
 	return last;
 }
 
@@ -66,8 +81,16 @@ static bool make_callback(struct command *cmd,
 			  struct param *def,
 			  const char *buffer, const jsmntok_t * tok)
 {
+	void *arg;
 	def->is_set = true;
-	if (!def->cb(buffer, tok, def->arg)) {
+	if (def->argsize && def->cb != (param_cb)json_tok_tok) {
+		*(void **)def->arg
+			= arg
+			= tal_alloc_(def->ctx, def->argsize, false, false,
+				     "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);
@@ -89,7 +112,7 @@ static struct param **post_check(struct command *cmd, struct param **params)
 	struct param **last = first + tal_count(params);
 
 	/* Make sure required params were provided. */
-	while (first != last && (*first)->required) {
+	while (first != last && (*first)->argsize == 0) {
 		if (!(*first)->is_set) {
 			command_fail(cmd, JSONRPC2_INVALID_PARAMS,
 				     "missing required parameter: '%s'",
@@ -98,16 +121,6 @@ static struct param **post_check(struct command *cmd, struct param **params)
 		}
 		first++;
 	}
-
-	/* Set optional missing jsmntok_t args to NULL.  */
-	while (first != last) {
-		struct param *p = *first;
-		if (!p->is_set && (p->cb == (param_cb) json_tok_tok)) {
-			jsmntok_t **tok = p->arg;
-			*tok = NULL;
-		}
-		first++;
-	}
 	return params;
 }
 
@@ -208,8 +221,8 @@ static int comp_by_arg(const void *a, const void *b)
  */
 static int comp_req_order(const void *a, const void *b)
 {
-	bool x = (bool) ((*(const struct param **) a)->required);
-	bool y = (bool) ((*(const struct param **) b)->required);
+	bool x = (bool) ((*(const struct param **) a)->argsize == 0);
+	bool y = (bool) ((*(const struct param **) b)->argsize == 0);
 	if (!x && y)
 		return 0;
 	return 1;

lightningd/params.h

@@ -11,13 +11,12 @@ struct param;
   Typical usage:
 	unsigned cltv;
 	const jsmntok_t *note;
-	u64 msatoshi;
-	struct param * mp;
+	u64 *msatoshi;
 
 	if (!param_parse(cmd, buffer, tokens,
 			 param_req("cltv", json_tok_number, &cltv),
-			 param_opt("note", json_tok_tok, &note),
-			 mp = param_opt("msatoshi", json_tok_u64, &msatoshi),
+			 param_opt_tok("note", &note),
+			 param_opt("msatoshi", json_tok_u64, &msatoshi),
 			 NULL))
 		return;
 
@@ -28,16 +27,12 @@ struct param;
 
   cltv is a required parameter, and is set correctly.
 
-  note and msatoshi are optional parameters.  You can see if they have been set
-  by calling param_is_set(); e.g.:
+  note and msatoshi are optional parameters.  Their argument will be set to NULL
+  if they are not provided.
 
-	if (param_is_set(mp))
-		do_something()
-
-  The note parameter uses a special callback, json_tok_tok(). It
-  simply sets seedtok to the appropriate value and lets the handler do the
-  validating. It has the added feature of setting seedtok to NULL if it is null
-  or not specified.
+  The note parameter uses a special callback, param_opt_tok: it
+  simply sets note to the appropriate value (or NULL) and lets the
+  handler do the validating.
 
   There are canned failure messages for common callbacks. An example:
 
@@ -65,29 +60,27 @@ 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_(true, name, \
+		  param_add_(NULL, name,			     \
 			     typesafe_cb_preargs(bool, void *,       \
 						 (cb), (arg),        \
 						 const char *,       \
 						 const jsmntok_t *), \
-			     (arg))
+			     (arg), 0)
 /*
  * Same as above but for optional parameters.
  */
-#define param_opt(name, cb, arg)          \
-		  param_add_(false, name, \
+#define param_opt(ctx, name, cb, arg)	  \
+		  param_add_(ctx, name,				     \
 			     typesafe_cb_preargs(bool, void *,       \
-						 (cb), (arg),        \
+						 (cb), *(arg),       \
 						 const char *,       \
 						 const jsmntok_t *), \
-			     (arg))
-struct param * param_add_(bool required, char *name, param_cb cb, void *arg);
+			     (arg), sizeof(**arg))
+struct param * param_add_(const tal_t *ctx, char *name, param_cb cb, void *arg, size_t argsize);
 
-/*
- * Check to see if an optional parameter was set during parsing (although it
- * works for all parameters).
- * Returns the @arg if set, otherwise NULL.
- */
-void * param_is_set(struct param *p);
+#define param_opt_tok(ctx, name, arg)		\
+	param_opt_add_(ctx, name, arg)
+
+struct param *param_opt_add_(const tal_t *ctx, char *name, const jsmntok_t **tok);
 
 #endif /* LIGHTNING_LIGHTNINGD_PARAMS_H */

lightningd/test/run-params.c

@@ -172,8 +172,7 @@ static void tok_tok(void)
 
 		struct json *j = json_parse(cmd, "{}");
 		assert(param_parse(cmd, j->buffer, j->toks,
-				   param_opt("satoshi", json_tok_tok,
-					     &tok), NULL));
+				   param_opt_tok(cmd, "satoshi", &tok), NULL));
 
 		/* make sure it *is* NULL */
 		assert(tok == NULL);
@@ -196,7 +195,8 @@ static void dup(void)
 
 static void null_params(void)
 {
-	uint64_t *ints = tal_arr(cmd, uint64_t, 7);
+	uint64_t *ints = tal_arr(cmd, uint64_t, 4);
+	uint64_t **intptrs = tal_arr(cmd, uint64_t *, 3);
 	/* no null params */
 	struct json *j =
 	    json_parse(cmd, "[ '10', '11', '12', '13', '14', '15', '16']");
@@ -208,31 +208,34 @@ static void null_params(void)
 			   param_req("1", json_tok_u64, &ints[1]),
 			   param_req("2", json_tok_u64, &ints[2]),
 			   param_req("3", json_tok_u64, &ints[3]),
-			   param_opt("4", json_tok_u64, &ints[4]),
-			   param_opt("5", json_tok_u64, &ints[5]),
-			   param_opt("6", json_tok_u64, &ints[6]), NULL));
+			   param_opt(tmpctx, "4", json_tok_u64, &intptrs[0]),
+			   param_opt(tmpctx, "5", json_tok_u64, &intptrs[1]),
+			   param_opt(tmpctx, "6", json_tok_u64, &intptrs[2]),
+			   NULL));
 	for (int i = 0; i < tal_count(ints); ++i)
 		assert(ints[i] == i + 10);
+	for (int i = 0; i < tal_count(intptrs); ++i)
+		assert(*intptrs[i] == i + 10 + tal_count(ints));
 
 	/* missing at end */
 	for (int i = 0; i < tal_count(ints); ++i)
 		ints[i] = 42;
+	for (int i = 0; i < tal_count(intptrs); ++i)
+		intptrs[i] = (void *)42;
 
 	j = json_parse(cmd, "[ '10', '11', '12', '13', '14']");
-	struct param *four, *five;
 	assert(param_parse(cmd, j->buffer, j->toks,
 			   param_req("0", json_tok_u64, &ints[0]),
 			   param_req("1", json_tok_u64, &ints[1]),
 			   param_req("2", json_tok_u64, &ints[2]),
 			   param_req("3", json_tok_u64, &ints[3]),
-			   four = param_opt("4", json_tok_u64, &ints[4]),
-			   five = param_opt("5", json_tok_u64, &ints[5]),
-			   param_opt("6", json_tok_u64, &ints[6]), NULL));
-	assert(ints[4] == 14);
-	assert(param_is_set(four) == &ints[4]);
-	assert(!param_is_set(five));
-	assert(ints[5] == 42);
-	assert(ints[6] == 42);
+			   param_opt(tmpctx, "4", json_tok_u64, &intptrs[0]),
+			   param_opt(tmpctx, "5", json_tok_u64, &intptrs[1]),
+			   param_opt(tmpctx, "6", json_tok_u64, &intptrs[2]),
+			   NULL));
+	assert(*intptrs[0] == 14);
+	assert(intptrs[1] == NULL);
+	assert(intptrs[2] == NULL);
 }
 
 #if DEVELOPER
@@ -340,12 +343,13 @@ static void bad_programmer(void)
 		/* Add required param after optional */
 		struct json *j =
 		    json_parse(cmd, "[ '25', '546', '26', '1.1' ]");
-		unsigned int msatoshi;
+		unsigned int *msatoshi;
 		double riskfactor;
 		param_parse(cmd, j->buffer, j->toks,
 			    param_req("u64", json_tok_u64, &ival),
 			    param_req("double", json_tok_double, &dval),
-			    param_opt("msatoshi", json_tok_number, &msatoshi),
+			    param_opt(tmpctx, "msatoshi",
+				      json_tok_number, &msatoshi),
 			    param_req("riskfactor", json_tok_double,
 				      &riskfactor), NULL);
 		restore_assert();
@@ -409,26 +413,27 @@ static void sendpay(void)
 	struct json *j = json_parse(cmd, "[ 'A', '123', 'hello there' '547']");
 
 	const jsmntok_t *routetok, *note;
-	u64 msatoshi;
+	u64 *msatoshi;
 	unsigned cltv;
-	struct param * mp;
 
 	if (!param_parse(cmd, j->buffer, j->toks,
 			 param_req("route", json_tok_tok, &routetok),
 			 param_req("cltv", json_tok_number, &cltv),
-			 param_opt("note", json_tok_tok, &note),
-			 mp = param_opt("msatoshi", json_tok_u64, &msatoshi),
+			 param_opt_tok(tmpctx, "note", &note),
+			 param_opt(tmpctx, "msatoshi", json_tok_u64, &msatoshi),
 			 NULL))
 		assert(false);
 
-	assert(param_is_set(mp));
-	assert(msatoshi == 547);
+	assert(note);
+	assert(msatoshi);
+	assert(*msatoshi == 547);
 }
 
 int main(void)
 {
 	setup_locale();
-	cmd = tal(NULL, struct command);
+	setup_tmpctx();
+	cmd = tal(tmpctx, struct command);
 	fail_msg = tal_arr(cmd, char, 10000);
 
 	zero_params();
@@ -441,6 +446,6 @@ int main(void)
 	dup();
 	five_hundred_params();
 	sendpay();
-	tal_free(cmd);
+	tal_free(tmpctx);
 	printf("run-params ok\n");
 }