On Fri, Nov 03, 2023 at 05:25:14PM +0100, Thomas Haller wrote: > All "struct stmt_ops" really must have a json hook set, to handle the > statement. And almost all of them do, except "struct chain_stmt_ops". > > Soon a unit test will be added, to check that all stmt_ops have a json() > hook. Also, the missing hook in "struct chain_stmt_ops" is a bug, that > is now understood and shall be fixed soon/later. > > Note that we can already hit the bug, if we would call `nft -j list > ruleset` at the end of test "tests/shell/testcases/nft-f/sample-ruleset": > > warning: stmt ops chain have no json callback > > Soon tests will be added, that hit this condition. Printing a message to > stderr breaks those tests, and blocks adding the tests. Why not make the tests tolerate messages on stderr instead? > Drop this warning on stderr, so we can add those other tests sooner, as > those tests are useful for testing JSON code in general. The warning > stderr was useful for finding the problem, but the problem is now > understood and will be addressed separately. Drop the message to unblock > adding those tests. What do you mean with "the problem is now understood"? > Signed-off-by: Thomas Haller <thaller@xxxxxxxxxx> > --- > src/json.c | 10 ++++++++-- > src/statement.c | 1 + > 2 files changed, 9 insertions(+), 2 deletions(-) > > diff --git a/src/json.c b/src/json.c > index 25e349155394..8fff401dfb3e 100644 > --- a/src/json.c > +++ b/src/json.c > @@ -83,8 +83,14 @@ static json_t *stmt_print_json(const struct stmt *stmt, struct output_ctx *octx) > if (stmt->ops->json) > return stmt->ops->json(stmt, octx); > > - fprintf(stderr, "warning: stmt ops %s have no json callback\n", > - stmt->ops->name); Converting this to using octx->error_fp does not help? > + /* In general, all "struct stmt_ops" must implement json() hook. Otherwise > + * we have a bug, and a unit test should check that all ops are correct. > + * > + * Currently, "chain_stmt_ops.json" is known to be NULL. That is a bug that > + * needs fixing. > + * > + * After the bug is fixed, and the unit test in place, this fallback code > + * can be dropped. */ How will those unit tests cover new statements added at a later time? We don't have a registration process, are you planning to discover them based on enum stmt_types or something like that? Cheers, Phil
