Skip to contentSkip to conversation

parsing update

PATCH Applied Arturo Borrero Gonzalez 2 participants 17 messages Jan 7, 2014 – Jan 8, 2014 netfilter

lore
Share & tools
lore

Arturo Borrero Gonzalez

author
#0 lore raw

cover parsing update

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(-)
Signature · 1 line
Arturo Borrero Gonzalez
author
#1 lore raw

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

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

Signed-off-by Arturo Borrero Gonzalez <[email protected]>

---

 0 files changed
src/mxml.c +5 -2
index bd09bb3..bc0f084 100644
@@ -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;
 }
maintainer Applied
#2 lore raw

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

Quoted 1 line · Arturo Borrero Gonzalez

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

Applied, thanks.

author
#3 lore raw

PATCH 2/6 set_elem: add json parsing to API

In reply to 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 <[email protected]>

---

 0 files changed
src/internal.h +2 -2
index 9ef505f..5fef6d6 100644
@@ -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);
src/jansson.c +2 -2
index 54a56b9..e62116b 100644
@@ -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;
src/set.c +2 -1
index 32c7fff..9317b9c 100644
@@ -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);
src/set_elem.c +21 -0
index 7365aff..14bf6f4 100644
@@ -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;
maintainer Applied
#4 lore raw

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

Quoted 3 lines · 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.

Applied, thanks.

author
#5 lore raw

PATCH 3/6 internal: rework parsing symbol logic

In reply to 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 <[email protected]>

---

 0 files changed
src/internal.h +8 -2
index 5fef6d6..83cb4e8 100644
@@ -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);
maintainer
#6 lore raw

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

Quoted 14 lines · 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 <[email protected]>

---

 0 files changed
src/internal.h +0 -0
index 5fef6d6..83cb4e8 100644

src/internal.h struct nft_parse_err { · line 45

Quoted 6 lines
src/internal.h +3 -0
@@ -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.

src/internal.h int nft_mxml_rule_parse(mxml_node_t *tree, struct nft_rule *r, · line 81

Quoted 9 lines
src/internal.h +1 -1
+
 #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.

author
#7 lore raw

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

Quoted 2 lines · Pablo Neira Ayuso

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.

Signature · 5 lines
Arturo Borrero González
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
maintainer
#8 lore raw

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

Quoted 9 lines · Arturo Borrero Gonzalez
Quoted 2 lines · Pablo Neira Ayuso

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.

author
#9 lore raw

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

In reply to 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 <[email protected]>

---

 0 files changed
src/chain.c +17 -8
index a4ddb06..fd56629 100644
@@ -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)
src/internal.h +20 -2
index 83cb4e8..afd8816 100644
@@ -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);
src/jansson.c +9 -2
index e62116b..90faeca 100644
@@ -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)
 {
src/mxml.c +9 -2
index bc0f084..13a8b89 100644
@@ -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)
 {
src/rule.c +17 -8
index 2e35aba..6f5718d 100644
@@ -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,
src/ruleset.c +19 -8
index a12efa9..84ada20 100644
@@ -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)
src/set.c +17 -8
index 9317b9c..248619c 100644
@@ -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,
src/set_elem.c +21 -9
index 14bf6f4..1861902 100644
@@ -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,
src/table.c +17 -8
index 9b5f5c9..7fb79dc 100644
@@ -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)
maintainer
#10 lore raw

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

Quoted 4 lines · 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.

This needs a rework.

Quoted 9 lines
Signed-off-by Arturo Borrero Gonzalez <[email protected]>

---

 0 files changed
src/chain.c +0 -0
index a4ddb06..fd56629 100644

src/chain.c err: · line 596

Quoted 6 lines
src/chain.c +2 -1
@@ -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.

author
#11 lore raw

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

Quoted 8 lines · Pablo Neira Ayuso

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.

Quoted 2 lines

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.

Signature · 5 lines
Arturo Borrero González
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
maintainer
#12 lore raw

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

Quoted 29 lines · Arturo Borrero Gonzalez
Quoted 8 lines · Pablo Neira Ayuso

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.

Quoted 2 lines

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()
}

Quoted 7 lines

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.

author
#13 lore raw

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

In reply to 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 <[email protected]>

---

 0 files changed
include/libnftables/chain.h +2 -0
index dec1a77..d213bf1 100644
@@ -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);
include/libnftables/rule.h +2 -0
index 1510203..48b9974 100644
@@ -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);
include/libnftables/ruleset.h +2 -0
index b523346..f916fba 100644
@@ -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);
include/libnftables/set.h +4 -0
index 9711729..c4b1ff6 100644
@@ -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);
include/libnftables/table.h +2 -0
index 80f2349..64fbf88 100644
@@ -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);
src/chain.c +7 -0
index fd56629..6d311b5 100644
@@ -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;
src/internal.h +13 -0
index afd8816..1a98685 100644
@@ -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);
src/jansson.c +24 -0
index 90faeca..20aa822 100644
@@ -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)
 {
src/libnftables.map +6 -0
index be5c783..faf0913 100644
@@ -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;
src/mxml.c +31 -0
index 13a8b89..9851365 100644
@@ -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)
 {
src/rule.c +7 -0
index 6f5718d..268acbf 100644
@@ -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)
 {
src/ruleset.c +7 -0
index 84ada20..2676a71 100644
@@ -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) {
src/set.c +7 -0
index 248619c..e374e12 100644
@@ -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)
 {
src/set_elem.c +7 -0
index 1861902..95eab6d 100644
@@ -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)
 {
src/table.c +7 -0
index 7fb79dc..9de93e2 100644
@@ -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,
author
#14 lore raw

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

In reply to Arturo Borrero Gonzalez

Lets do testing with nft_*_parse_file() functions.

Signed-off-by Arturo Borrero Gonzalez <[email protected]>
---
 tests/nft-parsing-test.c |  102 +++++++++++++++++++++++++++-------------------
 1 file changed, 59 insertions(+), 43 deletions(-)
tests/nft-parsing-test.c +59 -43
index 558c849..4702fd3 100644
@@ -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;
maintainer
#15 lore raw

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

Quoted 56 lines · Arturo Borrero Gonzalez

Lets do testing with nft_*_parse_file() functions.

Signed-off-by Arturo Borrero Gonzalez <[email protected]>
---
 tests/nft-parsing-test.c |  102 +++++++++++++++++++++++++++-------------------
 1 file changed, 59 insertions(+), 43 deletions(-)
tests/nft-parsing-test.c +9 -10
index 558c849..4702fd3 100644
@@ -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));

tests/nft-parsing-test.c static int test_json(const char *filename, struct nft_parse_err *err) · line 188

Quoted 10 lines
tests/nft-parsing-test.c +2 -2
+		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.

Quoted 11 lines
tests/nft-parsing-test.c +4 -2
 		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;
author
#16 lore raw

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

Quoted 5 lines · Pablo Neira Ayuso

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.

Signature · 5 lines
Arturo Borrero González
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html