[libnftables PATCH 0/6] parsing update

[libnftables PATCH 0/6] parsing update

[Arturo Borrero Gonzalez]

The following series implements a parsing update for libnftables.

The first two patches implements two minor fixes I found while working
with the nft_*_parse_file() funcs.

The last four patches are related to the file parsing.

Comments are welcome.

PS: I'm sorry, it seems my diffstat is not working properly :-(

---

Arturo Borrero Gonzalez (6):
      mxml: add error reference of the top node
      set_elem: add json parsing to API
      internal: rework parsing symbol logic
      internal: add a selector for parsing ops
      parsing: add interface to parse from file
      tests: update tests with nft_*_parse_file()


 tests/nft-parsing-test.c |  102 +++++++++++++++++++++++++++-------------------
 1 file changed, 59 insertions(+), 43 deletions(-)

-- 
Arturo Borrero Gonzalez

[libnftables PATCH 1/6] mxml: add error reference of the top node

[Arturo Borrero Gonzalez]

We know the top node we are building. Let the user also know it.

Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
---
 0 files changed

diff --git a/src/mxml.c b/src/mxml.c
index bd09bb3..bc0f084 100644
--- a/src/mxml.c
+++ b/src/mxml.c
@@ -30,16 +30,19 @@ mxml_node_t *nft_mxml_build_tree(const char *xml, const char *treename,
 	tree = mxmlLoadString(NULL, xml, MXML_OPAQUE_CALLBACK);
 	if (tree == NULL) {
 		err->error = NFT_PARSE_EBADINPUT;
-		err->line = 0;
-		err->column = 0;
 		goto err;
 	}
 
 	if (strcmp(tree->value.opaque, treename) == 0)
 		return tree;
 
+	err->error = NFT_PARSE_EMISSINGNODE;
+	err->node_name = treename;
+
 	mxmlDelete(tree);
 err:
+	err->line = 0;
+	err->column = 0;
 	errno = EINVAL;
 	return NULL;
 }

[libnftables PATCH 2/6] set_elem: add json parsing to API

[Arturo Borrero Gonzalez]

Add missing support in the API function to parse a JSON set_elem.

I've renamed the main JSON parsing function to prevent clashing.

Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
---
 0 files changed

diff --git a/src/internal.h b/src/internal.h
index 9ef505f..5fef6d6 100644
--- a/src/internal.h
+++ b/src/internal.h
@@ -105,8 +105,8 @@ int nft_jansson_data_reg_parse(json_t *root, const char *node_name,
 			       union nft_data_reg *data_reg,
 			       struct nft_parse_err *err);
 struct nft_set_elem;
-int nft_set_elem_json_parse(struct nft_set_elem *e, json_t *root,
-			    struct nft_parse_err *err);
+int nft_jansson_set_elem_parse(struct nft_set_elem *e, json_t *root,
+			       struct nft_parse_err *err);
 struct nft_table;
 int nft_jansson_parse_table(struct nft_table *t, json_t *tree,
 			    struct nft_parse_err *err);
diff --git a/src/jansson.c b/src/jansson.c
index 54a56b9..e62116b 100644
--- a/src/jansson.c
+++ b/src/jansson.c
@@ -244,8 +244,8 @@ int nft_jansson_data_reg_parse(json_t *root, const char *node_name,
 	}
 }
 
-int nft_set_elem_json_parse(struct nft_set_elem *e, json_t *root,
-			    struct nft_parse_err *err)
+int nft_jansson_set_elem_parse(struct nft_set_elem *e, json_t *root,
+			       struct nft_parse_err *err)
 {
 	uint32_t uval32;
 	int set_elem_data;
diff --git a/src/set.c b/src/set.c
index 32c7fff..9317b9c 100644
--- a/src/set.c
+++ b/src/set.c
@@ -357,7 +357,8 @@ int nft_jansson_parse_set(struct nft_set *s, json_t *tree,
 			if (json_elem == NULL)
 				goto err;
 
-			if (nft_set_elem_json_parse(elem, json_elem, err) < 0)
+			if (nft_jansson_set_elem_parse(elem,
+						       json_elem, err) < 0)
 				goto err;
 
 			list_add_tail(&elem->head, &s->element_list);
diff --git a/src/set_elem.c b/src/set_elem.c
index 7365aff..14bf6f4 100644
--- a/src/set_elem.c
+++ b/src/set_elem.c
@@ -414,6 +414,24 @@ static int nft_set_elem_xml_parse(struct nft_set_elem *e, const char *xml,
 #endif
 }
 
+static int nft_set_elem_json_parse(struct nft_set_elem *e, const void *json,
+				   struct nft_parse_err *err)
+{
+#ifdef JSON_PARSING
+	json_t *tree;
+	json_error_t error;
+
+	tree = nft_jansson_create_root(json, &error, err);
+	if (tree == NULL)
+		return -1;
+
+	return nft_jansson_set_elem_parse(e, tree, err);
+#else
+	errno = EOPNOTSUPP;
+	return -1;
+#endif
+}
+
 int nft_set_elem_parse(struct nft_set_elem *e,
 		       enum nft_parse_type type, const char *data,
 		       struct nft_parse_err *err) {
@@ -423,6 +441,9 @@ int nft_set_elem_parse(struct nft_set_elem *e,
 	case NFT_PARSE_XML:
 		ret = nft_set_elem_xml_parse(e, data, err);
 		break;
+	case NFT_PARSE_JSON:
+		ret = nft_set_elem_json_parse(e, data, err);
+		break;
 	default:
 		errno = EOPNOTSUPP;
 		ret = -1;

[libnftables PATCH 3/6] internal: rework parsing symbol logic

[Arturo Borrero Gonzalez]

Rework parsing symbols logic, so we get a cleaner file.

Also, this allows us to use XML(libmxml)/JSON(libjannson) internal datatypes
without fear.

Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
---
 0 files changed

diff --git a/src/internal.h b/src/internal.h
index 5fef6d6..83cb4e8 100644
--- a/src/internal.h
+++ b/src/internal.h
@@ -40,6 +40,10 @@ struct nft_parse_err {
 
 #ifdef XML_PARSING
 #include <mxml.h>
+#else
+#define mxml_node_t void
+#endif /* XML_PARSING */
+
 #define NFT_XML_MAND 0
 #define NFT_XML_OPT (1 << 0)
 mxml_node_t *nft_mxml_build_tree(const char *xml, const char *treename,
@@ -78,10 +82,13 @@ int nft_mxml_rule_parse(mxml_node_t *tree, struct nft_rule *r,
 struct nft_set;
 int nft_mxml_set_parse(mxml_node_t *tree, struct nft_set *s,
 		       struct nft_parse_err *err);
-#endif
 
 #ifdef JSON_PARSING
 #include <jansson.h>
+#else
+#define json_t void
+#define json_err_t void
+#endif /* JSON_PARSING */
 
 int nft_jansson_parse_val(json_t *root, const char *node_name, int type,
 			  void *out, struct nft_parse_err *err);
@@ -119,7 +126,6 @@ int nft_jansson_parse_rule(struct nft_rule *r, json_t *tree,
 struct nft_set;
 int nft_jansson_parse_set(struct nft_set *s, json_t *tree,
 			  struct nft_parse_err *err);
-#endif
 
 const char *nft_family2str(uint32_t family);
 int nft_str2family(const char *family);

[libnftables PATCH 4/6] internal: add a selector for parsing ops

[Arturo Borrero Gonzalez]

This patch adds a selector for parsing operations.

The selector allows to reduce code duplication in case more than
one parsing interface are used.

Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
---
 0 files changed

diff --git a/src/chain.c b/src/chain.c
index a4ddb06..fd56629 100644
--- a/src/chain.c
+++ b/src/chain.c
@@ -592,13 +592,14 @@ err:
 #endif
 
 static int nft_chain_json_parse(struct nft_chain *c, const char *json,
-				struct nft_parse_err *err)
+				struct nft_parse_err *err,
+				const struct nft_parse_ops *ops)
 {
 #ifdef JSON_PARSING
 	json_t *tree;
 	json_error_t error;
 
-	tree = nft_jansson_create_root(json, &error, err);
+	tree = ops->jsonbuilder(json, &error, err);
 	if (tree == NULL)
 		return -1;
 
@@ -709,11 +710,12 @@ int nft_mxml_chain_parse(mxml_node_t *tree, struct nft_chain *c,
 #endif
 
 static int nft_chain_xml_parse(struct nft_chain *c, const char *xml,
-			       struct nft_parse_err *err)
+			       struct nft_parse_err *err,
+			       const struct nft_parse_ops *ops)
 {
 #ifdef XML_PARSING
 	int ret;
-	mxml_node_t *tree = nft_mxml_build_tree(xml, "chain", err);
+	mxml_node_t *tree = ops->xmlbuilder(xml, "chain", err);
 	if (tree == NULL)
 		return -1;
 
@@ -726,18 +728,19 @@ static int nft_chain_xml_parse(struct nft_chain *c, const char *xml,
 #endif
 }
 
-int nft_chain_parse(struct nft_chain *c, enum nft_parse_type type,
-		    const char *data, struct nft_parse_err *err)
+static int nft_chain_do_parse(struct nft_chain *c, enum nft_parse_type type,
+			      const void *data, struct nft_parse_err *err,
+			      const struct nft_parse_ops *ops)
 {
 	int ret;
 	struct nft_parse_err perr;
 
 	switch (type) {
 	case NFT_PARSE_XML:
-		ret = nft_chain_xml_parse(c, data, &perr);
+		ret = nft_chain_xml_parse(c, data, &perr, ops);
 		break;
 	case NFT_PARSE_JSON:
-		ret = nft_chain_json_parse(c, data, &perr);
+		ret = nft_chain_json_parse(c, data, &perr, ops);
 		break;
 	default:
 		ret = -1;
@@ -750,6 +753,12 @@ int nft_chain_parse(struct nft_chain *c, enum nft_parse_type type,
 
 	return ret;
 }
+
+int nft_chain_parse(struct nft_chain *c, enum nft_parse_type type,
+		    const char *data, struct nft_parse_err *err)
+{
+	return nft_chain_do_parse(c, type, data, err, &nft_parse_string_ops);
+}
 EXPORT_SYMBOL(nft_chain_parse);
 
 static int nft_chain_snprintf_json(char *buf, size_t size, struct nft_chain *c)
diff --git a/src/internal.h b/src/internal.h
index 83cb4e8..afd8816 100644
--- a/src/internal.h
+++ b/src/internal.h
@@ -46,7 +46,7 @@ struct nft_parse_err {
 
 #define NFT_XML_MAND 0
 #define NFT_XML_OPT (1 << 0)
-mxml_node_t *nft_mxml_build_tree(const char *xml, const char *treename,
+mxml_node_t *nft_mxml_build_tree(const void *data, const char *treename,
 				 struct nft_parse_err *err);
 struct nft_rule_expr *nft_mxml_expr_parse(mxml_node_t *node,
 					  struct nft_parse_err *err);
@@ -95,7 +95,7 @@ int nft_jansson_parse_val(json_t *root, const char *node_name, int type,
 const char *nft_jansson_parse_str(json_t *root, const char *node_name,
 				  struct nft_parse_err *err);
 bool nft_jansson_node_exist(json_t *root, const char *node_name);
-json_t *nft_jansson_create_root(const char *json, json_error_t *error,
+json_t *nft_jansson_create_root(const void *data, json_error_t *error,
 				struct nft_parse_err *err);
 json_t *nft_jansson_get_node(json_t *root, const char *node_name,
 			     struct nft_parse_err *err);
@@ -127,6 +127,24 @@ struct nft_set;
 int nft_jansson_parse_set(struct nft_set *s, json_t *tree,
 			  struct nft_parse_err *err);
 
+struct nft_parse_ops {
+	mxml_node_t *(*xmlbuilder)(const void *input_data,
+				   const char *treename,
+				   struct nft_parse_err *err);
+	json_t *(*jsonbuilder)(const void *input_data,
+			       json_error_t *jerror,
+			       struct nft_parse_err *err);
+};
+
+static const struct nft_parse_ops nft_parse_string_ops = {
+#ifdef XML_PARSING
+	.xmlbuilder = nft_mxml_build_tree,
+#endif
+#ifdef JSON_PARSING
+	.jsonbuilder = nft_jansson_create_root
+#endif
+};
+
 const char *nft_family2str(uint32_t family);
 int nft_str2family(const char *family);
 int nft_strtoi(const char *string, int base, void *number, enum nft_type type);
diff --git a/src/jansson.c b/src/jansson.c
index e62116b..90faeca 100644
--- a/src/jansson.c
+++ b/src/jansson.c
@@ -89,8 +89,9 @@ bool nft_jansson_node_exist(json_t *root, const char *node_name)
 	return json_object_get(root, node_name) != NULL;
 }
 
-json_t *nft_jansson_create_root(const char *json, json_error_t *error,
-				struct nft_parse_err *err)
+static json_t *
+nft_jansson_do_create_root(const char *json, json_error_t *error,
+			   struct nft_parse_err *err)
 {
 	json_t *root;
 
@@ -107,6 +108,12 @@ json_t *nft_jansson_create_root(const char *json, json_error_t *error,
 	return root;
 }
 
+json_t *nft_jansson_create_root(const void *data, json_error_t *error,
+				struct nft_parse_err *err)
+{
+	return nft_jansson_do_create_root(data, error, err);
+}
+
 json_t *nft_jansson_get_node(json_t *root, const char *node_name,
 			     struct nft_parse_err *err)
 {
diff --git a/src/mxml.c b/src/mxml.c
index bc0f084..13a8b89 100644
--- a/src/mxml.c
+++ b/src/mxml.c
@@ -22,8 +22,9 @@
 #include <libnftables/set.h>
 
 #ifdef XML_PARSING
-mxml_node_t *nft_mxml_build_tree(const char *xml, const char *treename,
-				 struct nft_parse_err *err)
+static mxml_node_t *
+nft_mxml_do_build_tree(const char *xml, const char *treename,
+		       struct nft_parse_err *err)
 {
 	mxml_node_t *tree;
 
@@ -47,6 +48,12 @@ err:
 	return NULL;
 }
 
+mxml_node_t *nft_mxml_build_tree(const void *data, const char *treename,
+				 struct nft_parse_err *err)
+{
+	return nft_mxml_do_build_tree(data, treename, err);
+}
+
 struct nft_rule_expr *nft_mxml_expr_parse(mxml_node_t *node,
 					  struct nft_parse_err *err)
 {
diff --git a/src/rule.c b/src/rule.c
index 2e35aba..6f5718d 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -527,13 +527,14 @@ err:
 #endif
 
 static int nft_rule_json_parse(struct nft_rule *r, const char *json,
-			       struct nft_parse_err *err)
+			       struct nft_parse_err *err,
+			       const struct nft_parse_ops *ops)
 {
 #ifdef JSON_PARSING
 	json_t *tree;
 	json_error_t error;
 
-	tree = nft_jansson_create_root(json, &error, err);
+	tree = ops->jsonbuilder(json, &error, err);
 	if (tree == NULL)
 		return -1;
 
@@ -628,11 +629,12 @@ int nft_mxml_rule_parse(mxml_node_t *tree, struct nft_rule *r,
 #endif
 
 static int nft_rule_xml_parse(struct nft_rule *r, const char *xml,
-			      struct nft_parse_err *err)
+			      struct nft_parse_err *err,
+			      const struct nft_parse_ops *ops)
 {
 #ifdef XML_PARSING
 	int ret;
-	mxml_node_t *tree = nft_mxml_build_tree(xml, "rule", err);
+	mxml_node_t *tree = ops->xmlbuilder(xml, "rule", err);
 	if (tree == NULL)
 		return -1;
 
@@ -645,18 +647,19 @@ static int nft_rule_xml_parse(struct nft_rule *r, const char *xml,
 #endif
 }
 
-int nft_rule_parse(struct nft_rule *r, enum nft_parse_type type,
-		   const char *data, struct nft_parse_err *err)
+static int nft_rule_do_parse(struct nft_rule *r, enum nft_parse_type type,
+			     const void *data, struct nft_parse_err *err,
+			     const struct nft_parse_ops *ops)
 {
 	int ret;
 	struct nft_parse_err perr;
 
 	switch (type) {
 	case NFT_PARSE_XML:
-		ret = nft_rule_xml_parse(r, data, &perr);
+		ret = nft_rule_xml_parse(r, data, &perr, ops);
 		break;
 	case NFT_PARSE_JSON:
-		ret = nft_rule_json_parse(r, data, &perr);
+		ret = nft_rule_json_parse(r, data, &perr, ops);
 		break;
 	default:
 		ret = -1;
@@ -668,6 +671,12 @@ int nft_rule_parse(struct nft_rule *r, enum nft_parse_type type,
 
 	return ret;
 }
+
+int nft_rule_parse(struct nft_rule *r, enum nft_parse_type type,
+		   const char *data, struct nft_parse_err *err)
+{
+	return nft_rule_do_parse(r, type, data, err, &nft_parse_string_ops);
+}
 EXPORT_SYMBOL(nft_rule_parse);
 
 static int nft_rule_snprintf_json(char *buf, size_t size, struct nft_rule *r,
diff --git a/src/ruleset.c b/src/ruleset.c
index a12efa9..84ada20 100644
--- a/src/ruleset.c
+++ b/src/ruleset.c
@@ -331,13 +331,14 @@ err:
 #endif
 
 static int nft_ruleset_json_parse(struct nft_ruleset *rs, const char *json,
-				  struct nft_parse_err *err)
+				  struct nft_parse_err *err,
+				  const struct nft_parse_ops *ops)
 {
 #ifdef JSON_PARSING
 	json_t *root, *array;
 	json_error_t error;
 
-	root = nft_jansson_create_root(json, &error, err);
+	root = ops->jsonbuilder(json, &error, err);
 	if (root == NULL)
 		return -1;
 
@@ -535,12 +536,13 @@ err_free:
 #endif
 
 static int nft_ruleset_xml_parse(struct nft_ruleset *rs, const char *xml,
-				 struct nft_parse_err *err)
+				 struct nft_parse_err *err,
+				 const struct nft_parse_ops *ops)
 {
 #ifdef XML_PARSING
 	mxml_node_t *tree;
 
-	tree = nft_mxml_build_tree(xml, "nftables", err);
+	tree = ops->xmlbuilder(xml, "nftables", err);
 	if (tree == NULL)
 		return -1;
 
@@ -567,17 +569,19 @@ err:
 #endif
 }
 
-int nft_ruleset_parse(struct nft_ruleset *r, enum nft_parse_type type,
-		      const char *data, struct nft_parse_err *err)
+static int
+nft_ruleset_do_parse(struct nft_ruleset *r, enum nft_parse_type type,
+		     const void *data, struct nft_parse_err *err,
+		     const struct nft_parse_ops *ops)
 {
 	int ret;
 
 	switch (type) {
 	case NFT_PARSE_XML:
-		ret = nft_ruleset_xml_parse(r, data, err);
+		ret = nft_ruleset_xml_parse(r, data, err, ops);
 		break;
 	case NFT_PARSE_JSON:
-		ret = nft_ruleset_json_parse(r, data, err);
+		ret = nft_ruleset_json_parse(r, data, err, ops);
 		break;
 	default:
 		ret = -1;
@@ -587,6 +591,13 @@ int nft_ruleset_parse(struct nft_ruleset *r, enum nft_parse_type type,
 
 	return ret;
 }
+
+int nft_ruleset_parse(struct nft_ruleset *rs, enum nft_parse_type type,
+		      const char *data, struct nft_parse_err *err)
+{
+	return nft_ruleset_do_parse(rs, type, data, err,
+				    &nft_parse_string_ops);
+}
 EXPORT_SYMBOL(nft_ruleset_parse);
 
 static const char *nft_ruleset_o_opentag(uint32_t type)
diff --git a/src/set.c b/src/set.c
index 9317b9c..248619c 100644
--- a/src/set.c
+++ b/src/set.c
@@ -376,13 +376,14 @@ err:
 #endif
 
 static int nft_set_json_parse(struct nft_set *s, const char *json,
-			      struct nft_parse_err *err)
+			      struct nft_parse_err *err,
+			      const struct nft_parse_ops *ops)
 {
 #ifdef JSON_PARSING
 	json_t *tree;
 	json_error_t error;
 
-	tree = nft_jansson_create_root(json, &error, err);
+	tree = ops->jsonbuilder(json, &error, err);
 	if (tree == NULL)
 		return -1;
 
@@ -484,11 +485,12 @@ int nft_mxml_set_parse(mxml_node_t *tree, struct nft_set *s,
 #endif
 
 static int nft_set_xml_parse(struct nft_set *s, const char *xml,
-			     struct nft_parse_err *err)
+			     struct nft_parse_err *err,
+			     const struct nft_parse_ops *ops)
 {
 #ifdef XML_PARSING
 	int ret;
-	mxml_node_t *tree = nft_mxml_build_tree(xml, "set", err);
+	mxml_node_t *tree = ops->xmlbuilder(xml, "set", err);
 	if (tree == NULL)
 		return -1;
 
@@ -501,18 +503,19 @@ static int nft_set_xml_parse(struct nft_set *s, const char *xml,
 #endif
 }
 
-int nft_set_parse(struct nft_set *s, enum nft_parse_type type,
-		  const char *data, struct nft_parse_err *err)
+static int nft_set_do_parse(struct nft_set *s, enum nft_parse_type type,
+			    const void *data, struct nft_parse_err *err,
+			    const struct nft_parse_ops *ops)
 {
 	int ret;
 	struct nft_parse_err perr;
 
 	switch (type) {
 	case NFT_PARSE_XML:
-		ret = nft_set_xml_parse(s, data, &perr);
+		ret = nft_set_xml_parse(s, data, &perr, ops);
 		break;
 	case NFT_PARSE_JSON:
-		ret = nft_set_json_parse(s, data, &perr);
+		ret = nft_set_json_parse(s, data, &perr, ops);
 		break;
 	default:
 		ret = -1;
@@ -525,6 +528,12 @@ int nft_set_parse(struct nft_set *s, enum nft_parse_type type,
 
 	return ret;
 }
+
+int nft_set_parse(struct nft_set *s, enum nft_parse_type type,
+		  const char *data, struct nft_parse_err *err)
+{
+	return nft_set_do_parse(s, type, data, err, &nft_parse_string_ops);
+}
 EXPORT_SYMBOL(nft_set_parse);
 
 static int nft_set_snprintf_json(char *buf, size_t size, struct nft_set *s,
diff --git a/src/set_elem.c b/src/set_elem.c
index 14bf6f4..1861902 100644
--- a/src/set_elem.c
+++ b/src/set_elem.c
@@ -395,13 +395,14 @@ int nft_mxml_set_elem_parse(mxml_node_t *tree, struct nft_set_elem *e,
 #endif
 
 static int nft_set_elem_xml_parse(struct nft_set_elem *e, const char *xml,
-				  struct nft_parse_err *err)
+				  struct nft_parse_err *err,
+				  const struct nft_parse_ops *ops)
 {
 #ifdef XML_PARSING
 	mxml_node_t *tree;
 	int ret;
 
-	tree = nft_mxml_build_tree(xml, "set_elem", err);
+	tree = ops->xmlbuilder(xml, "set_elem", err);
 	if (tree == NULL)
 		return -1;
 
@@ -415,13 +416,14 @@ static int nft_set_elem_xml_parse(struct nft_set_elem *e, const char *xml,
 }
 
 static int nft_set_elem_json_parse(struct nft_set_elem *e, const void *json,
-				   struct nft_parse_err *err)
+				   struct nft_parse_err *err,
+				   const struct nft_parse_ops *ops)
 {
 #ifdef JSON_PARSING
 	json_t *tree;
 	json_error_t error;
 
-	tree = nft_jansson_create_root(json, &error, err);
+	tree = ops->jsonbuilder(json, &error, err);
 	if (tree == NULL)
 		return -1;
 
@@ -432,17 +434,19 @@ static int nft_set_elem_json_parse(struct nft_set_elem *e, const void *json,
 #endif
 }
 
-int nft_set_elem_parse(struct nft_set_elem *e,
-		       enum nft_parse_type type, const char *data,
-		       struct nft_parse_err *err) {
+static int
+nft_set_elem_do_parse(struct nft_set_elem *e, enum nft_parse_type type,
+		      const void *data, struct nft_parse_err *err,
+		      const struct nft_parse_ops *ops)
+{
 	int ret;
 
 	switch (type) {
 	case NFT_PARSE_XML:
-		ret = nft_set_elem_xml_parse(e, data, err);
+		ret = nft_set_elem_xml_parse(e, data, err, ops);
 		break;
 	case NFT_PARSE_JSON:
-		ret = nft_set_elem_json_parse(e, data, err);
+		ret = nft_set_elem_json_parse(e, data, err, ops);
 		break;
 	default:
 		errno = EOPNOTSUPP;
@@ -452,6 +456,14 @@ int nft_set_elem_parse(struct nft_set_elem *e,
 
 	return ret;
 }
+
+int nft_set_elem_parse(struct nft_set_elem *e,
+		       enum nft_parse_type type, const char *data,
+		       struct nft_parse_err *err)
+{
+	return nft_set_elem_do_parse(e, type, data, err,
+				     &nft_parse_string_ops);
+}
 EXPORT_SYMBOL(nft_set_elem_parse);
 
 static int nft_set_elem_snprintf_json(char *buf, size_t size,
diff --git a/src/table.c b/src/table.c
index 9b5f5c9..7fb79dc 100644
--- a/src/table.c
+++ b/src/table.c
@@ -248,11 +248,12 @@ int nft_mxml_table_parse(mxml_node_t *tree, struct nft_table *t,
 #endif
 
 static int nft_table_xml_parse(struct nft_table *t, const char *xml,
-			       struct nft_parse_err *err)
+			       struct nft_parse_err *err,
+			       const struct nft_parse_ops *ops)
 {
 #ifdef XML_PARSING
 	int ret;
-	mxml_node_t *tree = nft_mxml_build_tree(xml, "table", err);
+	mxml_node_t *tree = ops->xmlbuilder(xml, "table", err);
 	if (tree == NULL)
 		return -1;
 
@@ -303,13 +304,14 @@ err:
 #endif
 
 static int nft_table_json_parse(struct nft_table *t, const char *json,
-				struct nft_parse_err *err)
+				struct nft_parse_err *err,
+				const struct nft_parse_ops *ops)
 {
 #ifdef JSON_PARSING
 	json_t *tree;
 	json_error_t error;
 
-	tree = nft_jansson_create_root(json, &error, err);
+	tree = ops->jsonbuilder(json, &error, err);
 	if (tree == NULL)
 		return -1;
 
@@ -320,18 +322,19 @@ static int nft_table_json_parse(struct nft_table *t, const char *json,
 #endif
 }
 
-int nft_table_parse(struct nft_table *t, enum nft_parse_type type,
-		    const char *data, struct nft_parse_err *err)
+static int nft_table_do_parse(struct nft_table *t, enum nft_parse_type type,
+			      const void *data, struct nft_parse_err *err,
+			      const struct nft_parse_ops *ops)
 {
 	int ret;
 	struct nft_parse_err perr;
 
 	switch (type) {
 	case NFT_PARSE_XML:
-		ret = nft_table_xml_parse(t, data, &perr);
+		ret = nft_table_xml_parse(t, data, &perr, ops);
 		break;
 	case NFT_PARSE_JSON:
-		ret = nft_table_json_parse(t, data, &perr);
+		ret = nft_table_json_parse(t, data, &perr, ops);
 		break;
 	default:
 		ret = -1;
@@ -344,6 +347,12 @@ int nft_table_parse(struct nft_table *t, enum nft_parse_type type,
 
 	return ret;
 }
+
+int nft_table_parse(struct nft_table *t, enum nft_parse_type type,
+		    const char *data, struct nft_parse_err *err)
+{
+	return nft_table_do_parse(t, type, data, err, &nft_parse_string_ops);
+}
 EXPORT_SYMBOL(nft_table_parse);
 
 static int nft_table_snprintf_json(char *buf, size_t size, struct nft_table *t)

[libnftables PATCH 5/6] parsing: add interface to parse from file

[Arturo Borrero Gonzalez]

This patch adds API interfaces to parse nft objects from a given stream.

I found this useful in `nft', where I'm trying to parse XML/JSON from a file.

Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
---
 0 files changed

diff --git a/include/libnftables/chain.h b/include/libnftables/chain.h
index dec1a77..d213bf1 100644
--- a/include/libnftables/chain.h
+++ b/include/libnftables/chain.h
@@ -53,6 +53,8 @@ void nft_chain_nlmsg_build_payload(struct nlmsghdr *nlh, const struct nft_chain
 
 int nft_chain_parse(struct nft_chain *c, enum nft_parse_type type,
 		    const char *data, struct nft_parse_err *err);
+int nft_chain_parse_file(struct nft_chain *c, enum nft_parse_type type,
+			 FILE *fp, struct nft_parse_err *err);
 int nft_chain_snprintf(char *buf, size_t size, struct nft_chain *t, uint32_t type, uint32_t flags);
 int nft_chain_fprintf(FILE *fp, struct nft_chain *c, uint32_t type, uint32_t flags);
 
diff --git a/include/libnftables/rule.h b/include/libnftables/rule.h
index 1510203..48b9974 100644
--- a/include/libnftables/rule.h
+++ b/include/libnftables/rule.h
@@ -49,6 +49,8 @@ void nft_rule_nlmsg_build_payload(struct nlmsghdr *nlh, struct nft_rule *t);
 
 int nft_rule_parse(struct nft_rule *r, enum nft_parse_type type,
 		   const char *data, struct nft_parse_err *err);
+int nft_rule_parse_file(struct nft_rule *r, enum nft_parse_type type,
+			FILE *fp, struct nft_parse_err *err);
 int nft_rule_snprintf(char *buf, size_t size, struct nft_rule *t, uint32_t type, uint32_t flags);
 int nft_rule_fprintf(FILE *fp, struct nft_rule *r, uint32_t type, uint32_t flags);
 
diff --git a/include/libnftables/ruleset.h b/include/libnftables/ruleset.h
index b523346..f916fba 100644
--- a/include/libnftables/ruleset.h
+++ b/include/libnftables/ruleset.h
@@ -32,6 +32,8 @@ const void *nft_ruleset_attr_get(const struct nft_ruleset *r, uint16_t attr);
 
 int nft_ruleset_parse(struct nft_ruleset *rs, enum nft_parse_type type,
 		      const char *data, struct nft_parse_err *err);
+int nft_ruleset_parse_file(struct nft_ruleset *rs, enum nft_parse_type type,
+			   FILE *fp, struct nft_parse_err *err);
 int nft_ruleset_snprintf(char *buf, size_t size, const struct nft_ruleset *rs, uint32_t type, uint32_t flags);
 int nft_ruleset_fprintf(FILE *fp, const struct nft_ruleset *rs, uint32_t type, uint32_t flags);
 
diff --git a/include/libnftables/set.h b/include/libnftables/set.h
index 9711729..c4b1ff6 100644
--- a/include/libnftables/set.h
+++ b/include/libnftables/set.h
@@ -62,6 +62,8 @@ void nft_set_list_iter_destroy(struct nft_set_list_iter *iter);
 
 int nft_set_parse(struct nft_set *s, enum nft_parse_type type,
 		  const char *data, struct nft_parse_err *err);
+int nft_set_parse_file(struct nft_set *s, enum nft_parse_type type,
+		       FILE *fp, struct nft_parse_err *err);
 
 /*
  * Set elements
@@ -101,6 +103,8 @@ int nft_set_elem_nlmsg_parse(const struct nlmsghdr *nlh, struct nft_set_elem *s)
 
 int nft_set_elem_parse(struct nft_set_elem *e, enum nft_parse_type type,
 		       const char *data, struct nft_parse_err *err);
+int nft_set_elem_parse_file(struct nft_set_elem *e, enum nft_parse_type type,
+			    FILE *fp, struct nft_parse_err *err);
 int nft_set_elem_snprintf(char *buf, size_t size, struct nft_set_elem *s, uint32_t type, uint32_t flags);
 int nft_set_elem_fprintf(FILE *fp, struct nft_set_elem *se, uint32_t type, uint32_t flags);
 
diff --git a/include/libnftables/table.h b/include/libnftables/table.h
index 80f2349..64fbf88 100644
--- a/include/libnftables/table.h
+++ b/include/libnftables/table.h
@@ -41,6 +41,8 @@ void nft_table_nlmsg_build_payload(struct nlmsghdr *nlh, const struct nft_table
 
 int nft_table_parse(struct nft_table *t, enum nft_parse_type type,
 		    const char *data, struct nft_parse_err *err);
+int nft_table_parse_file(struct nft_table *t, enum nft_parse_type type,
+			 FILE *fp, struct nft_parse_err *err);
 int nft_table_snprintf(char *buf, size_t size, struct nft_table *t, uint32_t type, uint32_t flags);
 int nft_table_fprintf(FILE *fp, struct nft_table *t, uint32_t type, uint32_t flags);
 
diff --git a/src/chain.c b/src/chain.c
index fd56629..6d311b5 100644
--- a/src/chain.c
+++ b/src/chain.c
@@ -761,6 +761,13 @@ int nft_chain_parse(struct nft_chain *c, enum nft_parse_type type,
 }
 EXPORT_SYMBOL(nft_chain_parse);
 
+int nft_chain_parse_file(struct nft_chain *c, enum nft_parse_type type,
+			 FILE *fp, struct nft_parse_err *err)
+{
+	return nft_chain_do_parse(c, type, fp, err, &nft_parse_file_ops);
+}
+EXPORT_SYMBOL(nft_chain_parse_file);
+
 static int nft_chain_snprintf_json(char *buf, size_t size, struct nft_chain *c)
 {
 	int ret, len = size, offset = 0;
diff --git a/src/internal.h b/src/internal.h
index afd8816..1a98685 100644
--- a/src/internal.h
+++ b/src/internal.h
@@ -48,6 +48,8 @@ struct nft_parse_err {
 #define NFT_XML_OPT (1 << 0)
 mxml_node_t *nft_mxml_build_tree(const void *data, const char *treename,
 				 struct nft_parse_err *err);
+mxml_node_t *nft_mxml_build_tree_file(const void *data, const char *treename,
+				      struct nft_parse_err *err);
 struct nft_rule_expr *nft_mxml_expr_parse(mxml_node_t *node,
 					  struct nft_parse_err *err);
 int nft_mxml_reg_parse(mxml_node_t *tree, const char *reg_name, uint32_t flags,
@@ -97,6 +99,8 @@ const char *nft_jansson_parse_str(json_t *root, const char *node_name,
 bool nft_jansson_node_exist(json_t *root, const char *node_name);
 json_t *nft_jansson_create_root(const void *data, json_error_t *error,
 				struct nft_parse_err *err);
+json_t *nft_jansson_create_root_file(const void *data, json_error_t *jerror,
+				     struct nft_parse_err *err);
 json_t *nft_jansson_get_node(json_t *root, const char *node_name,
 			     struct nft_parse_err *err);
 void nft_jansson_free_root(json_t *root);
@@ -145,6 +149,15 @@ static const struct nft_parse_ops nft_parse_string_ops = {
 #endif
 };
 
+static const struct nft_parse_ops nft_parse_file_ops = {
+#ifdef XML_PARSING
+	.xmlbuilder = nft_mxml_build_tree_file,
+#endif
+#ifdef JSON_PARSING
+	.jsonbuilder = nft_jansson_create_root_file
+#endif
+};
+
 const char *nft_family2str(uint32_t family);
 int nft_str2family(const char *family);
 int nft_strtoi(const char *string, int base, void *number, enum nft_type type);
diff --git a/src/jansson.c b/src/jansson.c
index 90faeca..20aa822 100644
--- a/src/jansson.c
+++ b/src/jansson.c
@@ -114,6 +114,30 @@ json_t *nft_jansson_create_root(const void *data, json_error_t *error,
 	return nft_jansson_do_create_root(data, error, err);
 }
 
+static json_t *nft_jansson_do_create_root_file(FILE *f, json_error_t *jerror,
+					       struct nft_parse_err *err)
+{
+	json_t *root;
+
+	root = json_loadf(f, 0, jerror);
+	if (root == NULL) {
+		err->error = NFT_PARSE_EBADINPUT;
+		err->line = jerror->line;
+		err->column = jerror->column;
+		err->node_name = jerror->source;
+		errno = EINVAL;
+		return NULL;
+	}
+
+	return root;
+}
+
+json_t *nft_jansson_create_root_file(const void *data, json_error_t *jerror,
+				     struct nft_parse_err *err)
+{
+	return nft_jansson_do_create_root_file((FILE *)data, jerror, err);
+}
+
 json_t *nft_jansson_get_node(json_t *root, const char *node_name,
 			     struct nft_parse_err *err)
 {
diff --git a/src/libnftables.map b/src/libnftables.map
index be5c783..faf0913 100644
--- a/src/libnftables.map
+++ b/src/libnftables.map
@@ -13,6 +13,7 @@ global:
   nft_table_attr_get_u32;
   nft_table_attr_get_str;
   nft_table_parse;
+  nft_table_parse_file;
   nft_table_snprintf;
   nft_table_fprintf;
   nft_table_nlmsg_build_payload;
@@ -45,6 +46,7 @@ global:
   nft_chain_attr_get_u64;
   nft_chain_attr_get_str;
   nft_chain_parse;
+  nft_chain_parse_file;
   nft_chain_snprintf;
   nft_chain_fprintf;
   nft_chain_nlmsg_build_payload;
@@ -74,6 +76,7 @@ global:
   nft_rule_attr_get_u64;
   nft_rule_attr_get_str;
   nft_rule_parse;
+  nft_rule_parse_file;
   nft_rule_snprintf;
   nft_rule_fprintf;
   nft_rule_nlmsg_build_payload;
@@ -128,6 +131,7 @@ global:
   nft_set_nlmsg_build_payload;
   nft_set_nlmsg_parse;
   nft_set_parse;
+  nft_set_parse_file;
   nft_set_snprintf;
   nft_set_fprintf;
 
@@ -159,6 +163,7 @@ global:
   nft_set_elem_nlmsg_build_payload;
   nft_set_elem_nlmsg_parse;
   nft_set_elem_parse;
+  nft_set_elem_parse_file;
   nft_set_elem_snprintf;
   nft_set_elem_fprinf;
 
@@ -179,6 +184,7 @@ global:
   nft_ruleset_attr_set;
   nft_ruleset_attr_get;
   nft_ruleset_parse;
+  nft_ruleset_parse_file;
   nft_ruleset_snprintf;
   nft_ruleset_fprintf;
 
diff --git a/src/mxml.c b/src/mxml.c
index 13a8b89..9851365 100644
--- a/src/mxml.c
+++ b/src/mxml.c
@@ -54,6 +54,37 @@ mxml_node_t *nft_mxml_build_tree(const void *data, const char *treename,
 	return nft_mxml_do_build_tree(data, treename, err);
 }
 
+static mxml_node_t *nft_mxml_do_build_tree_file(FILE *f, const char *treename,
+						struct nft_parse_err *err)
+{
+	mxml_node_t *tree;
+
+	tree = mxmlLoadFile(NULL, f, MXML_OPAQUE_CALLBACK);
+	if (tree == NULL) {
+		err->error = NFT_PARSE_EBADINPUT;
+		goto err;
+	}
+
+	if (strcmp(tree->value.opaque, treename) == 0)
+		return tree;
+
+	err->error = NFT_PARSE_EMISSINGNODE;
+	err->node_name = treename;
+
+	mxmlDelete(tree);
+err:
+	err->line = 0;
+	err->column = 0;
+	errno = EINVAL;
+	return NULL;
+}
+
+mxml_node_t *nft_mxml_build_tree_file(const void *data, const char *treename,
+				      struct nft_parse_err *err)
+{
+	return nft_mxml_do_build_tree_file((FILE *)data, treename, err);
+}
+
 struct nft_rule_expr *nft_mxml_expr_parse(mxml_node_t *node,
 					  struct nft_parse_err *err)
 {
diff --git a/src/rule.c b/src/rule.c
index 6f5718d..268acbf 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -679,6 +679,13 @@ int nft_rule_parse(struct nft_rule *r, enum nft_parse_type type,
 }
 EXPORT_SYMBOL(nft_rule_parse);
 
+int nft_rule_parse_file(struct nft_rule *r, enum nft_parse_type type,
+			FILE *fp, struct nft_parse_err *err)
+{
+	return nft_rule_do_parse(r, type, fp, err, &nft_parse_file_ops);
+}
+EXPORT_SYMBOL(nft_rule_parse_file);
+
 static int nft_rule_snprintf_json(char *buf, size_t size, struct nft_rule *r,
 					 uint32_t type, uint32_t flags)
 {
diff --git a/src/ruleset.c b/src/ruleset.c
index 84ada20..2676a71 100644
--- a/src/ruleset.c
+++ b/src/ruleset.c
@@ -600,6 +600,13 @@ int nft_ruleset_parse(struct nft_ruleset *rs, enum nft_parse_type type,
 }
 EXPORT_SYMBOL(nft_ruleset_parse);
 
+int nft_ruleset_parse_file(struct nft_ruleset *rs, enum nft_parse_type type,
+			   FILE *fp, struct nft_parse_err *err)
+{
+	return nft_ruleset_do_parse(rs, type, fp, err, &nft_parse_file_ops);
+}
+EXPORT_SYMBOL(nft_ruleset_parse_file);
+
 static const char *nft_ruleset_o_opentag(uint32_t type)
 {
 	switch (type) {
diff --git a/src/set.c b/src/set.c
index 248619c..e374e12 100644
--- a/src/set.c
+++ b/src/set.c
@@ -536,6 +536,13 @@ int nft_set_parse(struct nft_set *s, enum nft_parse_type type,
 }
 EXPORT_SYMBOL(nft_set_parse);
 
+int nft_set_parse_file(struct nft_set *s, enum nft_parse_type type,
+		       FILE *fp, struct nft_parse_err *err)
+{
+	return nft_set_do_parse(s, type, fp, err, &nft_parse_file_ops);
+}
+EXPORT_SYMBOL(nft_set_parse_file);
+
 static int nft_set_snprintf_json(char *buf, size_t size, struct nft_set *s,
 				  uint32_t type, uint32_t flags)
 {
diff --git a/src/set_elem.c b/src/set_elem.c
index 1861902..95eab6d 100644
--- a/src/set_elem.c
+++ b/src/set_elem.c
@@ -466,6 +466,13 @@ int nft_set_elem_parse(struct nft_set_elem *e,
 }
 EXPORT_SYMBOL(nft_set_elem_parse);
 
+int nft_set_elem_parse_file(struct nft_set_elem *e, enum nft_parse_type type,
+			    FILE *fp, struct nft_parse_err *err)
+{
+	return nft_set_elem_do_parse(e, type, fp, err, &nft_parse_file_ops);
+}
+EXPORT_SYMBOL(nft_set_elem_parse_file);
+
 static int nft_set_elem_snprintf_json(char *buf, size_t size,
 				      struct nft_set_elem *e, uint32_t flags)
 {
diff --git a/src/table.c b/src/table.c
index 7fb79dc..9de93e2 100644
--- a/src/table.c
+++ b/src/table.c
@@ -355,6 +355,13 @@ int nft_table_parse(struct nft_table *t, enum nft_parse_type type,
 }
 EXPORT_SYMBOL(nft_table_parse);
 
+int nft_table_parse_file(struct nft_table *t, enum nft_parse_type type,
+			 FILE *fp, struct nft_parse_err *err)
+{
+	return nft_table_do_parse(t, type, fp, err, &nft_parse_file_ops);
+}
+EXPORT_SYMBOL(nft_table_parse_file);
+
 static int nft_table_snprintf_json(char *buf, size_t size, struct nft_table *t)
 {
 	return snprintf(buf, size,

[libnftables PATCH 6/6] tests: update tests with nft_*_parse_file()

[Arturo Borrero Gonzalez]

Lets do testing with nft_*_parse_file() functions.

Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
---
 tests/nft-parsing-test.c |  102 +++++++++++++++++++++++++++-------------------
 1 file changed, 59 insertions(+), 43 deletions(-)

diff --git a/tests/nft-parsing-test.c b/tests/nft-parsing-test.c
index 558c849..4702fd3 100644
--- a/tests/nft-parsing-test.c
+++ b/tests/nft-parsing-test.c
@@ -73,7 +73,8 @@ static void print_detail_error(char *a, char *b)
 	}
 }
 
-static int compare_test(uint32_t type, void *input, const char *filename)
+static int compare_test(uint32_t type, void *input, const char *filename,
+			FILE *fp)
 {
 	struct nft_table *t = NULL;
 	struct nft_chain *c = NULL;
@@ -82,7 +83,6 @@ static int compare_test(uint32_t type, void *input, const char *filename)
 	struct nft_ruleset *rs = NULL;
 	char orig[4096];
 	char out[4096];
-	FILE *fp;
 
 	switch (type) {
 	case TEST_XML_TABLE:
@@ -148,14 +148,8 @@ static int compare_test(uint32_t type, void *input, const char *filename)
 		return -1;
 	}
 
-	fp = fopen(filename, "r");
-	if (fp == NULL) {
-		perror("open");
-		exit(EXIT_FAILURE);
-	}
-
+	rewind(fp);
 	fgets(orig, sizeof(orig), fp);
-	fclose(fp);
 
 	if (strncmp(orig, out, strlen(out)) == 0)
 		return 0;
@@ -178,19 +172,26 @@ static int test_json(const char *filename, struct nft_parse_err *err)
 	struct nft_ruleset *rs;
 	json_t *root;
 	json_error_t error;
-	char *json;
+	FILE *fp;
+
+	fp = fopen(filename, "r");
+	if (fp == NULL) {
+		printf("unable to open file %s: %s\n", filename,
+		       strerror(errno));
+		return -1;
+	}
 
 	root = json_load_file(filename, 0, &error);
 	if (!root)
 		return -1;
 
-	json = json_dumps(root, JSON_INDENT(0));
-
 	if (json_object_get(root, "table") != NULL) {
 		t = nft_table_alloc();
 		if (t != NULL) {
-			if (nft_table_parse(t, NFT_PARSE_JSON, json, err) == 0)
-				ret = compare_test(TEST_JSON_TABLE, t, filename);
+			if (nft_table_parse_file(t, NFT_PARSE_JSON,
+						 fp, err) == 0)
+				ret = compare_test(TEST_JSON_TABLE, t,
+						   filename, fp);
 			else
 				goto failparsing;
 
@@ -199,8 +200,10 @@ static int test_json(const char *filename, struct nft_parse_err *err)
 	} else if (json_object_get(root, "chain") != NULL) {
 		c = nft_chain_alloc();
 		if (c != NULL) {
-			if (nft_chain_parse(c, NFT_PARSE_JSON, json, err) == 0)
-				ret = compare_test(TEST_JSON_CHAIN, c, filename);
+			if (nft_chain_parse_file(c, NFT_PARSE_JSON,
+						 fp, err) == 0)
+				ret = compare_test(TEST_JSON_CHAIN, c,
+						   filename, fp);
 			else
 				goto failparsing;
 
@@ -209,8 +212,10 @@ static int test_json(const char *filename, struct nft_parse_err *err)
 	} else if (json_object_get(root, "rule") != NULL) {
 		r = nft_rule_alloc();
 		if (r != NULL) {
-			if (nft_rule_parse(r, NFT_PARSE_JSON, json, err) == 0)
-				ret = compare_test(TEST_JSON_RULE, r, filename);
+			if (nft_rule_parse_file(r, NFT_PARSE_JSON,
+						fp, err) == 0)
+				ret = compare_test(TEST_JSON_RULE, r,
+						   filename, fp);
 			else
 				goto failparsing;
 
@@ -219,8 +224,10 @@ static int test_json(const char *filename, struct nft_parse_err *err)
 	} else if (json_object_get(root, "set") != NULL) {
 		s = nft_set_alloc();
 		if (s != NULL) {
-			if (nft_set_parse(s, NFT_PARSE_JSON, json, err) == 0)
-				ret = compare_test(TEST_JSON_SET, s, filename);
+			if (nft_set_parse_file(s, NFT_PARSE_JSON,
+					       fp, err) == 0)
+				ret = compare_test(TEST_JSON_SET, s,
+						   filename, fp);
 			else
 				goto failparsing;
 
@@ -229,8 +236,10 @@ static int test_json(const char *filename, struct nft_parse_err *err)
 	} else if (json_object_get(root, "nftables") != NULL) {
 		rs = nft_ruleset_alloc();
 		if (rs != NULL) {
-			if (nft_ruleset_parse(rs, NFT_PARSE_JSON, json, err) == 0)
-				ret = compare_test(TEST_JSON_RULESET, rs, filename);
+			if (nft_ruleset_parse_file(rs, NFT_PARSE_JSON,
+						   fp, err) == 0)
+				ret = compare_test(TEST_JSON_RULESET, rs,
+						   filename, fp);
 			else
 				goto failparsing;
 
@@ -238,14 +247,14 @@ static int test_json(const char *filename, struct nft_parse_err *err)
 			}
 	}
 
-	free(json);
+	fclose(fp);
 	json_decref(root);
 	return ret;
 
 failparsing:
+	fclose(fp);
 	printf("parsing %s: ", filename);
 	printf("\033[31mFAILED\e[0m (%s)\n", strerror(errno));
-	free(json);
 	json_decref(root);
 	return -1;
 #else
@@ -265,11 +274,15 @@ static int test_xml(const char *filename, struct nft_parse_err *err)
 	struct nft_ruleset *rs;
 	FILE *fp;
 	mxml_node_t *tree;
-	char *xml;
 
 	fp = fopen(filename, "r");
+	if (fp == NULL) {
+		printf("unable to open file %s: %s\n", filename,
+		       strerror(errno));
+		return -1;
+	}
+
 	tree = mxmlLoadFile(NULL, fp, MXML_NO_CALLBACK);
-	fclose(fp);
 
 	if (tree == NULL) {
 		printf("unable to build XML tree from file "
@@ -277,19 +290,16 @@ static int test_xml(const char *filename, struct nft_parse_err *err)
 		return -1;
 	}
 
-	xml = mxmlSaveAllocString(tree, MXML_NO_CALLBACK);
-	if (xml == NULL) {
-		printf("unable to alloc string from XML tree from %s "
-		       "\033[31mFAILED\e[0m\n", filename);
-		return -1;
-	}
+	rewind(fp);
 
 	/* Check what parsing should be done */
 	if (strcmp(tree->value.opaque, "table") == 0) {
 		t = nft_table_alloc();
 		if (t != NULL) {
-			if (nft_table_parse(t, NFT_PARSE_XML, xml, err) == 0)
-				ret = compare_test(TEST_XML_TABLE, t, filename);
+			if (nft_table_parse_file(t, NFT_PARSE_XML,
+						 fp, err) == 0)
+				ret = compare_test(TEST_XML_TABLE, t,
+						   filename, fp);
 			else
 				goto failparsing;
 
@@ -298,8 +308,10 @@ static int test_xml(const char *filename, struct nft_parse_err *err)
 	} else if (strcmp(tree->value.opaque, "chain") == 0) {
 		c = nft_chain_alloc();
 		if (c != NULL) {
-			if (nft_chain_parse(c, NFT_PARSE_XML, xml, err) == 0)
-				ret = compare_test(TEST_XML_CHAIN, c, filename);
+			if (nft_chain_parse_file(c, NFT_PARSE_XML,
+						 fp, err) == 0)
+				ret = compare_test(TEST_XML_CHAIN, c,
+						   filename, fp);
 			else
 				goto failparsing;
 
@@ -308,8 +320,10 @@ static int test_xml(const char *filename, struct nft_parse_err *err)
 	} else if (strcmp(tree->value.opaque, "rule") == 0) {
 		r = nft_rule_alloc();
 		if (r != NULL) {
-			if (nft_rule_parse(r, NFT_PARSE_XML, xml, err) == 0)
-				ret = compare_test(TEST_XML_RULE, r, filename);
+			if (nft_rule_parse_file(r, NFT_PARSE_XML,
+						fp, err) == 0)
+				ret = compare_test(TEST_XML_RULE, r,
+						   filename, fp);
 			else
 				goto failparsing;
 
@@ -318,8 +332,10 @@ static int test_xml(const char *filename, struct nft_parse_err *err)
 	} else if (strcmp(tree->value.opaque, "set") == 0) {
 		s = nft_set_alloc();
 		if (s != NULL) {
-			if (nft_set_parse(s, NFT_PARSE_XML, xml, err) == 0)
-				ret = compare_test(TEST_XML_SET, s, filename);
+			if (nft_set_parse_file(s, NFT_PARSE_XML,
+					       fp, err) == 0)
+				ret = compare_test(TEST_XML_SET, s,
+						   filename, fp);
 			else
 				goto failparsing;
 
@@ -328,10 +344,10 @@ static int test_xml(const char *filename, struct nft_parse_err *err)
 	} else if (strcmp(tree->value.opaque, "nftables") == 0) {
 		rs = nft_ruleset_alloc();
 		if (rs != NULL) {
-			if (nft_ruleset_parse(rs, NFT_PARSE_XML,
-					      xml, err) == 0)
+			if (nft_ruleset_parse_file(rs, NFT_PARSE_XML,
+						   fp, err) == 0)
 				ret = compare_test(TEST_XML_RULESET, rs,
-						   filename);
+						   filename, fp);
 			else
 				goto failparsing;
 

Re: [libnftables PATCH 1/6] mxml: add error reference of the top node

[Pablo Neira Ayuso]

On Tue, Jan 07, 2014 at 12:47:16PM +0100, Arturo Borrero Gonzalez wrote:
> We know the top node we are building. Let the user also know it.

Applied, thanks.

Re: [libnftables PATCH 2/6] set_elem: add json parsing to API

[Pablo Neira Ayuso]

On Tue, Jan 07, 2014 at 12:47:21PM +0100, Arturo Borrero Gonzalez wrote:
> Add missing support in the API function to parse a JSON set_elem.
> 
> I've renamed the main JSON parsing function to prevent clashing.

Applied, thanks.

Re: [libnftables PATCH 3/6] internal: rework parsing symbol logic

[Pablo Neira Ayuso]

On Tue, Jan 07, 2014 at 12:47:27PM +0100, Arturo Borrero Gonzalez wrote:
> Rework parsing symbols logic, so we get a cleaner file.
> 
> Also, this allows us to use XML(libmxml)/JSON(libjannson) internal datatypes
> without fear.
> 
> Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
> ---
>  0 files changed
> 
> diff --git a/src/internal.h b/src/internal.h
> index 5fef6d6..83cb4e8 100644
> --- a/src/internal.h
> +++ b/src/internal.h
> @@ -40,6 +40,10 @@ struct nft_parse_err {
>  
>  #ifdef XML_PARSING
>  #include <mxml.h>
> +#else
> +#define mxml_node_t void
> +#endif /* XML_PARSING */

I failed to see the benefit of this patch.

> +
>  #define NFT_XML_MAND 0
>  #define NFT_XML_OPT (1 << 0)
>  mxml_node_t *nft_mxml_build_tree(const char *xml, const char *treename,
> @@ -78,10 +82,13 @@ int nft_mxml_rule_parse(mxml_node_t *tree, struct nft_rule *r,
>  struct nft_set;
>  int nft_mxml_set_parse(mxml_node_t *tree, struct nft_set *s,
>  		       struct nft_parse_err *err);
> -#endif

We don't save any ifdef.

Re: [libnftables PATCH 4/6] internal: add a selector for parsing ops

[Pablo Neira Ayuso]

On Tue, Jan 07, 2014 at 12:47:32PM +0100, Arturo Borrero Gonzalez wrote:
> This patch adds a selector for parsing operations.
> 
> The selector allows to reduce code duplication in case more than
> one parsing interface are used.

This needs a rework.

> Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
> ---
>  0 files changed
> 
> diff --git a/src/chain.c b/src/chain.c
> index a4ddb06..fd56629 100644
> --- a/src/chain.c
> +++ b/src/chain.c
> @@ -592,13 +592,14 @@ err:
>  #endif
>  
>  static int nft_chain_json_parse(struct nft_chain *c, const char *json,
> -				struct nft_parse_err *err)
> +				struct nft_parse_err *err,
> +				const struct nft_parse_ops *ops)

You should pass a callback function instead, eg.

static int nft_chain_json_parse(struct nft_chain *c, const char *json,
				struct nft_parse_err *err,
                                json_t *(*jsonbuilder)(const void *input_data,
                                                       const char *treename,
                                                       struct nft_parse_err *e))
But I don't understand yet what you save (in terms of lines of code)
by using this aproach.

Re: [libnftables PATCH 6/6] tests: update tests with nft_*_parse_file()

[Pablo Neira Ayuso]

On Tue, Jan 07, 2014 at 12:47:42PM +0100, Arturo Borrero Gonzalez wrote:
> Lets do testing with nft_*_parse_file() functions.
> 
> Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
> ---
>  tests/nft-parsing-test.c |  102 +++++++++++++++++++++++++++-------------------
>  1 file changed, 59 insertions(+), 43 deletions(-)
> 
> diff --git a/tests/nft-parsing-test.c b/tests/nft-parsing-test.c
> index 558c849..4702fd3 100644
> --- a/tests/nft-parsing-test.c
> +++ b/tests/nft-parsing-test.c
> @@ -73,7 +73,8 @@ static void print_detail_error(char *a, char *b)
>  	}
>  }
>  
> -static int compare_test(uint32_t type, void *input, const char *filename)
> +static int compare_test(uint32_t type, void *input, const char *filename,
> +			FILE *fp)
>  {
>  	struct nft_table *t = NULL;
>  	struct nft_chain *c = NULL;
> @@ -82,7 +83,6 @@ static int compare_test(uint32_t type, void *input, const char *filename)
>  	struct nft_ruleset *rs = NULL;
>  	char orig[4096];
>  	char out[4096];
> -	FILE *fp;
>  
>  	switch (type) {
>  	case TEST_XML_TABLE:
> @@ -148,14 +148,8 @@ static int compare_test(uint32_t type, void *input, const char *filename)
>  		return -1;
>  	}
>  
> -	fp = fopen(filename, "r");
> -	if (fp == NULL) {
> -		perror("open");
> -		exit(EXIT_FAILURE);
> -	}
> -
> +	rewind(fp);
>  	fgets(orig, sizeof(orig), fp);
> -	fclose(fp);
>  
>  	if (strncmp(orig, out, strlen(out)) == 0)
>  		return 0;
> @@ -178,19 +172,26 @@ static int test_json(const char *filename, struct nft_parse_err *err)
>  	struct nft_ruleset *rs;
>  	json_t *root;
>  	json_error_t error;
> -	char *json;
> +	FILE *fp;
> +
> +	fp = fopen(filename, "r");
> +	if (fp == NULL) {
> +		printf("unable to open file %s: %s\n", filename,
> +		       strerror(errno));
> +		return -1;
> +	}
>  
>  	root = json_load_file(filename, 0, &error);
>  	if (!root)
>  		return -1;
>  
> -	json = json_dumps(root, JSON_INDENT(0));
> -
>  	if (json_object_get(root, "table") != NULL) {

This was fine in an early stage, but now that you have the ruleset
API, we should provide a nft_ruleset_parse_file(...) function that do
all this for you. As a side effect, the test will be simplified.

>  		t = nft_table_alloc();
>  		if (t != NULL) {
> -			if (nft_table_parse(t, NFT_PARSE_JSON, json, err) == 0)
> -				ret = compare_test(TEST_JSON_TABLE, t, filename);
> +			if (nft_table_parse_file(t, NFT_PARSE_JSON,
> +						 fp, err) == 0)
> +				ret = compare_test(TEST_JSON_TABLE, t,
> +						   filename, fp);
>  			else
>  				goto failparsing;
>  

Re: [libnftables PATCH 3/6] internal: rework parsing symbol logic

[Arturo Borrero Gonzalez]

On 8 January 2014 00:18, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>
> We don't save any ifdef.

Well, we need at least one ifdef per format. I don't see how we can
get rid of them.

This patch just condensates the logic to one single point, also adding
the void definition, which is good for other patches in the series.
-- 
Arturo Borrero González
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [libnftables PATCH 6/6] tests: update tests with nft_*_parse_file()

[Arturo Borrero Gonzalez]

On 8 January 2014 00:32, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>
> This was fine in an early stage, but now that you have the ruleset
> API, we should provide a nft_ruleset_parse_file(...) function that do
> all this for you. As a side effect, the test will be simplified.
>

good point. I will rework this.

-- 
Arturo Borrero González
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [libnftables PATCH 3/6] internal: rework parsing symbol logic

[Pablo Neira Ayuso]

On Wed, Jan 08, 2014 at 01:20:51PM +0100, Arturo Borrero Gonzalez wrote:
> On 8 January 2014 00:18, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> >
> > We don't save any ifdef.
> 
> Well, we need at least one ifdef per format. I don't see how we can
> get rid of them.
> 
> This patch just condensates the logic to one single point, also adding
> the void definition, which is good for other patches in the series.

OK, then you have to include in the description that it simplifies
follow-up patches and in what way.

But I also rised some concerns on the follow-up patches that you have
to address.

Re: [libnftables PATCH 4/6] internal: add a selector for parsing ops

[Arturo Borrero Gonzalez]

On 8 January 2014 00:29, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>
> You should pass a callback function instead, eg.
>
> static int nft_chain_json_parse(struct nft_chain *c, const char *json,
>                                 struct nft_parse_err *err,
>                                 json_t *(*jsonbuilder)(const void *input_data,
>                                                        const char *treename,
>                                                        struct nft_parse_err *e))

Ok.

> But I don't understand yet what you save (in terms of lines of code)
> by using this aproach.

I avoid doing something like:

nft_*_parse() {
switch (type)
 if XML return xml_parse()
 if JSON return json_parse()
}

nft_*_parse_file() {
switch (type)
 if XML return xml_parse_file()
 if JSON return json_parse_file()
}

We double the format switch, and also two functions per format are
needed to do build and parsing.
Total = 6 functions heavily duplicating code.

With my approach, we have 1 function that decides which format to
parse, and a one function per format to build and do parsing.
Total = 3 functions, no duplicate code.
-- 
Arturo Borrero González
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [libnftables PATCH 4/6] internal: add a selector for parsing ops

[Pablo Neira Ayuso]

On Wed, Jan 08, 2014 at 01:31:44PM +0100, Arturo Borrero Gonzalez wrote:
> On 8 January 2014 00:29, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> >
> > You should pass a callback function instead, eg.
> >
> > static int nft_chain_json_parse(struct nft_chain *c, const char *json,
> >                                 struct nft_parse_err *err,
> >                                 json_t *(*jsonbuilder)(const void *input_data,
> >                                                        const char *treename,
> >                                                        struct nft_parse_err *e))
> 
> Ok.
> 
> > But I don't understand yet what you save (in terms of lines of code)
> > by using this aproach.
> 
> I avoid doing something like:
> 
> nft_*_parse() {
> switch (type)
>  if XML return xml_parse()
>  if JSON return json_parse()
> }
> 
> nft_*_parse_file() {
> switch (type)
>  if XML return xml_parse_file()
>  if JSON return json_parse_file()
> }
> 
> We double the format switch, and also two functions per format are
> needed to do build and parsing.
> Total = 6 functions heavily duplicating code.
> 
> With my approach, we have 1 function that decides which format to
> parse, and a one function per format to build and do parsing.
> Total = 3 functions, no duplicate code.

I guess this is saving you code, but I think this abstraction needs to
be refined, your functions:

* nft_mxml_do_build_tree
* nft_mxml_do_build_tree_file

look almost the same, only difference is mxmlLoadString / mxmlLoadFile.

I really think you can save more code and make this look better if you
rework the internal nft_*_json_parse functions to receive the xml/json
trees, eg.

static int nft_set_json_parse(struct nft_set *s, json_t tree
                              struct nft_parse_err *err)
{
        ...
}

So you nft_set_do_parse() creates the tree and pass it to it.

You can also add an enum like:

enum {
        NFT_PARSE_BUFFER,
        NFT_PARSE_FILE,
};

json_t *nft_json_build_tree(uint32_t type, void *data)
{
        json_t *tree;

        switch (type) {
        case NFT_PARSE_BUFFER:
                tree = nft_json_build_tree(data, ...);
                break;
        case NFT_PARSE_FILE:
                tree = nft_json_build_tree_file(data, ...);
                break;
        }

        return tree;
}

that you can pass this enum to nft_set_do_parse() to indicate how the
tree need to be build, eg.

statiuc int nft_set_do_parse(..., uint32_t format)
{
        switch (format) {
        ...
        case NFT_PARSE_JSON:
                tree = nft_json_build_tree(type, data);
                break;
        ...
        }
        ...
}

Where data is the file descriptor / buffer area.