[PATCH nft v3 1/2] json: drop handling missing json() hook in expr_print_json()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



There are only two "struct expr_ops" that don't have a json() hook set
("symbol_expr_ops", "variable_exrp_ops"). For those, we never expect to
call expr_print_json().

All other "struct expr_ops" must have a hook set. Soon a unit test shall
be added to also ensure that for all expr_ops (except EXPR_SYMBOL and
EXPR_VARIABLE).

It thus should not be possible to ever call expr_print_json() with a
NULL hook. Drop the code that tries to handle that.

The previous code was essentially a graceful assertion, which only
prints a message to stderr (instead of assert()/abort()) and implemented
a fallback solution. The fallback solution is not really useful, because
it's just the bison string which cannot be parsed back.

This seems too much effort trying to handle a potential bug, for
something that we most likely will not be wrong (once the unit test is
in place). Drop the fallback solution and just require the bug not to be
present. We now get a hard crash if the requirement is violated.

Also add code comments hinting to all of this.

Signed-off-by: Thomas Haller <thaller@xxxxxxxxxx>
---
 src/expression.c |  2 ++
 src/json.c       | 28 ++++++++--------------------
 2 files changed, 10 insertions(+), 20 deletions(-)

diff --git a/src/expression.c b/src/expression.c
index a21dfec25722..53fa630e099c 100644
--- a/src/expression.c
+++ b/src/expression.c
@@ -321,6 +321,7 @@ static const struct expr_ops symbol_expr_ops = {
 	.type		= EXPR_SYMBOL,
 	.name		= "symbol",
 	.print		= symbol_expr_print,
+	.json		= NULL, /* expr_print_json() must never be called. */
 	.clone		= symbol_expr_clone,
 	.destroy	= symbol_expr_destroy,
 };
@@ -362,6 +363,7 @@ static const struct expr_ops variable_expr_ops = {
 	.type		= EXPR_VARIABLE,
 	.name		= "variable",
 	.print		= variable_expr_print,
+	.json		= NULL, /* expr_print_json() must never be called. */
 	.clone		= variable_expr_clone,
 	.destroy	= variable_expr_destroy,
 };
diff --git a/src/json.c b/src/json.c
index 068c423addc7..25e349155394 100644
--- a/src/json.c
+++ b/src/json.c
@@ -44,26 +44,14 @@
 
 static json_t *expr_print_json(const struct expr *expr, struct output_ctx *octx)
 {
-	const struct expr_ops *ops;
-	char buf[1024];
-	FILE *fp;
-
-	ops = expr_ops(expr);
-	if (ops->json)
-		return ops->json(expr, octx);
-
-	fprintf(stderr, "warning: expr ops %s have no json callback\n",
-		expr_name(expr));
-
-	fp = octx->output_fp;
-	octx->output_fp = fmemopen(buf, 1024, "w");
-
-	ops->print(expr, octx);
-
-	fclose(octx->output_fp);
-	octx->output_fp = fp;
-
-	return json_pack("s", buf);
+	/* The json() hooks of "symbol_expr_ops" and "variable_expr_ops" are
+	 * known to be NULL, but for such expressions we never expect to call
+	 * expr_print_json().
+	 *
+	 * All other expr_ops must have a json() hook.
+	 *
+	 * Unconditionally access the hook (and segfault in case of a bug).  */
+	return expr_ops(expr)->json(expr, octx);
 }
 
 static json_t *set_dtype_json(const struct expr *key)
-- 
2.41.0




[Index of Archives]     [Netfitler Users]     [Berkeley Packet Filter]     [LARTC]     [Bugtraq]     [Yosemite Forum]

  Powered by Linux