[libnftnl PATCH 0/5] Set: Do not print unset value

[libnftnl PATCH 0/5] Set: Do not print unset value

[Ana Rey]

This patchset do task about does not print unset value in xml and json file
and about refactoring code: "Use nft_rule_expr_set_* in the xml parsing
code" Also, It add test files that is need to be modificated to these patch
and It fixes a leak memory.

Ana Rey (5):
  src: set: Use nft_rule_expr_set_* in the xml parsing code
  src: set: Do not print unset values in xml
  src: set: Do not print unset values in json
  src: set: Free memory in the same function that is reserved.
  tests: xmlfile: Test files without unset child elements in set
    elements

 src/jansson.c                 |  12 +--
 src/set.c                     | 208 +++++++++++++++++++++++++-----------------
 src/set_elem.c                |  50 +++++-----
 tests/xmlfiles/73-set.xml     |   2 +-
 tests/xmlfiles/74-set.xml     |   2 +-
 tests/xmlfiles/75-ruleset.xml |   2 +-
 6 files changed, 158 insertions(+), 118 deletions(-)

-- 
2.0.0

[libnftnl PATCH 1/5] src: set: Use nft_rule_expr_set_* in the xml parsing code

[Ana Rey]

Code refactoring to use nft_rule_expr_set_* in parse functions.

Signed-off-by: Ana Rey <anarey@gmail.com>
---
 src/set.c      | 46 ++++++++++++++++------------------------------
 src/set_elem.c | 10 +++++-----
 2 files changed, 21 insertions(+), 35 deletions(-)

diff --git a/src/set.c b/src/set.c
index 57bbcaa..d1d176c 100644
--- a/src/set.c
+++ b/src/set.c
@@ -454,67 +454,53 @@ int nft_mxml_set_parse(mxml_node_t *tree, struct nft_set *s,
 	struct nft_set_elem *elem;
 	const char *name, *table;
 	int family;
+	uint32_t set_flags, key_type, key_len;
+	uint32_t data_type, data_len;
 
 	name = nft_mxml_str_parse(tree, "name", MXML_DESCEND_FIRST,
 				  NFT_XML_MAND, err);
 	if (name == NULL)
 		return -1;
-
-	if (s->name)
-		xfree(s->name);
-
-	s->name = strdup(name);
-	s->flags |= (1 << NFT_SET_ATTR_NAME);
+	nft_set_attr_set_str(s, NFT_SET_ATTR_NAME, name);
 
 	table = nft_mxml_str_parse(tree, "table", MXML_DESCEND_FIRST,
 				   NFT_XML_MAND, err);
 	if (table == NULL)
 		return -1;
-
-	if (s->table)
-		xfree(s->table);
-
-	s->table = strdup(table);
-	s->flags |= (1 << NFT_SET_ATTR_TABLE);
+	nft_set_attr_set_str(s, NFT_SET_ATTR_TABLE, table);
 
 	family = nft_mxml_family_parse(tree, "family", MXML_DESCEND_FIRST,
 				       NFT_XML_MAND, err);
 	if (family < 0)
 		return -1;
-
-	s->family = family;
-
-	s->flags |= (1 << NFT_SET_ATTR_FAMILY);
+	nft_set_attr_set_u32(s, NFT_SET_ATTR_FAMILY, family);
 
 	if (nft_mxml_num_parse(tree, "flags", MXML_DESCEND_FIRST, BASE_DEC,
-			       &s->set_flags, NFT_TYPE_U32, NFT_XML_MAND, err) != 0)
+			       &set_flags, NFT_TYPE_U32, NFT_XML_MAND, err) < 0)
 		return -1;
-
-	s->flags |= (1 << NFT_SET_ATTR_FLAGS);
+	nft_set_attr_set_u32(s, NFT_SET_ATTR_FLAGS, set_flags);
 
 	if (nft_mxml_num_parse(tree, "key_type", MXML_DESCEND_FIRST, BASE_DEC,
-			       &s->key_type, NFT_TYPE_U32, NFT_XML_MAND, err) != 0)
+			       &key_type, NFT_TYPE_U32, NFT_XML_MAND, err) < 0)
 		return -1;
-
-	s->flags |= (1 << NFT_SET_ATTR_KEY_TYPE);
+	nft_set_attr_set_u32(s, NFT_SET_ATTR_KEY_TYPE, key_type);
 
 	if (nft_mxml_num_parse(tree, "key_len", MXML_DESCEND_FIRST, BASE_DEC,
-			       &s->key_len, NFT_TYPE_U32, NFT_XML_MAND, err) != 0)
+			       &key_len, NFT_TYPE_U32, NFT_XML_MAND, err) < 0)
 		return -1;
-
-	s->flags |= (1 << NFT_SET_ATTR_KEY_LEN);
+	nft_set_attr_set_u32(s, NFT_SET_ATTR_KEY_LEN, key_len);
 
 	if (nft_mxml_num_parse(tree, "data_type", MXML_DESCEND_FIRST, BASE_DEC,
-			       &s->data_type, NFT_TYPE_U32,
+			       &data_type, NFT_TYPE_U32,
 			       NFT_XML_OPT, err) == 0) {
-		s->flags |= (1 << NFT_SET_ATTR_DATA_TYPE);
+		nft_set_attr_set_u32(s, NFT_SET_ATTR_DATA_TYPE, data_type);
 
 		if (nft_mxml_num_parse(tree, "data_len", MXML_DESCEND_FIRST,
-				       BASE_DEC, &s->data_len, NFT_TYPE_U32,
-				       NFT_XML_MAND, err) != 0)
+				       BASE_DEC, &data_len, NFT_TYPE_U32,
+				       NFT_XML_MAND, err) < 0)
 			return -1;
+		nft_set_attr_set_u32(s, NFT_SET_ATTR_DATA_LEN, data_len);
 
-		s->flags |= (1 << NFT_SET_ATTR_DATA_LEN);
 	}
 
 	for (node = mxmlFindElement(tree, tree, "set_elem", NULL,
diff --git a/src/set_elem.c b/src/set_elem.c
index a56fcb2..9d1dc41 100644
--- a/src/set_elem.c
+++ b/src/set_elem.c
@@ -382,13 +382,13 @@ int nft_mxml_set_elem_parse(mxml_node_t *tree, struct nft_set_elem *e,
 			    struct nft_parse_err *err)
 {
 	int set_elem_data;
+	uint32_t set_elem_flags;
 
-	if (nft_mxml_num_parse(tree, "flags", MXML_DESCEND_FIRST,
-			       BASE_DEC, &e->set_elem_flags,
-			       NFT_TYPE_U32, NFT_XML_MAND, err) != 0)
+	if (nft_mxml_num_parse(tree, "flags", MXML_DESCEND_FIRST, BASE_DEC,
+			       &set_elem_flags, NFT_TYPE_U32, NFT_XML_MAND,
+			       err) < 0)
 		return -1;
-
-	e->flags |= (1 << NFT_SET_ELEM_ATTR_FLAGS);
+	nft_set_elem_attr_set_u32(e, NFT_SET_ELEM_ATTR_FLAGS, set_elem_flags);
 
 	if (nft_mxml_data_reg_parse(tree, "key", &e->key,
 				    NFT_XML_MAND, err) != DATA_VALUE)
-- 
2.0.0

Re: [libnftnl PATCH 1/5] src: set: Use nft_rule_expr_set_* in the xml parsing code

[Pablo Neira Ayuso]

On Wed, Jun 11, 2014 at 05:50:46PM +0200, Ana Rey wrote:
> Code refactoring to use nft_rule_expr_set_* in parse functions.

Applied, thanks Ana.

[libnftnl PATCH] tests: Use the system errors in test_xml.

[Ana Rey]

Add the use of system errors (nft_parse_perror) in test_xml to know which
node is not found.

Example:
parsing xmlfiles/75-ruleset.xml: FAILED (Invalid argument)
fail : Node "flags" not found

Signed-off-by: Ana Rey <anarey@gmail.com>
---
 tests/nft-parsing-test.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/nft-parsing-test.c b/tests/nft-parsing-test.c
index 2a15109..5eff0b9 100644
--- a/tests/nft-parsing-test.c
+++ b/tests/nft-parsing-test.c
@@ -177,6 +177,7 @@ failparsing:
 	fclose(fp);
 	printf("parsing %s: ", filename);
 	printf("\033[31mFAILED\e[0m (%s)\n", strerror(errno));
+	nft_parse_perror("fail", err);
 	return -1;
 }
 
-- 
2.0.0

Re: [libnftnl PATCH] tests: Use the system errors in test_xml.

[Pablo Neira Ayuso]

On Wed, Jun 11, 2014 at 05:50:47PM +0200, Ana Rey wrote:
> Add the use of system errors (nft_parse_perror) in test_xml to know which
> node is not found.
> 
> Example:
> parsing xmlfiles/75-ruleset.xml: FAILED (Invalid argument)
> fail : Node "flags" not found

Renamed 'fail' to 'Reason'. I know the other part of code it was using
'fail', but it already said 'FAILED' above, so I think it clarifies a
bit more.

> Signed-off-by: Ana Rey <anarey@gmail.com>
> ---
>  tests/nft-parsing-test.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tests/nft-parsing-test.c b/tests/nft-parsing-test.c
> index 2a15109..5eff0b9 100644
> --- a/tests/nft-parsing-test.c
> +++ b/tests/nft-parsing-test.c
> @@ -177,6 +177,7 @@ failparsing:
>  	fclose(fp);
>  	printf("parsing %s: ", filename);
>  	printf("\033[31mFAILED\e[0m (%s)\n", strerror(errno));
> +	nft_parse_perror("fail", err);
>  	return -1;
>  }
>  
> -- 
> 2.0.0
> 
> --
> 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

[libnftnl PATCH 2/5] src: set: Do not print unset values in xml

[Ana Rey]

It changes the parse and the snprint functions to omit unset values.

This xml file is gotten for a set:

<set>
	<family>unknown</family>
	<table>test</table>
	<name>miset</name>
	<flags>0</flags>
	<key_type>0</key_type>
	<key_len>0</key_len>
	<set_elem>
		<flags>0</flags>
		<key>
			<data_reg type="value">
				<len>4</len>
				<data0>0x0401a8c0</data0>
			</data_reg>
		</key>
	</set_elem>
	<set_elem>
		<flags>0</flags>
		<key>
			<data_reg type="value">
				<len>4</len>
				<data0>0x0501a8c0</data0>
			</data_reg>
		</key>
	</set_elem>
</set>

Now, this xml file is gotten without unset values:

<set>
	<family>unknown</family>
	<table>test</table>
	<name>miset</name>
	<set_elem>
		<key>
			<data_reg type="value">
				<len>4</len>
				<data0>0x00000002</data0>
			</data_reg>
		</key>
	</set_elem>
	<set_elem>
		<key>
			<data_reg type="value">
				<len>4</len>
				<data0>0x0401a8c0</data0>
			</data_reg>
		</key>
	</set_elem>
</set>

Signed-off-by: Ana Rey <anarey@gmail.com>
---
 src/set.c      | 76 +++++++++++++++++++++++++++++++++++++++-------------------
 src/set_elem.c | 36 ++++++++++++++++------------
 2 files changed, 72 insertions(+), 40 deletions(-)

diff --git a/src/set.c b/src/set.c
index d1d176c..e328288 100644
--- a/src/set.c
+++ b/src/set.c
@@ -471,19 +471,17 @@ int nft_mxml_set_parse(mxml_node_t *tree, struct nft_set *s,
 
 	family = nft_mxml_family_parse(tree, "family", MXML_DESCEND_FIRST,
 				       NFT_XML_MAND, err);
-	if (family < 0)
-		return -1;
-	nft_set_attr_set_u32(s, NFT_SET_ATTR_FAMILY, family);
+	if (family >= 0)
+		nft_set_attr_set_u32(s, NFT_SET_ATTR_FAMILY, family);
 
 	if (nft_mxml_num_parse(tree, "flags", MXML_DESCEND_FIRST, BASE_DEC,
-			       &set_flags, NFT_TYPE_U32, NFT_XML_MAND, err) < 0)
-		return -1;
-	nft_set_attr_set_u32(s, NFT_SET_ATTR_FLAGS, set_flags);
+			       &set_flags, NFT_TYPE_U32, NFT_XML_MAND,
+			       err) == 0)
+		nft_set_attr_set_u32(s, NFT_SET_ATTR_FLAGS, set_flags);
 
 	if (nft_mxml_num_parse(tree, "key_type", MXML_DESCEND_FIRST, BASE_DEC,
-			       &key_type, NFT_TYPE_U32, NFT_XML_MAND, err) < 0)
-		return -1;
-	nft_set_attr_set_u32(s, NFT_SET_ATTR_KEY_TYPE, key_type);
+			       &key_type, NFT_TYPE_U32, NFT_XML_MAND, err) == 0)
+		nft_set_attr_set_u32(s, NFT_SET_ATTR_KEY_TYPE, key_type);
 
 	if (nft_mxml_num_parse(tree, "key_len", MXML_DESCEND_FIRST, BASE_DEC,
 			       &key_len, NFT_TYPE_U32, NFT_XML_MAND, err) < 0)
@@ -497,9 +495,8 @@ int nft_mxml_set_parse(mxml_node_t *tree, struct nft_set *s,
 
 		if (nft_mxml_num_parse(tree, "data_len", MXML_DESCEND_FIRST,
 				       BASE_DEC, &data_len, NFT_TYPE_U32,
-				       NFT_XML_MAND, err) < 0)
-			return -1;
-		nft_set_attr_set_u32(s, NFT_SET_ATTR_DATA_LEN, data_len);
+				       NFT_XML_MAND, err) == 0)
+			nft_set_attr_set_u32(s, NFT_SET_ATTR_DATA_LEN, data_len);
 
 	}
 
@@ -668,22 +665,51 @@ static int nft_set_snprintf_xml(char *buf, size_t size, struct nft_set *s,
 	int len = size, offset = 0;
 	struct nft_set_elem *elem;
 
-	ret = snprintf(buf, len, "<set><family>%s</family>"
-				  "<table>%s</table>"
-				  "<name>%s</name>"
-				  "<flags>%u</flags>"
-				  "<key_type>%u</key_type>"
-				  "<key_len>%u</key_len>",
-			nft_family2str(s->family), s->table, s->name,
-			s->set_flags, s->key_type, s->key_len);
+	ret = snprintf(buf, len, "<set>");
 	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
 
-	if (s->flags & (1 << NFT_SET_ATTR_DATA_TYPE) &&
-	    s->flags & (1 << NFT_SET_ATTR_DATA_LEN)) {
-		ret = snprintf(buf+offset, len, "<data_type>%u</data_type>"
-			       "<data_len>%u</data_len>",
-			       s->data_type, s->data_len);
+	if (s->flags & (1 << NFT_SET_ATTR_FAMILY)) {
+		ret = snprintf(buf+offset, len, "<family>%s</family>",
+			       nft_family2str(s->family));
+		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+	}
+
+	if (s->flags & (1 << NFT_SET_ATTR_TABLE)) {
+		ret = snprintf(buf+offset, len, "<table>%s</table>",
+			       s->table);
+		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+	}
+
+	if (s->flags & (1 << NFT_SET_ATTR_NAME)) {
+		ret = snprintf(buf+offset, len, "<name>%s</name>",
+			       s->name);
+		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+	}
 
+	if (s->flags & (1 << NFT_SET_ATTR_FLAGS)) {
+		ret = snprintf(buf+offset, len, "<flags>%u</flags>",
+			       s->set_flags);
+		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+	}
+	if (s->flags & (1 << NFT_SET_ATTR_KEY_TYPE)) {
+		ret = snprintf(buf+offset, len, "<key_type>%u</key_type>",
+			       s->key_type);
+		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+	}
+	if (s->flags & (1 << NFT_SET_ATTR_KEY_LEN)) {
+		ret = snprintf(buf+offset, len, "<key_len>%u</key_len>",
+			       s->key_len);
+		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+	}
+
+	if (s->flags & (1 << NFT_SET_ATTR_DATA_TYPE)) {
+		ret = snprintf(buf+offset, len, "<data_type>%u</data_type>",
+			       s->data_type);
+		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+	}
+	if (s->flags & (1 << NFT_SET_ATTR_DATA_LEN)) {
+		ret = snprintf(buf+offset, len, "<data_len>%u</data_len>",
+			       s->data_len);
 		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
 	}
 
diff --git a/src/set_elem.c b/src/set_elem.c
index 9d1dc41..2ccaf3f 100644
--- a/src/set_elem.c
+++ b/src/set_elem.c
@@ -386,15 +386,12 @@ int nft_mxml_set_elem_parse(mxml_node_t *tree, struct nft_set_elem *e,
 
 	if (nft_mxml_num_parse(tree, "flags", MXML_DESCEND_FIRST, BASE_DEC,
 			       &set_elem_flags, NFT_TYPE_U32, NFT_XML_MAND,
-			       err) < 0)
-		return -1;
-	nft_set_elem_attr_set_u32(e, NFT_SET_ELEM_ATTR_FLAGS, set_elem_flags);
+			       err) == 0)
+		nft_set_elem_attr_set_u32(e, NFT_SET_ELEM_ATTR_FLAGS, set_elem_flags);
 
 	if (nft_mxml_data_reg_parse(tree, "key", &e->key,
-				    NFT_XML_MAND, err) != DATA_VALUE)
-		return -1;
-
-	e->flags |= (1 << NFT_SET_ELEM_ATTR_KEY);
+				    NFT_XML_MAND, err) == DATA_VALUE)
+		e->flags |= (1 << NFT_SET_ELEM_ATTR_KEY);
 
 	/* <set_elem_data> is not mandatory */
 	set_elem_data = nft_mxml_data_reg_parse(tree, "data",
@@ -563,17 +560,26 @@ static int nft_set_elem_snprintf_xml(char *buf, size_t size,
 {
 	int ret, len = size, offset = 0, type = DATA_NONE;
 
-	ret = snprintf(buf, size, "<set_elem>"
-				"<flags>%u</flags><key>",
-				e->set_elem_flags);
+	ret = snprintf(buf, size, "<set_elem>");
 	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
 
-	ret = nft_data_reg_snprintf(buf+offset, len, &e->key,
-				    NFT_OUTPUT_XML, flags, DATA_VALUE);
-	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+	if (e->flags & (1 << NFT_SET_ELEM_ATTR_FLAGS)) {
+		ret = snprintf(buf, size, "<flags>%u</flags>",
+			       e->set_elem_flags);
+		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+	}
 
-	ret = snprintf(buf+offset, len, "</key>");
-	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+	if (e->flags & (1 << NFT_SET_ELEM_ATTR_KEY)) {
+		ret = snprintf(buf+offset, len, "<key>");
+		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+
+		ret = nft_data_reg_snprintf(buf+offset, len, &e->key,
+					    NFT_OUTPUT_XML, flags, DATA_VALUE);
+		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+
+		ret = snprintf(buf+offset, len, "</key>");
+		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+	}
 
 	if (e->flags & (1 << NFT_SET_ELEM_ATTR_DATA))
 		type = DATA_VALUE;
-- 
2.0.0

Re: [libnftnl PATCH 2/5] src: set: Do not print unset values in xml

[Arturo Borrero Gonzalez]

Hi Ana,

a small thing below,

On 11 June 2014 17:50, Ana Rey <anarey@gmail.com> wrote:
[...]
> @@ -563,17 +560,26 @@ static int nft_set_elem_snprintf_xml(char *buf, size_t size,
>  {
>         int ret, len = size, offset = 0, type = DATA_NONE;
>
> -       ret = snprintf(buf, size, "<set_elem>"
> -                               "<flags>%u</flags><key>",
> -                               e->set_elem_flags);
> +       ret = snprintf(buf, size, "<set_elem>");
>         SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
>
> -       ret = nft_data_reg_snprintf(buf+offset, len, &e->key,
> -                                   NFT_OUTPUT_XML, flags, DATA_VALUE);
> -       SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
> +       if (e->flags & (1 << NFT_SET_ELEM_ATTR_FLAGS)) {
> +               ret = snprintf(buf, size, "<flags>%u</flags>",
> +                              e->set_elem_flags);

I think this snprintf call may overwrite the buffer, so you should use
snprintf(buf+offset, len, ...)

Maybe we need more testfiles, to cover all combinations of optionals
node-elements.

regards.
-- 
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: [libnftnl PATCH 2/5] src: set: Do not print unset values in xml

[Pablo Neira Ayuso]

On Wed, Jun 11, 2014 at 06:14:40PM +0200, Arturo Borrero Gonzalez wrote:
> Hi Ana,
> 
> a small thing below,
> 
> On 11 June 2014 17:50, Ana Rey <anarey@gmail.com> wrote:
> [...]
> > @@ -563,17 +560,26 @@ static int nft_set_elem_snprintf_xml(char *buf, size_t size,
> >  {
> >         int ret, len = size, offset = 0, type = DATA_NONE;
> >
> > -       ret = snprintf(buf, size, "<set_elem>"
> > -                               "<flags>%u</flags><key>",
> > -                               e->set_elem_flags);
> > +       ret = snprintf(buf, size, "<set_elem>");
> >         SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
> >
> > -       ret = nft_data_reg_snprintf(buf+offset, len, &e->key,
> > -                                   NFT_OUTPUT_XML, flags, DATA_VALUE);
> > -       SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
> > +       if (e->flags & (1 << NFT_SET_ELEM_ATTR_FLAGS)) {
> > +               ret = snprintf(buf, size, "<flags>%u</flags>",
> > +                              e->set_elem_flags);
> 
> I think this snprintf call may overwrite the buffer, so you should use
> snprintf(buf+offset, len, ...)

Thanks for reviewing Arturo.

I have fixed this here and applied.

[libnftnl PATCH 3/5] src: set: Do not print unset values in json

[Ana Rey]

It changes the parse and the snprint functions to omit unset values.

This json file is gotten for a set:
{
  "set": {
    "name": "mi6set3",
    "table": "test6",
    "flags": "0",
    "family": "unknown",
    "key_type": "0",
    "key_len": "0",
    "set_elem": [
      {
        "flags": "0",
        "key": {
          "data_reg": {
            "type": "value",
            "len": "16",
            "data0": "0x000080fe",
            "data1": "0x00000000",
            "data2": "0xffb30202",
            "data3": "0x89001efe"

Now, This json file is gotten for a set without unset elements.
{
  "set": {
    "name": "mi6set3",
    "table": "test6",
    "family": "unknown",
    "set_elem": [
      {
        "key": {
          "data_reg": {
            "type": "value",
            "len": "16",
            "data0": "0x000080fe",
            "data1": "0x00000000",
            "data2": "0xffb30202",
            "data3": "0x89001efe"


Signed-off-by: Ana Rey <anarey@gmail.com>
---
 src/jansson.c  | 12 +++------
 src/set.c      | 79 ++++++++++++++++++++++++++++++++++++++--------------------
 src/set_elem.c |  8 +++---
 3 files changed, 61 insertions(+), 38 deletions(-)

diff --git a/src/jansson.c b/src/jansson.c
index affe8fe..8805044 100644
--- a/src/jansson.c
+++ b/src/jansson.c
@@ -243,15 +243,11 @@ int nft_jansson_set_elem_parse(struct nft_set_elem *e, json_t *root,
 	uint32_t uval32;
 	int set_elem_data;
 
-	if (nft_jansson_parse_val(root, "flags", NFT_TYPE_U32, &uval32, err) < 0)
-		return -1;
-
-	nft_set_elem_attr_set_u32(e, NFT_SET_ELEM_ATTR_FLAGS, uval32);
-
-	if (nft_jansson_data_reg_parse(root, "key", &e->key, err) != DATA_VALUE)
-		return -1;
+	if (nft_jansson_parse_val(root, "flags", NFT_TYPE_U32, &uval32, err) == 0)
+		nft_set_elem_attr_set_u32(e, NFT_SET_ELEM_ATTR_FLAGS, uval32);
 
-	e->flags |= (1 << NFT_SET_ELEM_ATTR_KEY);
+	if (nft_jansson_data_reg_parse(root, "key", &e->key, err) == DATA_VALUE)
+		e->flags |= (1 << NFT_SET_ELEM_ATTR_KEY);
 
 	if (nft_jansson_node_exist(root, "data")) {
 		set_elem_data = nft_jansson_data_reg_parse(root, "data",
diff --git a/src/set.c b/src/set.c
index e328288..152b81c 100644
--- a/src/set.c
+++ b/src/set.c
@@ -358,29 +358,21 @@ int nft_jansson_parse_set(struct nft_set *s, json_t *tree,
 	if (valstr == NULL)
 		return -1;
 
-	nft_set_attr_set_str(s, NFT_SET_ATTR_TABLE, valstr);
-
-	if (nft_jansson_parse_val(root, "flags", NFT_TYPE_U32, &uval32, err) < 0)
-		return -1;
-
-	nft_set_attr_set_u32(s, NFT_SET_ATTR_FLAGS, uval32);
+	if (nft_jansson_parse_family(root, &family, err) == 0)
+		nft_set_attr_set_u32(s, NFT_SET_ATTR_FAMILY, family);
 
-	if (nft_jansson_parse_family(root, &family, err) < 0)
-		return -1;
+	nft_set_attr_set_str(s, NFT_SET_ATTR_TABLE, valstr);
 
-	nft_set_attr_set_u32(s, NFT_SET_ATTR_FAMILY, family);
+	if (nft_jansson_parse_val(root, "flags", NFT_TYPE_U32, &uval32, err) == 0)
+		nft_set_attr_set_u32(s, NFT_SET_ATTR_FLAGS, uval32);
 
 	if (nft_jansson_parse_val(root, "key_type", NFT_TYPE_U32, &uval32,
-				  err) < 0)
-		return -1;
-
-	nft_set_attr_set_u32(s, NFT_SET_ATTR_KEY_TYPE, uval32);
+				  err) == 0)
+		nft_set_attr_set_u32(s, NFT_SET_ATTR_KEY_TYPE, uval32);
 
 	if (nft_jansson_parse_val(root, "key_len", NFT_TYPE_U32, &uval32,
-				  err) < 0)
-		return -1;
-
-	nft_set_attr_set_u32(s, NFT_SET_ATTR_KEY_LEN, uval32);
+				  err) == 0)
+		nft_set_attr_set_u32(s, NFT_SET_ATTR_KEY_LEN, uval32);
 
 	if (nft_jansson_node_exist(root, "data_type")) {
 		if (nft_jansson_parse_val(root, "data_type", NFT_TYPE_U32,
@@ -584,19 +576,52 @@ static int nft_set_snprintf_json(char *buf, size_t size, struct nft_set *s,
 	int len = size, offset = 0, ret;
 	struct nft_set_elem *elem;
 
-	ret = snprintf(buf, len, "{\"set\":{\"name\":\"%s\","
-				  "\"table\":\"%s\","
-				  "\"flags\":%u,\"family\":\"%s\","
-				  "\"key_type\":%u,\"key_len\":%u",
-			s->name, s->table, s->set_flags,
-			nft_family2str(s->family), s->key_type, s->key_len);
+	ret = snprintf(buf, len, "{\"set\":{");
 	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
 
-	if(s->flags & (1 << NFT_SET_ATTR_DATA_TYPE) &&
-	   s->flags & (1 << NFT_SET_ATTR_DATA_LEN)){
+	if (s->flags & (1 << NFT_SET_ATTR_NAME)) {
+		ret = snprintf(buf+offset, len, "\"name\":\"%s\"",
+			       s->name);
+		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+	} else
+		return -1;
+	if (s->flags & (1 << NFT_SET_ATTR_TABLE)) {
+		ret = snprintf(buf+offset, len, ",\"table\":\"%s\"",
+			       s->table);
+		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+	} else
+		return -1;
+
+	if (s->flags & (1 << NFT_SET_ATTR_FLAGS)) {
+		ret = snprintf(buf+offset, len, ",\"flags\":%u",
+			       s->set_flags);
+		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+	}
+	if (s->flags & (1 << NFT_SET_ATTR_FAMILY)) {
+		ret = snprintf(buf+offset, len, ",\"family\":\"%s\"",
+			       nft_family2str(s->family));
+		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+	} else
+		return -1;
+	if (s->flags & (1 << NFT_SET_ATTR_KEY_TYPE)) {
+		ret = snprintf(buf+offset, len, ",\"key_type\":%u",
+			       s->key_type);
+		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+	}
+	if (s->flags & (1 << NFT_SET_ATTR_KEY_LEN)) {
+		ret = snprintf(buf+offset, len, ",\"key_len\":%u",
+			       s->key_len);
+		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+	}
+
+	if(s->flags & (1 << NFT_SET_ATTR_DATA_TYPE)) {
 		ret = snprintf(buf+offset, len,
-				  ",\"data_type\":%u,\"data_len\":%u",
-			s->data_type, s->data_len);
+				  ",\"data_type\":%u", s->data_type);
+		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+	}
+
+	if(s->flags & (1 << NFT_SET_ATTR_DATA_LEN)) {
+		ret = snprintf(buf+offset, len, ",\"data_len\":%u", s->data_len);
 		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
 	}
 
diff --git a/src/set_elem.c b/src/set_elem.c
index 2ccaf3f..09f6fc5 100644
--- a/src/set_elem.c
+++ b/src/set_elem.c
@@ -493,10 +493,12 @@ static int nft_set_elem_snprintf_json(char *buf, size_t size,
 {
 	int ret, len = size, offset = 0, type = -1;
 
-	ret = snprintf(buf, len, "\"flags\":%u", e->set_elem_flags);
-	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+	if (e->flags & (1 << NFT_SET_ELEM_ATTR_FLAGS)) {
+		ret = snprintf(buf, len, "\"flags\":%u,", e->set_elem_flags);
+		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+	}
 
-	ret = snprintf(buf+offset, len, ",\"key\":{");
+	ret = snprintf(buf+offset, len, "\"key\":{");
 	SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
 
 	ret = nft_data_reg_snprintf(buf+offset, len, &e->key,
-- 
2.0.0

Re: [libnftnl PATCH 3/5] src: set: Do not print unset values in json

[Pablo Neira Ayuso]

On Wed, Jun 11, 2014 at 05:50:49PM +0200, Ana Rey wrote:
> It changes the parse and the snprint functions to omit unset values.
> 
> This json file is gotten for a set:
> {
>   "set": {
>     "name": "mi6set3",
>     "table": "test6",
>     "flags": "0",
>     "family": "unknown",
>     "key_type": "0",
>     "key_len": "0",
>     "set_elem": [
>       {
>         "flags": "0",
>         "key": {
>           "data_reg": {
>             "type": "value",
>             "len": "16",
>             "data0": "0x000080fe",
>             "data1": "0x00000000",
>             "data2": "0xffb30202",
>             "data3": "0x89001efe"
> 
> Now, This json file is gotten for a set without unset elements.
> {
>   "set": {
>     "name": "mi6set3",
>     "table": "test6",
>     "family": "unknown",
>     "set_elem": [
>       {
>         "key": {
>           "data_reg": {
>             "type": "value",
>             "len": "16",
>             "data0": "0x000080fe",
>             "data1": "0x00000000",
>             "data2": "0xffb30202",
>             "data3": "0x89001efe"

I have included a note that says that this is the output when printing
set elements (where the key and key length are not available).

Applied, thanks.

[libnftnl PATCH 4/5] src: set: Free memory in the same function that is reserved.

[Ana Rey]

Free memory in the same function that is reserved.

Signed-off-by: Ana Rey <anarey@gmail.com>
---
 src/set.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/src/set.c b/src/set.c
index 152b81c..53e5db1 100644
--- a/src/set.c
+++ b/src/set.c
@@ -377,7 +377,7 @@ int nft_jansson_parse_set(struct nft_set *s, json_t *tree,
 	if (nft_jansson_node_exist(root, "data_type")) {
 		if (nft_jansson_parse_val(root, "data_type", NFT_TYPE_U32,
 					  &uval32, err) < 0)
-			goto err;
+			return -1;
 
 		nft_set_attr_set_u32(s, NFT_SET_ATTR_DATA_TYPE, uval32);
 	}
@@ -385,7 +385,7 @@ int nft_jansson_parse_set(struct nft_set *s, json_t *tree,
 	if (nft_jansson_node_exist(root, "data_len")) {
 		if (nft_jansson_parse_val(root, "data_len", NFT_TYPE_U32,
 					  &uval32, err) < 0)
-			goto err;
+			return -1;
 
 		nft_set_attr_set_u32(s, NFT_SET_ATTR_DATA_LEN, uval32);
 	}
@@ -395,27 +395,22 @@ int nft_jansson_parse_set(struct nft_set *s, json_t *tree,
 		for (i = 0; i < json_array_size(array); i++) {
 			elem = nft_set_elem_alloc();
 			if (elem == NULL)
-				goto err;
+				return -1;
 
 			json_elem = json_array_get(array, i);
 			if (json_elem == NULL)
-				goto err;
+				return -1;
 
 			if (nft_jansson_set_elem_parse(elem,
 						       json_elem, err) < 0)
-				goto err;
+				return -1;
 
 			list_add_tail(&elem->head, &s->element_list);
 		}
 
 	}
 
-	nft_jansson_free_root(tree);
 	return 0;
-err:
-	nft_jansson_free_root(tree);
-	return -1;
-
 }
 #endif
 
@@ -426,12 +421,16 @@ static int nft_set_json_parse(struct nft_set *s, const void *json,
 #ifdef JSON_PARSING
 	json_t *tree;
 	json_error_t error;
+	int ret;
 
 	tree = nft_jansson_create_root(json, &error, err, input);
 	if (tree == NULL)
 		return -1;
 
-	return nft_jansson_parse_set(s, tree, err);
+	ret = nft_jansson_parse_set(s, tree, err);
+	nft_jansson_free_root(tree);
+
+	return ret;
 #else
 	errno = EOPNOTSUPP;
 	return -1;
-- 
2.0.0

Re: [libnftnl PATCH 4/5] src: set: Free memory in the same function that is reserved.

[Pablo Neira Ayuso]

On Wed, Jun 11, 2014 at 05:50:50PM +0200, Ana Rey wrote:
> Free memory in the same function that is reserved.

Applied, thanks.

[libnftnl PATCH 5/5] tests: xmlfile: Test files without unset child elements in set elements

[Ana Rey]

Test files without unset child element in set elements.

Signed-off-by: Ana Rey <anarey@gmail.com>
---
 tests/xmlfiles/73-set.xml     | 2 +-
 tests/xmlfiles/74-set.xml     | 2 +-
 tests/xmlfiles/75-ruleset.xml | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/xmlfiles/73-set.xml b/tests/xmlfiles/73-set.xml
index 6a9323a..1af61fa 100644
--- a/tests/xmlfiles/73-set.xml
+++ b/tests/xmlfiles/73-set.xml
@@ -1 +1 @@
-<nftables><set><family>ip</family><table>filter</table><name>set0</name><flags>0</flags><key_type>0</key_type><key_len>0</key_len><data_type>0</data_type><data_len>0</data_len><set_elem><flags>0</flags><key><data_reg type="value"><len>4</len><data0>0x0300a8c0</data0></data_reg></key></set_elem><set_elem><flags>0</flags><key><data_reg type="value"><len>4</len><data0>0x0200a8c0</data0></data_reg></key></set_elem><set_elem><flags>0</flags><key><data_reg type="value"><len>4</len><data0>0x0100a8c0</data0></data_reg></key></set_elem></set></nftables>
+<nftables><set><family>ip</family><table>filter</table><name>set0</name><key_type>0</key_type><key_len>0</key_len><data_type>0</data_type><data_len>0</data_len><set_elem><key><data_reg type="value"><len>4</len><data0>0x0300a8c0</data0></data_reg></key></set_elem><set_elem><key><data_reg type="value"><len>4</len><data0>0x0200a8c0</data0></data_reg></key></set_elem><set_elem><key><data_reg type="value"><len>4</len><data0>0x0100a8c0</data0></data_reg></key></set_elem></set></nftables>
diff --git a/tests/xmlfiles/74-set.xml b/tests/xmlfiles/74-set.xml
index 5f0e7ae..a8cabaf 100644
--- a/tests/xmlfiles/74-set.xml
+++ b/tests/xmlfiles/74-set.xml
@@ -1 +1 @@
-<nftables><set><family>ip6</family><table>filter</table><name>set0</name><flags>0</flags><key_type>0</key_type><key_len>0</key_len><data_type>0</data_type><data_len>0</data_len><set_elem><flags>0</flags><key><data_reg type="value"><len>16</len><data0>0xc09a002a</data0><data1>0x2700cac1</data1><data2>0x00000000</data2><data3>0x70010000</data3></data_reg></key></set_elem><set_elem><flags>0</flags><key><data_reg type="value"><len>16</len><data0>0xc09a002a</data0><data1>0x2700cac1</data1><data2>0x00000000</data2><data3>0x50010000</data3></data_reg></key></set_elem></set></nftables>
+<nftables><set><family>ip6</family><table>filter</table><name>set0</name><key_type>0</key_type><key_len>0</key_len><data_type>0</data_type><data_len>0</data_len><set_elem><key><data_reg type="value"><len>16</len><data0>0xc09a002a</data0><data1>0x2700cac1</data1><data2>0x00000000</data2><data3>0x70010000</data3></data_reg></key></set_elem><set_elem><key><data_reg type="value"><len>16</len><data0>0xc09a002a</data0><data1>0x2700cac1</data1><data2>0x00000000</data2><data3>0x50010000</data3></data_reg></key></set_elem></set></nftables>
diff --git a/tests/xmlfiles/75-ruleset.xml b/tests/xmlfiles/75-ruleset.xml
index e4c8389..0eb4075 100644
--- a/tests/xmlfiles/75-ruleset.xml
+++ b/tests/xmlfiles/75-ruleset.xml
@@ -1 +1 @@
-<nftables><table><name>filter</name><family>ip</family><flags>0</flags><use>0</use></table><table><name>filter</name><family>ip6</family><flags>0</flags><use>0</use></table><chain><name>input</name><handle>1</handle><bytes>0</bytes><packets>0</packets><table>filter</table><family>ip</family></chain><chain><name>output</name><handle>2</handle><bytes>0</bytes><packets>0</packets><table>filter</table><family>ip</family></chain><chain><name>forward</name><handle>1</handle><bytes>0</bytes><packets>0</packets><table>filter</table><family>ip6</family></chain><set><family>ip6</family><table>filter</table><name>set0</name><flags>3</flags><key_type>12</key_type><key_len>2</key_len><data_type>0</data_type><data_len>0</data_len><set_elem><flags>0</flags><key><data_reg type="value"><len>2</len><data0>
 0x00004300</data0></data_reg></key></set_elem><set_elem><flags>0</flags><key><data_reg type="value"><len>2</len><data0>0x00003500</data0></data_reg></key></set_elem></set><set><family>ip</family><table>filter</table><name>map0</name><flags>11</flags><key_type>12</key_type><key_len>2</key_len><data_type>4294967040</data_type><data_len>16</data_len><set_elem><flags>0</flags><key><data_reg type="value"><len>2</len><data0>0x00005000</data0></data_reg></key><data><data_reg type="verdict"><verdict>drop</verdict></data_reg></data></set_elem><set_elem><flags>0</flags><key><data_reg type="value"><len>2</len><data0>0x00001600</data0></data_reg></key><data><data_reg type="verdict"><verdict>accept</verdict></data_reg></data></set_elem></set><rule><family>ip</family><table>filter</table><chain>input<
 /chain><handle>8</handle><expr type="counter"><pkts>0</pkts><bytes>0</bytes></expr><expr type="immediate"><dreg>0</dreg><data_reg type="verdict"><verdict>accept</verdict></data_reg></expr></rule><rule><family>ip</family><table>filter</table><chain>output</chain><handle>9</handle><expr type="payload"><dreg>1</dreg><offset>9</offset><len>1</len><base>network</base></expr><expr type="cmp"><sreg>1</sreg><op>eq</op><data_reg type="value"><len>1</len><data0>0x00000006</data0></data_reg></expr><expr type="payload"><dreg>1</dreg><offset>2</offset><len>2</len><base>transport</base></expr><expr type="lookup"><set>map0</set><sreg>1</sreg><dreg>0</dreg></expr></rule><rule><family>ip6</family><table>filter</table><chain>forward</chain><handle>2</handle><expr type="payload"><dreg>1</dreg><offset>6</of
 fset><len>1</len><base>network</base></expr><expr type="cmp"><sreg>1</sreg><op>eq</op><data_reg type="value"><len>1</len><data0>0x00000011</data0></data_reg></expr><expr type="payload"><dreg>1</dreg><offset>2</offset><len>2</len><base>transport</base></expr><expr type="lookup"><set>set0</set><sreg>1</sreg><dreg>0</dreg></expr><expr type="counter"><pkts>0</pkts><bytes>0</bytes></expr><expr type="immediate"><dreg>0</dreg><data_reg type="verdict"><verdict>accept</verdict></data_reg></expr></rule></nftables>
+<nftables><table><name>filter</name><family>ip</family><flags>0</flags><use>0</use></table><table><name>filter</name><family>ip6</family><flags>0</flags><use>0</use></table><chain><name>input</name><handle>1</handle><bytes>0</bytes><packets>0</packets><table>filter</table><family>ip</family></chain><chain><name>output</name><handle>2</handle><bytes>0</bytes><packets>0</packets><table>filter</table><family>ip</family></chain><chain><name>forward</name><handle>1</handle><bytes>0</bytes><packets>0</packets><table>filter</table><family>ip6</family></chain><set><family>ip6</family><table>filter</table><name>set0</name><flags>3</flags><key_type>12</key_type><key_len>2</key_len><data_type>0</data_type><data_len>0</data_len><set_elem><key><data_reg type="value"><len>2</len><data0>0x00004300</data
 0></data_reg></key></set_elem><set_elem><key><data_reg type="value"><len>2</len><data0>0x00003500</data0></data_reg></key></set_elem></set><set><family>ip</family><table>filter</table><name>map0</name><flags>11</flags><key_type>12</key_type><key_len>2</key_len><data_type>4294967040</data_type><data_len>16</data_len><set_elem><key><data_reg type="value"><len>2</len><data0>0x00005000</data0></data_reg></key><data><data_reg type="verdict"><verdict>drop</verdict></data_reg></data></set_elem><set_elem><key><data_reg type="value"><len>2</len><data0>0x00001600</data0></data_reg></key><data><data_reg type="verdict"><verdict>accept</verdict></data_reg></data></set_elem></set><rule><family>ip</family><table>filter</table><chain>input</chain><handle>8</handle><expr type="counter"><pkts>0</pkts><byt
 es>0</bytes></expr><expr type="immediate"><dreg>0</dreg><data_reg type="verdict"><verdict>accept</verdict></data_reg></expr></rule><rule><family>ip</family><table>filter</table><chain>output</chain><handle>9</handle><expr type="payload"><dreg>1</dreg><offset>9</offset><len>1</len><base>network</base></expr><expr type="cmp"><sreg>1</sreg><op>eq</op><data_reg type="value"><len>1</len><data0>0x00000006</data0></data_reg></expr><expr type="payload"><dreg>1</dreg><offset>2</offset><len>2</len><base>transport</base></expr><expr type="lookup"><set>map0</set><sreg>1</sreg><dreg>0</dreg></expr></rule><rule><family>ip6</family><table>filter</table><chain>forward</chain><handle>2</handle><expr type="payload"><dreg>1</dreg><offset>6</offset><len>1</len><base>network</base></expr><expr type="cmp"><sr
 eg>1</sreg><op>eq</op><data_reg type="value"><len>1</len><data0>0x00000011</data0></data_reg></expr><expr type="payload"><dreg>1</dreg><offset>2</offset><len>2</len><base>transport</base></expr><expr type="lookup"><set>set0</set><sreg>1</sreg><dreg>0</dreg></expr><expr type="counter"><pkts>0</pkts><bytes>0</bytes></expr><expr type="immediate"><dreg>0</dreg><data_reg type="verdict"><verdict>accept</verdict></data_reg></expr></rule></nftables>
-- 
2.0.0

Re: [libnftnl PATCH 5/5] tests: xmlfile: Test files without unset child elements in set elements

[Pablo Neira Ayuso]

On Wed, Jun 11, 2014 at 05:50:51PM +0200, Ana Rey wrote:
> Test files without unset child element in set elements.

Applied, thanks Ana.