Skip to contentSkip to conversation

parsing: add interface to parse from file

PATCH In review Arturo Borrero Gonzalez 2 participants 4 messages Jan 3, 2014 – Jan 6, 2014 netfilter

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 parsing: add interface to parse from file

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.

about the implementation:
* a small selector is added in src/internal.h to avoid duplicating so much code.
* The ifdef/endif logic in internal.h is reworked, looking for a cleaner file, and also allowing us to use without fear XML/JSON internal datatypes.

Examples will be updated in a separated patch, but the test infraestructure is updated, using this new file-parsing approach.

---
v1: initial release, using a filename/path as input.
v2: rework the patch and use a FILE *stream as input. Implement the
struct parse_ops and update nft-parsing-test to include tests for this new API.

 include/libnftables/chain.h   |    1 +
 include/libnftables/rule.h    |    1 +
 include/libnftables/ruleset.h |    1 +
 include/libnftables/set.h     |    2 +
 include/libnftables/table.h   |    1 +
 src/chain.c                   |   30 ++++++++++++----
 src/internal.h                |   45 ++++++++++++++++++++++--
 src/jansson.c                 |   25 +++++++++++++
 src/libnftables.map           |    6 +++
 src/mxml.c                    |   31 ++++++++++++++++
 src/rule.c                    |   28 +++++++++++----
 src/ruleset.c                 |   30 ++++++++++++----
 src/set.c                     |   29 +++++++++++----
 src/set_elem.c                |   46 ++++++++++++++++++++++--
 src/table.c                   |   31 ++++++++++++----
 tests/jsonfiles/01-table.json |    1 -
 tests/nft-parsing-test.c      |   78 ++++++++++++++++++++++++-----------------
 17 files changed, 301 insertions(+), 85 deletions(-)
 delete mode 100644 tests/jsonfiles/01-table.json
include/libnftables/chain.h +1 -0
index 8b4eab9..2cffd35 100644
@@ -52,6 +52,7 @@ struct nlmsghdr;
 void nft_chain_nlmsg_build_payload(struct nlmsghdr *nlh, const struct nft_chain *t);
 
 int nft_chain_parse(struct nft_chain *c, enum nft_parse_type type, const char *data);
+int nft_chain_fparse(struct nft_chain *c, enum nft_parse_type type, FILE *fp);
 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 +1 -0
index 86dbc17..5f2fa5e 100644
@@ -48,6 +48,7 @@ struct nlmsghdr;
 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);
+int nft_rule_fparse(struct nft_rule *r, enum nft_parse_type type, FILE *fp);
 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 +1 -0
index 1ec3059..1501fc2 100644
@@ -31,6 +31,7 @@ void nft_ruleset_attr_set(struct nft_ruleset *r, uint16_t attr, void *data);
 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);
+int nft_ruleset_fparse(struct nft_ruleset *rs, enum nft_parse_type type, FILE *fp);
 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 +2 -0
index 13ac857..ec12769 100644
@@ -61,6 +61,7 @@ struct nft_set *nft_set_list_iter_next(struct nft_set_list_iter *iter);
 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);
+int nft_set_fparse(struct nft_set *s, enum nft_parse_type type, FILE *fp);
 
 /*
  * Set elements
@@ -99,6 +100,7 @@ void nft_set_elem_nlmsg_build_payload(struct nlmsghdr *nlh, struct nft_set_elem
 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);
+int nft_set_elem_fparse(struct nft_set_elem *e, enum nft_parse_type type, FILE *fp);
 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 +1 -0
index 1d2be07..8758289 100644
@@ -41,6 +41,7 @@ struct nlmsghdr;
 void nft_table_nlmsg_build_payload(struct nlmsghdr *nlh, const struct nft_table *t);
 
 int nft_table_parse(struct nft_table *t, enum nft_parse_type type, const char *data);
+int nft_table_fparse(struct nft_table *t, enum nft_parse_type type, FILE *fp);
 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 +22 -8
index a0004b5..45af8db 100644
@@ -587,13 +587,14 @@ err:
 }
 #endif
 
-static int nft_chain_json_parse(struct nft_chain *c, const char *json)
+static int nft_chain_json_buildparse(struct nft_chain *c, const char *json,
+				     const struct parse_ops *ops)
 {
 #ifdef JSON_PARSING
 	json_t *tree;
 	json_error_t error;
 
-	tree = nft_jansson_create_root(json, &error);
+	tree = ops->jsonbuilder(json, &error);
 	if (tree == NULL)
 		return -1;
 
@@ -702,11 +703,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)
+static int nft_chain_xml_buildparse(struct nft_chain *c, const char *xml,
+				    const struct parse_ops *ops)
 {
 #ifdef XML_PARSING
 	int ret;
-	mxml_node_t *tree = nft_mxml_build_tree(xml, "chain");
+	mxml_node_t *tree = ops->xmlbuilder(xml, "chain");
 	if (tree == NULL)
 		return -1;
 
@@ -719,17 +721,17 @@ 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)
+static int nft_chain_do_parse(struct nft_chain *c, enum nft_parse_type type,
+			      const void *data, const struct parse_ops *ops)
 {
 	int ret;
 
 	switch (type) {
 	case NFT_PARSE_XML:
-		ret = nft_chain_xml_parse(c, data);
+		ret = nft_chain_xml_buildparse(c, data, ops);
 		break;
 	case NFT_PARSE_JSON:
-		ret = nft_chain_json_parse(c, data);
+		ret = nft_chain_json_buildparse(c, data, ops);
 		break;
 	default:
 		ret = -1;
@@ -739,8 +741,20 @@ 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)
+{
+	return nft_chain_do_parse(c, type, data, &parse_string_ops);
+}
 EXPORT_SYMBOL(nft_chain_parse);
 
+int nft_chain_fparse(struct nft_chain *c, enum nft_parse_type type, FILE *fp)
+{
+	return nft_chain_do_parse(c, type, fp, &parse_stream_ops);
+}
+EXPORT_SYMBOL(nft_chain_fparse);
+
+
 static int nft_chain_snprintf_json(char *buf, size_t size, struct nft_chain *c)
 {
 	int ret, len = size, offset = 0;
src/internal.h +41 -4

79 lines not on this page · raw message

src/jansson.c +24 -1
index 04146e2..53c9272 100644
@@ -76,7 +76,7 @@ bool nft_jansson_node_exist(json_t *root, const char *tag)
 	return json_object_get(root, tag) != NULL;
 }
 
-json_t *nft_jansson_create_root(const char *json, json_error_t *err)
+static json_t *nft_jansson_do_create_root(const char *json, json_error_t *err)
 {
 	json_t *root;
 
@@ -89,6 +89,29 @@ json_t *nft_jansson_create_root(const char *json, json_error_t *err)
 	return root;
 }
 
+json_t *nft_jansson_create_root(const void *data, void *err)
+{
+	return nft_jansson_do_create_root(data, err);
+}
+
+static json_t *nft_jansson_do_create_root_file(FILE *f, json_error_t *err)
+{
+	json_t *root;
+
+	root = json_loadf(f, 0, err);
+	if (root == NULL) {
+		errno = EINVAL;
+		return NULL;
+	}
+
+	return root;
+}
+
+json_t *nft_jansson_create_root_file(const void *data, void *err)
+{
+	return nft_jansson_do_create_root_file((FILE *)data, err);
+}
+
 json_t *nft_jansson_get_node(json_t *root, const char *tag)
 {
 	json_t *node;
src/libnftables.map +6 -0
index 7dc9aee..eb5ed5c 100644
@@ -13,6 +13,7 @@ global:
   nft_table_attr_get_u32;
   nft_table_attr_get_str;
   nft_table_parse;
+  nft_table_fparse;
   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_fparse;
   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_fparse;
   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_fparse;
   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_fparse;
   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_fparse;
   nft_ruleset_snprintf;
   nft_ruleset_fprintf;
 
src/mxml.c +30 -1
index 82156b7..3457207 100644
@@ -22,7 +22,9 @@
 #include <libnftables/set.h>
 
 #ifdef XML_PARSING
-mxml_node_t *nft_mxml_build_tree(const char *xml, const char *treename)
+
+static mxml_node_t *nft_mxml_do_build_tree(const char *xml,
+					   const char *treename)
 {
 	mxml_node_t *tree;
 
@@ -39,6 +41,33 @@ err:
 	return NULL;
 }
 
+mxml_node_t *nft_mxml_build_tree(const void *data, const char *treename)
+{
+	return nft_mxml_do_build_tree(data, treename);
+}
+
+static mxml_node_t *nft_mxml_do_build_tree_file(FILE *f, const char *treename)
+{
+	mxml_node_t *tree;
+
+	tree = mxmlLoadFile(NULL, f, MXML_OPAQUE_CALLBACK);
+	if (tree == NULL)
+		goto err;
+
+	if (strcmp(tree->value.opaque, treename) == 0)
+		return tree;
+
+	mxmlDelete(tree);
+err:
+	errno = EINVAL;
+	return NULL;
+}
+
+mxml_node_t *nft_mxml_build_tree_file(const void *data, const char *treename)
+{
+	return nft_mxml_do_build_tree_file((void *)data, treename);
+}
+
 struct nft_rule_expr *nft_mxml_expr_parse(mxml_node_t *node)
 {
 	mxml_node_t *tree;
src/rule.c +20 -8
index 280350a..55312e5 100644
@@ -521,13 +521,14 @@ err:
 }
 #endif
 
-static int nft_rule_json_parse(struct nft_rule *r, const char *json)
+static int nft_rule_json_buildparse(struct nft_rule *r, const char *json,
+				    const struct parse_ops *ops)
 {
 #ifdef JSON_PARSING
 	json_t *tree;
 	json_error_t error;
 
-	tree = nft_jansson_create_root(json, &error);
+	tree = ops->jsonbuilder(json, &error);
 	if (tree == NULL)
 		return -1;
 
@@ -620,11 +621,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)
+static int nft_rule_xml_buildparse(struct nft_rule *r, const char *xml,
+				   const struct parse_ops *ops)
 {
 #ifdef XML_PARSING
 	int ret;
-	mxml_node_t *tree = nft_mxml_build_tree(xml, "rule");
+	mxml_node_t *tree = ops->xmlbuilder(xml, "rule");
 	if (tree == NULL)
 		return -1;
 
@@ -637,17 +639,17 @@ 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)
+static int nft_rule_do_parse(struct nft_rule *r, enum nft_parse_type type,
+			     const void *data, const struct parse_ops *ops)
 {
 	int ret;
 
 	switch (type) {
 	case NFT_PARSE_XML:
-		ret = nft_rule_xml_parse(r, data);
+		ret = nft_rule_xml_buildparse(r, data, ops);
 		break;
 	case NFT_PARSE_JSON:
-		ret = nft_rule_json_parse(r, data);
+		ret = nft_rule_json_buildparse(r, data, ops);
 		break;
 	default:
 		ret = -1;
@@ -657,8 +659,18 @@ 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)
+{
+	return nft_rule_do_parse(r, type, data, &parse_string_ops);
+}
 EXPORT_SYMBOL(nft_rule_parse);
 
+int nft_rule_fparse(struct nft_rule *r, enum nft_parse_type type, FILE *fp)
+{
+	return nft_rule_do_parse(r, type, fp, &parse_stream_ops);
+}
+EXPORT_SYMBOL(nft_rule_fparse);
+
 static int nft_rule_snprintf_json(char *buf, size_t size, struct nft_rule *r,
 					 uint32_t type, uint32_t flags)
 {
src/ruleset.c +22 -8
index f591382..953d663 100644
@@ -326,13 +326,14 @@ err:
 
 #endif
 
-static int nft_ruleset_json_parse(struct nft_ruleset *rs, const char *json)
+static int nft_ruleset_json_buildparse(struct nft_ruleset *rs, const char *json,
+				       const struct parse_ops *ops)
 {
 #ifdef JSON_PARSING
 	json_t *root, *array;
 	json_error_t error;
 
-	root = nft_jansson_create_root(json, &error);
+	root = ops->jsonbuilder(json, &error);
 	if (root == NULL)
 		return -1;
 
@@ -525,12 +526,13 @@ err_free:
 }
 #endif
 
-static int nft_ruleset_xml_parse(struct nft_ruleset *rs, const char *xml)
+static int nft_ruleset_xml_buildparse(struct nft_ruleset *rs, const char *xml,
+				      const struct parse_ops *ops)
 {
 #ifdef XML_PARSING
 	mxml_node_t *tree;
 
-	tree = nft_mxml_build_tree(xml, "nftables");
+	tree = ops->xmlbuilder(xml, "nftables");
 	if (tree == NULL)
 		return -1;
 
@@ -557,17 +559,17 @@ err:
 #endif
 }
 
-int nft_ruleset_parse(struct nft_ruleset *r, enum nft_parse_type type,
-		      const char *data)
+static int nft_ruleset_do_parse(struct nft_ruleset *r, enum nft_parse_type type,
+				const void *data, const struct parse_ops *ops)
 {
 	int ret;
 
 	switch (type) {
 	case NFT_PARSE_XML:
-		ret = nft_ruleset_xml_parse(r, data);
+		ret = nft_ruleset_xml_buildparse(r, data, ops);
 		break;
 	case NFT_PARSE_JSON:
-		ret = nft_ruleset_json_parse(r, data);
+		ret = nft_ruleset_json_buildparse(r, data, ops);
 		break;
 	default:
 		ret = -1;
@@ -577,8 +579,20 @@ 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)
+{
+	return nft_ruleset_do_parse(rs, type, data, &parse_string_ops);
+}
 EXPORT_SYMBOL(nft_ruleset_parse);
 
+int nft_ruleset_fparse(struct nft_ruleset *rs, enum nft_parse_type type, FILE *fp)
+{
+	return nft_ruleset_do_parse(rs, type, fp, &parse_stream_ops);
+}
+EXPORT_SYMBOL(nft_ruleset_fparse);
+
 static const char *nft_ruleset_o_opentag(uint32_t type)
 {
 	switch (type) {
src/set.c +21 -8
index c5204cc..624a659 100644
@@ -371,13 +371,14 @@ err:
 }
 #endif
 
-static int nft_set_json_parse(struct nft_set *s, const char *json)
+static int nft_set_json_buildparse(struct nft_set *s, const char *json,
+				   const struct parse_ops *ops)
 {
 #ifdef JSON_PARSING
 	json_t *tree;
 	json_error_t error;
 
-	tree = nft_jansson_create_root(json, &error);
+	tree = ops->jsonbuilder(json, &error);
 	if (tree == NULL)
 		return -1;
 
@@ -477,11 +478,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)
+static int nft_set_xml_buildparse(struct nft_set *s, const void *data,
+				  const struct parse_ops *ops)
 {
 #ifdef XML_PARSING
 	int ret;
-	mxml_node_t *tree = nft_mxml_build_tree(xml, "set");
+	mxml_node_t *tree = ops->xmlbuilder(data, "set");
 	if (tree == NULL)
 		return -1;
 
@@ -494,17 +496,17 @@ 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)
+static int nft_set_do_parse(struct nft_set *s, enum nft_parse_type type,
+			    const void *data, const struct parse_ops *ops)
 {
 	int ret;
 
 	switch (type) {
 	case NFT_PARSE_XML:
-		ret = nft_set_xml_parse(s, data);
+		ret = nft_set_xml_buildparse(s, data, ops);
 		break;
 	case NFT_PARSE_JSON:
-		ret = nft_set_json_parse(s, data);
+		ret = nft_set_json_buildparse(s, data, ops);
 		break;
 	default:
 		ret = -1;
@@ -514,8 +516,19 @@ 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)
+{
+	return nft_set_do_parse(s, type, data, &parse_string_ops);
+}
 EXPORT_SYMBOL(nft_set_parse);
 
+int nft_set_fparse(struct nft_set *s, enum nft_parse_type type, FILE *fp)
+{
+	return nft_set_do_parse(s, type, fp, &parse_stream_ops);
+}
+EXPORT_SYMBOL(nft_set_fparse);
+
 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 +41 -5

75 lines not on this page · raw message

src/table.c +23 -8
index ba84264..53cb8c1 100644
@@ -265,11 +265,13 @@ 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)
+static int nft_table_xml_buildparse(struct nft_table *t, const void *data,
+				    const struct parse_ops *ops)
 {
 #ifdef XML_PARSING
 	int ret;
-	mxml_node_t *tree = nft_mxml_build_tree(xml, "table");
+	mxml_node_t *tree = (mxml_node_t *)ops->xmlbuilder(data, "table");
+
 	if (tree == NULL)
 		return -1;
 
@@ -318,13 +320,14 @@ err:
 }
 #endif
 
-static int nft_table_json_parse(struct nft_table *t, const char *json)
+static int nft_table_json_buildparse(struct nft_table *t, const void *data,
+				     const struct parse_ops *ops)
 {
 #ifdef JSON_PARSING
 	json_t *tree;
 	json_error_t error;
 
-	tree = nft_jansson_create_root(json, &error);
+	tree = (json_t *)ops->jsonbuilder(data, &error);
 	if (tree == NULL)
 		return -1;
 
@@ -335,17 +338,17 @@ 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)
+static int nft_table_do_parse(struct nft_table *t, enum nft_parse_type type,
+			      const void *data, const struct parse_ops *ops)
 {
 	int ret;
 
 	switch (type) {
 	case NFT_PARSE_XML:
-		ret = nft_table_xml_parse(t, data);
+		ret = nft_table_xml_buildparse(t, data, ops);
 		break;
 	case NFT_PARSE_JSON:
-		ret = nft_table_json_parse(t, data);
+		ret = nft_table_json_buildparse(t, data, ops);
 		break;
 	default:
 		ret = -1;
@@ -355,8 +358,20 @@ 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)
+{
+	return nft_table_do_parse(t, type, data, &parse_string_ops);
+}
 EXPORT_SYMBOL(nft_table_parse);
 
+int nft_table_fparse(struct nft_table *t, enum nft_parse_type type, FILE *fp)
+{
+	return nft_table_do_parse(t, type, fp, &parse_stream_ops);
+}
+EXPORT_SYMBOL(nft_table_fparse);
+
 static int nft_table_snprintf_json(char *buf, size_t size, struct nft_table *t)
 {
 	return snprintf(buf, size,
tests/jsonfiles/01-table.json deleted +0 -1
deleted file mode 100644
index f217189..0000000
@@ -1 +0,0 @@
-{"table":{"name":"filter","family":"ip","flags":0,"use":0}}
tests/nft-parsing-test.c +45 -33

208 lines not on this page · raw message

#1 lore raw
Quoted 7 lines · 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.

about the implementation:
* a small selector is added in src/internal.h to avoid duplicating so much code.

This needs to go in a separated patch.

Firstly, patches series with small cleanups/polishing to prepare the feature, then, the last patch should be the new feature itself. That helps the reviewer and it's good for the git history as we stick to "one commit - one logical change".

Quoted 2 lines

* The ifdef/endif logic in internal.h is reworked, looking for a cleaner file, and also allowing us to use without fear XML/JSON internal datatypes.

It's good if we simplify that ifdef burden that we have. Please, rework this in an initial cleanup patch.

In this case, I'd like to see something like a changeset composed of three patches.

Quoted 35 lines

Examples will be updated in a separated patch, but the test infraestructure is updated, using this new file-parsing approach.

---
v1: initial release, using a filename/path as input.
v2: rework the patch and use a FILE *stream as input. Implement the
struct parse_ops and update nft-parsing-test to include tests for this new API.

 include/libnftables/chain.h   |    1 +
 include/libnftables/rule.h    |    1 +
 include/libnftables/ruleset.h |    1 +
 include/libnftables/set.h     |    2 +
 include/libnftables/table.h   |    1 +
 src/chain.c                   |   30 ++++++++++++----
 src/internal.h                |   45 ++++++++++++++++++++++--
 src/jansson.c                 |   25 +++++++++++++
 src/libnftables.map           |    6 +++
 src/mxml.c                    |   31 ++++++++++++++++
 src/rule.c                    |   28 +++++++++++----
 src/ruleset.c                 |   30 ++++++++++++----
 src/set.c                     |   29 +++++++++++----
 src/set_elem.c                |   46 ++++++++++++++++++++++--
 src/table.c                   |   31 ++++++++++++----
 tests/jsonfiles/01-table.json |    1 -
 tests/nft-parsing-test.c      |   78 ++++++++++++++++++++++++-----------------
 17 files changed, 301 insertions(+), 85 deletions(-)
 delete mode 100644 tests/jsonfiles/01-table.json
include/libnftables/chain.h +0 -0
index 8b4eab9..2cffd35 100644

include/libnftables/chain.h struct nlmsghdr; · line 55

Quoted 4 lines
include/libnftables/chain.h +1 -0
@@ -52,6 +52,7 @@ struct nlmsghdr;
 void nft_chain_nlmsg_build_payload(struct nlmsghdr *nlh, const struct nft_chain *t);
 
 int nft_chain_parse(struct nft_chain *c, enum nft_parse_type type, const char *data);
+int nft_chain_fparse(struct nft_chain *c, enum nft_parse_type type, FILE *fp);

perhaps you can call all this function _parse_file(...) instead.

src/internal.h

Quoted 141 lines
include/libnftables/chain.h +0 -0
 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 +1 -0
index 86dbc17..5f2fa5e 100644
@@ -48,6 +48,7 @@ struct nlmsghdr;
 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);
+int nft_rule_fparse(struct nft_rule *r, enum nft_parse_type type, FILE *fp);
 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 +1 -0
index 1ec3059..1501fc2 100644
@@ -31,6 +31,7 @@ void nft_ruleset_attr_set(struct nft_ruleset *r, uint16_t attr, void *data);
 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);
+int nft_ruleset_fparse(struct nft_ruleset *rs, enum nft_parse_type type, FILE *fp);
 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 +2 -0
index 13ac857..ec12769 100644
@@ -61,6 +61,7 @@ struct nft_set *nft_set_list_iter_next(struct nft_set_list_iter *iter);
 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);
+int nft_set_fparse(struct nft_set *s, enum nft_parse_type type, FILE *fp);
 
 /*
  * Set elements
@@ -99,6 +100,7 @@ void nft_set_elem_nlmsg_build_payload(struct nlmsghdr *nlh, struct nft_set_elem
 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);
+int nft_set_elem_fparse(struct nft_set_elem *e, enum nft_parse_type type, FILE *fp);
 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 +1 -0
index 1d2be07..8758289 100644
@@ -41,6 +41,7 @@ struct nlmsghdr;
 void nft_table_nlmsg_build_payload(struct nlmsghdr *nlh, const struct nft_table *t);
 
 int nft_table_parse(struct nft_table *t, enum nft_parse_type type, const char *data);
+int nft_table_fparse(struct nft_table *t, enum nft_parse_type type, FILE *fp);
 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 +22 -8
index a0004b5..45af8db 100644
@@ -587,13 +587,14 @@ err:
 }
 #endif
 
-static int nft_chain_json_parse(struct nft_chain *c, const char *json)
+static int nft_chain_json_buildparse(struct nft_chain *c, const char *json,
+				     const struct parse_ops *ops)
 {
 #ifdef JSON_PARSING
 	json_t *tree;
 	json_error_t error;
 
-	tree = nft_jansson_create_root(json, &error);
+	tree = ops->jsonbuilder(json, &error);
 	if (tree == NULL)
 		return -1;
 
@@ -702,11 +703,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)
+static int nft_chain_xml_buildparse(struct nft_chain *c, const char *xml,
+				    const struct parse_ops *ops)
 {
 #ifdef XML_PARSING
 	int ret;
-	mxml_node_t *tree = nft_mxml_build_tree(xml, "chain");
+	mxml_node_t *tree = ops->xmlbuilder(xml, "chain");
 	if (tree == NULL)
 		return -1;
 
@@ -719,17 +721,17 @@ 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)
+static int nft_chain_do_parse(struct nft_chain *c, enum nft_parse_type type,
+			      const void *data, const struct parse_ops *ops)
 {
 	int ret;
 
 	switch (type) {
 	case NFT_PARSE_XML:
-		ret = nft_chain_xml_parse(c, data);
+		ret = nft_chain_xml_buildparse(c, data, ops);
 		break;
 	case NFT_PARSE_JSON:
-		ret = nft_chain_json_parse(c, data);
+		ret = nft_chain_json_buildparse(c, data, ops);
 		break;
 	default:
 		ret = -1;
@@ -739,8 +741,20 @@ 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)
+{
+	return nft_chain_do_parse(c, type, data, &parse_string_ops);
+}
 EXPORT_SYMBOL(nft_chain_parse);
 
+int nft_chain_fparse(struct nft_chain *c, enum nft_parse_type type, FILE *fp)
+{
+	return nft_chain_do_parse(c, type, fp, &parse_stream_ops);
+}
+EXPORT_SYMBOL(nft_chain_fparse);
+
+
 static int nft_chain_snprintf_json(char *buf, size_t size, struct nft_chain *c)
 {
 	int ret, len = size, offset = 0;
src/internal.h +0 -0
index a10d874..e0b9761 100644

Alvaro's patch is also changing the internal.h file. You need to coordinate since you're working on the parsing infrastructure at the same time to avoid clashing on this, otherwise the patch coming later won't apply and one of you will have to rework it.

#2 lore raw
Quoted 3 lines · Pablo Neira Ayuso

In this case, I'd like to see something like a changeset composed of three patches.

Ok!

Quoted 3 lines

perhaps you can call all this function _parse_file(...) instead.

of course.

Quoted 5 lines

Alvaro's patch is also changing the internal.h file. You need to coordinate since you're working on the parsing infrastructure at the same time to avoid clashing on this, otherwise the patch coming later won't apply and one of you will have to rework it.

I think Alvaro's patch should come first, as he was already working on this previous to my patch.

I will wait until alvaro's one is applied to make the refresh.

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
#3 lore raw

On Mon, Jan 06, 2014 at 01:06:04PM +0100, Arturo Borrero Gonzalez wrote:
[...]

Quoted 9 lines
Quoted 4 lines

Alvaro's patch is also changing the internal.h file. You need to coordinate since you're working on the parsing infrastructure at the same time to avoid clashing on this, otherwise the patch coming later won't apply and one of you will have to rework it.

I think Alvaro's patch should come first, as he was already working on this previous to my patch.

I will wait until alvaro's one is applied to make the refresh.

I did apply it already, thanks Arturo.