[PATCH 1/3 libnftnl] chain: Fix bug. Check correct attribute

[PATCH 1/3 libnftnl] chain: Fix bug. Check correct attribute

[Carlos Falgueras García]

Signed-off-by: Carlos Falgueras García <carlosfg@riseup.net>
---
 src/chain.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/chain.c b/src/chain.c
index f55dfa1..bfffbe0 100644
--- a/src/chain.c
+++ b/src/chain.c
@@ -219,7 +219,7 @@ int nftnl_chain_set_data(struct nftnl_chain *c, uint16_t attr,
 			xfree(c->dev);
 
 		c->dev = strdup(data);
-		if (!c->type)
+		if (!c->dev)
 			return -1;
 		break;
 	}
-- 
2.8.3

--
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

[PATCH 2/3 libnftnl] fix some error checking in parser functions

[Carlos Falgueras García]

Use a variable 'ret' multiple times without treat the error between can
overwrite the previous error value, and may execute code which should not.

Bad way:
	int f() {
		int ret;

		ret = g();
		ret = h();

		return ret;
	}

Good way:
	int f() {
		int ret;

		ret = g();
		if (ret)
			return ret;

		ret = h();
		if (ret)
			return ret;

		return 0;
	}

Signed-off-by: Carlos Falgueras García <carlosfg@riseup.net>
---
 src/rule.c     | 14 ++++++++++----
 src/set.c      |  9 ++++++---
 src/set_elem.c | 41 ++++++++++++++++++++++++-----------------
 3 files changed, 40 insertions(+), 24 deletions(-)

diff --git a/src/rule.c b/src/rule.c
index b009c37..930ecc0 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -427,7 +427,7 @@ int nftnl_rule_nlmsg_parse(const struct nlmsghdr *nlh, struct nftnl_rule *r)
 {
 	struct nlattr *tb[NFTA_RULE_MAX+1] = {};
 	struct nfgenmsg *nfg = mnl_nlmsg_get_payload(nlh);
-	int ret = 0;
+	int ret;
 
 	if (mnl_attr_parse(nlh, sizeof(*nfg), nftnl_rule_parse_attr_cb, tb) < 0)
 		return -1;
@@ -452,10 +452,16 @@ int nftnl_rule_nlmsg_parse(const struct nlmsghdr *nlh, struct nftnl_rule *r)
 		r->handle = be64toh(mnl_attr_get_u64(tb[NFTA_RULE_HANDLE]));
 		r->flags |= (1 << NFTNL_RULE_HANDLE);
 	}
-	if (tb[NFTA_RULE_EXPRESSIONS])
+	if (tb[NFTA_RULE_EXPRESSIONS]) {
 		ret = nftnl_rule_parse_expr(tb[NFTA_RULE_EXPRESSIONS], r);
-	if (tb[NFTA_RULE_COMPAT])
+		if (ret)
+			return ret;
+	}
+	if (tb[NFTA_RULE_COMPAT]) {
 		ret = nftnl_rule_parse_compat(tb[NFTA_RULE_COMPAT], r);
+		if (ret)
+			return ret;
+	}
 	if (tb[NFTA_RULE_POSITION]) {
 		r->position = be64toh(mnl_attr_get_u64(tb[NFTA_RULE_POSITION]));
 		r->flags |= (1 << NFTNL_RULE_POSITION);
@@ -480,7 +486,7 @@ int nftnl_rule_nlmsg_parse(const struct nlmsghdr *nlh, struct nftnl_rule *r)
 	r->family = nfg->nfgen_family;
 	r->flags |= (1 << NFTNL_RULE_FAMILY);
 
-	return ret;
+	return 0;
 }
 EXPORT_SYMBOL_ALIAS(nftnl_rule_nlmsg_parse, nft_rule_nlmsg_parse);
 
diff --git a/src/set.c b/src/set.c
index 08d5797..e1691c7 100644
--- a/src/set.c
+++ b/src/set.c
@@ -433,7 +433,7 @@ int nftnl_set_nlmsg_parse(const struct nlmsghdr *nlh, struct nftnl_set *s)
 {
 	struct nlattr *tb[NFTA_SET_MAX+1] = {};
 	struct nfgenmsg *nfg = mnl_nlmsg_get_payload(nlh);
-	int ret = 0;
+	int ret;
 
 	if (mnl_attr_parse(nlh, sizeof(*nfg), nftnl_set_parse_attr_cb, tb) < 0)
 		return -1;
@@ -490,13 +490,16 @@ int nftnl_set_nlmsg_parse(const struct nlmsghdr *nlh, struct nftnl_set *s)
 		s->gc_interval = ntohl(mnl_attr_get_u32(tb[NFTA_SET_GC_INTERVAL]));
 		s->flags |= (1 << NFTNL_SET_GC_INTERVAL);
 	}
-	if (tb[NFTA_SET_DESC])
+	if (tb[NFTA_SET_DESC]) {
 		ret = nftnl_set_desc_parse(s, tb[NFTA_SET_DESC]);
+		if (ret)
+			return ret;
+	}
 
 	s->family = nfg->nfgen_family;
 	s->flags |= (1 << NFTNL_SET_FAMILY);
 
-	return ret;
+	return 0;
 }
 EXPORT_SYMBOL_ALIAS(nftnl_set_nlmsg_parse, nft_set_nlmsg_parse);
 
diff --git a/src/set_elem.c b/src/set_elem.c
index 8cceeae..e4dd313 100644
--- a/src/set_elem.c
+++ b/src/set_elem.c
@@ -345,16 +345,15 @@ static int nftnl_set_elems_parse2(struct nftnl_set *s, const struct nlattr *nest
 {
 	struct nlattr *tb[NFTA_SET_ELEM_MAX+1] = {};
 	struct nftnl_set_elem *e;
-	int ret = 0, type;
+	int ret, type;
 
 	e = nftnl_set_elem_alloc();
 	if (e == NULL)
 		return -1;
 
-	if (mnl_attr_parse_nested(nest, nftnl_set_elem_parse_attr_cb, tb) < 0) {
-		nftnl_set_elem_free(e);
-		return -1;
-	}
+	ret = mnl_attr_parse_nested(nest, nftnl_set_elem_parse_attr_cb, tb);
+	if (ret)
+		goto out_set_elem;
 
 	if (tb[NFTA_SET_ELEM_FLAGS]) {
 		e->set_elem_flags =
@@ -371,10 +370,14 @@ static int nftnl_set_elems_parse2(struct nftnl_set *s, const struct nlattr *nest
 	}
         if (tb[NFTA_SET_ELEM_KEY]) {
 		ret = nftnl_parse_data(&e->key, tb[NFTA_SET_ELEM_KEY], &type);
+		if (ret)
+			goto out_set_elem;
 		e->flags |= (1 << NFTNL_SET_ELEM_KEY);
         }
         if (tb[NFTA_SET_ELEM_DATA]) {
 		ret = nftnl_parse_data(&e->data, tb[NFTA_SET_ELEM_DATA], &type);
+		if (ret)
+			goto out_set_elem;
 		switch(type) {
 		case DATA_VERDICT:
 			e->flags |= (1 << NFTNL_SET_ELEM_VERDICT);
@@ -391,7 +394,7 @@ static int nftnl_set_elems_parse2(struct nftnl_set *s, const struct nlattr *nest
 	if (tb[NFTA_SET_ELEM_EXPR]) {
 		e->expr = nftnl_expr_parse(tb[NFTA_SET_ELEM_EXPR]);
 		if (e->expr == NULL)
-			goto err;
+			goto out_set_elem;
 		e->flags |= (1 << NFTNL_SET_ELEM_EXPR);
 	}
 	if (tb[NFTA_SET_ELEM_USERDATA]) {
@@ -404,20 +407,19 @@ static int nftnl_set_elems_parse2(struct nftnl_set *s, const struct nlattr *nest
 		e->user.len  = mnl_attr_get_payload_len(tb[NFTA_SET_ELEM_USERDATA]);
 		e->user.data = malloc(e->user.len);
 		if (e->user.data == NULL)
-			goto err;
+			goto out_expr;
 		memcpy(e->user.data, udata, e->user.len);
 		e->flags |= (1 << NFTNL_RULE_USERDATA);
 	}
 
-	if (ret < 0) {
-err:
-		nftnl_set_elem_free(e);
-		return -1;
-	}
-
 	/* Add this new element to this set */
 	list_add_tail(&e->head, &s->element_list);
 
+	return 0;
+out_expr:
+	nftnl_expr_free(e->expr);
+out_set_elem:
+	nftnl_set_elem_free(e);
 	return ret;
 }
 
@@ -449,13 +451,15 @@ nftnl_set_elem_list_parse_attr_cb(const struct nlattr *attr, void *data)
 static int nftnl_set_elems_parse(struct nftnl_set *s, const struct nlattr *nest)
 {
 	struct nlattr *attr;
-	int ret = 0;
+	int ret;
 
 	mnl_attr_for_each_nested(attr, nest) {
 		if (mnl_attr_get_type(attr) != NFTA_LIST_ELEM)
 			return -1;
 
 		ret = nftnl_set_elems_parse2(s, attr);
+		if (ret)
+			return ret;
 	}
 	return ret;
 }
@@ -464,7 +468,7 @@ int nftnl_set_elems_nlmsg_parse(const struct nlmsghdr *nlh, struct nftnl_set *s)
 {
 	struct nlattr *tb[NFTA_SET_ELEM_LIST_MAX+1] = {};
 	struct nfgenmsg *nfg = mnl_nlmsg_get_payload(nlh);
-	int ret = 0;
+	int ret;
 
 	if (mnl_attr_parse(nlh, sizeof(*nfg),
 			   nftnl_set_elem_list_parse_attr_cb, tb) < 0)
@@ -492,13 +496,16 @@ int nftnl_set_elems_nlmsg_parse(const struct nlmsghdr *nlh, struct nftnl_set *s)
 		s->id = ntohl(mnl_attr_get_u32(tb[NFTA_SET_ELEM_LIST_SET_ID]));
 		s->flags |= (1 << NFTNL_SET_ID);
 	}
-        if (tb[NFTA_SET_ELEM_LIST_ELEMENTS])
+        if (tb[NFTA_SET_ELEM_LIST_ELEMENTS]) {
 	 	ret = nftnl_set_elems_parse(s, tb[NFTA_SET_ELEM_LIST_ELEMENTS]);
+		if (ret)
+			return ret;
+	}
 
 	s->family = nfg->nfgen_family;
 	s->flags |= (1 << NFTNL_SET_FAMILY);
 
-	return ret;
+	return 0;
 }
 EXPORT_SYMBOL_ALIAS(nftnl_set_elems_nlmsg_parse, nft_set_elems_nlmsg_parse);
 
-- 
2.8.3

--
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

[PATCH 3/3 libnftnl] Consolidate setters

[Carlos Falgueras García]

Use setter method in all place where we set an attribute instead of repeat
the code.

Signed-off-by: Carlos Falgueras García <carlosfg@riseup.net>
---
 src/chain.c    | 91 ++++++++++++++++++++++++--------------------------------
 src/gen.c      |  7 ++---
 src/rule.c     | 72 ++++++++++++++++-----------------------------
 src/set.c      | 93 ++++++++++++++++++++++++----------------------------------
 src/set_elem.c | 72 ++++++++++++++++++---------------------------
 src/table.c    | 30 ++++++++-----------
 6 files changed, 148 insertions(+), 217 deletions(-)

diff --git a/src/chain.c b/src/chain.c
index bfffbe0..9c8b436 100644
--- a/src/chain.c
+++ b/src/chain.c
@@ -465,14 +465,12 @@ static int nftnl_chain_parse_counters(struct nlattr *attr, struct nftnl_chain *c
 	if (mnl_attr_parse_nested(attr, nftnl_chain_parse_counters_cb, tb) < 0)
 		return -1;
 
-	if (tb[NFTA_COUNTER_PACKETS]) {
-		c->packets = be64toh(mnl_attr_get_u64(tb[NFTA_COUNTER_PACKETS]));
-		c->flags |= (1 << NFTNL_CHAIN_PACKETS);
-	}
-	if (tb[NFTA_COUNTER_BYTES]) {
-		c->bytes = be64toh(mnl_attr_get_u64(tb[NFTA_COUNTER_BYTES]));
-		c->flags |= (1 << NFTNL_CHAIN_BYTES);
-	}
+	if (tb[NFTA_COUNTER_PACKETS])
+		nftnl_chain_set_u64(c, NFTNL_CHAIN_PACKETS,
+			be64toh(mnl_attr_get_u64(tb[NFTA_COUNTER_PACKETS])));
+	if (tb[NFTA_COUNTER_BYTES])
+		nftnl_chain_set_u64(c, NFTNL_CHAIN_BYTES,
+			be64toh(mnl_attr_get_u64(tb[NFTA_COUNTER_BYTES])));
 
 	return 0;
 }
@@ -504,23 +502,22 @@ static int nftnl_chain_parse_hook_cb(const struct nlattr *attr, void *data)
 static int nftnl_chain_parse_hook(struct nlattr *attr, struct nftnl_chain *c)
 {
 	struct nlattr *tb[NFTA_HOOK_MAX+1] = {};
+	int err;
 
 	if (mnl_attr_parse_nested(attr, nftnl_chain_parse_hook_cb, tb) < 0)
 		return -1;
 
-	if (tb[NFTA_HOOK_HOOKNUM]) {
-		c->hooknum = ntohl(mnl_attr_get_u32(tb[NFTA_HOOK_HOOKNUM]));
-		c->flags |= (1 << NFTNL_CHAIN_HOOKNUM);
-	}
-	if (tb[NFTA_HOOK_PRIORITY]) {
-		c->prio = ntohl(mnl_attr_get_u32(tb[NFTA_HOOK_PRIORITY]));
-		c->flags |= (1 << NFTNL_CHAIN_PRIO);
-	}
+	if (tb[NFTA_HOOK_HOOKNUM])
+		nftnl_chain_set_u32(c, NFTNL_CHAIN_HOOKNUM,
+			ntohl(mnl_attr_get_u32(tb[NFTA_HOOK_HOOKNUM])));
+	if (tb[NFTA_HOOK_PRIORITY])
+		nftnl_chain_set_s32(c, NFTNL_CHAIN_PRIO,
+			ntohl(mnl_attr_get_u32(tb[NFTA_HOOK_PRIORITY])));
 	if (tb[NFTA_HOOK_DEV]) {
-		c->dev = strdup(mnl_attr_get_str(tb[NFTA_HOOK_DEV]));
-		if (!c->dev)
-			return -1;
-		c->flags |= (1 << NFTNL_CHAIN_DEV);
+		err = nftnl_chain_set_str(c, NFTNL_CHAIN_DEV,
+			mnl_attr_get_str(tb[NFTA_HOOK_DEV]));
+		if (err)
+			return err;
 	}
 
 	return 0;
@@ -536,54 +533,44 @@ int nftnl_chain_nlmsg_parse(const struct nlmsghdr *nlh, struct nftnl_chain *c)
 		return -1;
 
 	if (tb[NFTA_CHAIN_NAME]) {
-		if (c->flags & (1 << NFTNL_CHAIN_NAME))
-			xfree(c->name);
-		c->name = strdup(mnl_attr_get_str(tb[NFTA_CHAIN_NAME]));
-		if (!c->name)
-			return -1;
-		c->flags |= (1 << NFTNL_CHAIN_NAME);
+		ret = nftnl_chain_set_str(c, NFTNL_CHAIN_NAME,
+			mnl_attr_get_str(tb[NFTA_CHAIN_NAME]));
+		if (ret)
+			return ret;
 	}
 	if (tb[NFTA_CHAIN_TABLE]) {
-		if (c->flags & (1 << NFTNL_CHAIN_TABLE))
-			xfree(c->table);
-		c->table = strdup(mnl_attr_get_str(tb[NFTA_CHAIN_TABLE]));
-		if (!c->table)
-			return -1;
-		c->flags |= (1 << NFTNL_CHAIN_TABLE);
+		ret = nftnl_chain_set_str(c, NFTNL_CHAIN_TABLE,
+			mnl_attr_get_str(tb[NFTA_CHAIN_TABLE]));
+		if (ret)
+			return ret;
 	}
 	if (tb[NFTA_CHAIN_HOOK]) {
 		ret = nftnl_chain_parse_hook(tb[NFTA_CHAIN_HOOK], c);
 		if (ret < 0)
 			return ret;
 	}
-	if (tb[NFTA_CHAIN_POLICY]) {
-		c->policy = ntohl(mnl_attr_get_u32(tb[NFTA_CHAIN_POLICY]));
-		c->flags |= (1 << NFTNL_CHAIN_POLICY);
-	}
-	if (tb[NFTA_CHAIN_USE]) {
-		c->use = ntohl(mnl_attr_get_u32(tb[NFTA_CHAIN_USE]));
-		c->flags |= (1 << NFTNL_CHAIN_USE);
-	}
+	if (tb[NFTA_CHAIN_POLICY])
+		nftnl_chain_set_u32(c, NFTNL_CHAIN_POLICY,
+			ntohl(mnl_attr_get_u32(tb[NFTA_CHAIN_POLICY])));
+	if (tb[NFTA_CHAIN_USE])
+		nftnl_chain_set_u32(c, NFTNL_CHAIN_USE,
+			ntohl(mnl_attr_get_u32(tb[NFTA_CHAIN_USE])));
 	if (tb[NFTA_CHAIN_COUNTERS]) {
 		ret = nftnl_chain_parse_counters(tb[NFTA_CHAIN_COUNTERS], c);
 		if (ret < 0)
 			return ret;
 	}
-	if (tb[NFTA_CHAIN_HANDLE]) {
-		c->handle = be64toh(mnl_attr_get_u64(tb[NFTA_CHAIN_HANDLE]));
-		c->flags |= (1 << NFTNL_CHAIN_HANDLE);
-	}
+	if (tb[NFTA_CHAIN_HANDLE])
+		nftnl_chain_set_u64(c, NFTNL_CHAIN_HANDLE,
+			be64toh(mnl_attr_get_u64(tb[NFTA_CHAIN_HANDLE])));
 	if (tb[NFTA_CHAIN_TYPE]) {
-		if (c->flags & (1 << NFTNL_CHAIN_TYPE))
-			xfree(c->type);
-		c->type = strdup(mnl_attr_get_str(tb[NFTA_CHAIN_TYPE]));
-		if (!c->type)
-			return -1;
-		c->flags |= (1 << NFTNL_CHAIN_TYPE);
+		ret = nftnl_chain_set_str(c, NFTNL_CHAIN_TYPE,
+			mnl_attr_get_str(tb[NFTA_CHAIN_TYPE]));
+		if (ret)
+			return ret;
 	}
 
-	c->family = nfg->nfgen_family;
-	c->flags |= (1 << NFTNL_CHAIN_FAMILY);
+	nftnl_chain_set_u32(c, NFTNL_CHAIN_FAMILY, nfg->nfgen_family);
 
 	return ret;
 }
diff --git a/src/gen.c b/src/gen.c
index 37a9049..d73b00f 100644
--- a/src/gen.c
+++ b/src/gen.c
@@ -147,10 +147,9 @@ int nftnl_gen_nlmsg_parse(const struct nlmsghdr *nlh, struct nftnl_gen *gen)
 	if (mnl_attr_parse(nlh, sizeof(*nfg), nftnl_gen_parse_attr_cb, tb) < 0)
 		return -1;
 
-	if (tb[NFTA_GEN_ID]) {
-		gen->id = ntohl(mnl_attr_get_u32(tb[NFTA_GEN_ID]));
-		gen->flags |= (1 << NFTNL_GEN_ID);
-	}
+	if (tb[NFTA_GEN_ID])
+		nftnl_gen_set_u32(gen, NFTNL_GEN_ID,
+				ntohl(mnl_attr_get_u32(tb[NFTA_GEN_ID])));
 	return 0;
 }
 EXPORT_SYMBOL_ALIAS(nftnl_gen_nlmsg_parse, nft_gen_nlmsg_parse);
diff --git a/src/rule.c b/src/rule.c
index 930ecc0..fa31357 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -410,16 +410,12 @@ static int nftnl_rule_parse_compat(struct nlattr *nest, struct nftnl_rule *r)
 	if (mnl_attr_parse_nested(nest, nftnl_rule_parse_compat_cb, tb) < 0)
 		return -1;
 
-	if (tb[NFTA_RULE_COMPAT_PROTO]) {
-		r->compat.proto =
-			ntohl(mnl_attr_get_u32(tb[NFTA_RULE_COMPAT_PROTO]));
-		r->flags |= (1 << NFTNL_RULE_COMPAT_PROTO);
-	}
-	if (tb[NFTA_RULE_COMPAT_FLAGS]) {
-		r->compat.flags =
-			ntohl(mnl_attr_get_u32(tb[NFTA_RULE_COMPAT_FLAGS]));
-		r->flags |= (1 << NFTNL_RULE_COMPAT_FLAGS);
-	}
+	if (tb[NFTA_RULE_COMPAT_PROTO])
+		nftnl_rule_set_u32(r, NFTNL_RULE_COMPAT_PROTO,
+			ntohl(mnl_attr_get_u32(tb[NFTA_RULE_COMPAT_PROTO])));
+	if (tb[NFTA_RULE_COMPAT_FLAGS])
+		nftnl_rule_set_u32(r, NFTNL_RULE_COMPAT_FLAGS,
+			ntohl(mnl_attr_get_u32(tb[NFTA_RULE_COMPAT_FLAGS])));
 	return 0;
 }
 
@@ -433,25 +429,20 @@ int nftnl_rule_nlmsg_parse(const struct nlmsghdr *nlh, struct nftnl_rule *r)
 		return -1;
 
 	if (tb[NFTA_RULE_TABLE]) {
-		if (r->flags & (1 << NFTNL_RULE_TABLE))
-			xfree(r->table);
-		r->table = strdup(mnl_attr_get_str(tb[NFTA_RULE_TABLE]));
-		if (!r->table)
-			return -1;
-		r->flags |= (1 << NFTNL_RULE_TABLE);
+		ret = nftnl_rule_set_str(r, NFTNL_RULE_TABLE,
+				mnl_attr_get_str(tb[NFTA_RULE_TABLE]));
+		if (ret)
+			return ret;
 	}
 	if (tb[NFTA_RULE_CHAIN]) {
-		if (r->flags & (1 << NFTNL_RULE_CHAIN))
-			xfree(r->chain);
-		r->chain = strdup(mnl_attr_get_str(tb[NFTA_RULE_CHAIN]));
-		if (!r->chain)
-			return -1;
-		r->flags |= (1 << NFTNL_RULE_CHAIN);
-	}
-	if (tb[NFTA_RULE_HANDLE]) {
-		r->handle = be64toh(mnl_attr_get_u64(tb[NFTA_RULE_HANDLE]));
-		r->flags |= (1 << NFTNL_RULE_HANDLE);
+		ret = nftnl_rule_set_str(r, NFTNL_RULE_CHAIN,
+			mnl_attr_get_str(tb[NFTA_RULE_CHAIN]));
+		if (ret)
+			return ret;
 	}
+	if (tb[NFTA_RULE_HANDLE])
+		nftnl_rule_set_u64(r, NFTNL_RULE_HANDLE,
+			be64toh(mnl_attr_get_u64(tb[NFTA_RULE_HANDLE])));
 	if (tb[NFTA_RULE_EXPRESSIONS]) {
 		ret = nftnl_rule_parse_expr(tb[NFTA_RULE_EXPRESSIONS], r);
 		if (ret)
@@ -462,29 +453,18 @@ int nftnl_rule_nlmsg_parse(const struct nlmsghdr *nlh, struct nftnl_rule *r)
 		if (ret)
 			return ret;
 	}
-	if (tb[NFTA_RULE_POSITION]) {
-		r->position = be64toh(mnl_attr_get_u64(tb[NFTA_RULE_POSITION]));
-		r->flags |= (1 << NFTNL_RULE_POSITION);
-	}
+	if (tb[NFTA_RULE_POSITION])
+		nftnl_rule_set_u64(r, NFTNL_RULE_POSITION,
+			be64toh(mnl_attr_get_u64(tb[NFTA_RULE_POSITION])));
 	if (tb[NFTA_RULE_USERDATA]) {
-		const void *udata =
-			mnl_attr_get_payload(tb[NFTA_RULE_USERDATA]);
-
-		if (r->flags & (1 << NFTNL_RULE_USERDATA))
-			xfree(r->user.data);
-
-		r->user.len = mnl_attr_get_payload_len(tb[NFTA_RULE_USERDATA]);
-
-		r->user.data = malloc(r->user.len);
-		if (r->user.data == NULL)
-			return -1;
-
-		memcpy(r->user.data, udata, r->user.len);
-		r->flags |= (1 << NFTNL_RULE_USERDATA);
+		nftnl_rule_set_data(r, NFTNL_RULE_USERDATA,
+			mnl_attr_get_payload(tb[NFTA_RULE_USERDATA]),
+			mnl_attr_get_payload_len(tb[NFTA_RULE_USERDATA]));
+		if (ret)
+			return ret;
 	}
 
-	r->family = nfg->nfgen_family;
-	r->flags |= (1 << NFTNL_RULE_FAMILY);
+	nftnl_rule_set_u32(r, NFTNL_RULE_FAMILY, nfg->nfgen_family);
 
 	return 0;
 }
diff --git a/src/set.c b/src/set.c
index e1691c7..4042cc0 100644
--- a/src/set.c
+++ b/src/set.c
@@ -421,10 +421,9 @@ static int nftnl_set_desc_parse(struct nftnl_set *s,
 	if (mnl_attr_parse_nested(attr, nftnl_set_desc_parse_attr_cb, tb) < 0)
 		return -1;
 
-	if (tb[NFTA_SET_DESC_SIZE]) {
-		s->desc.size = ntohl(mnl_attr_get_u32(tb[NFTA_SET_DESC_SIZE]));
-		s->flags |= (1 << NFTNL_SET_DESC_SIZE);
-	}
+	if (tb[NFTA_SET_DESC_SIZE])
+		nftnl_set_set_u32(s, NFTNL_SET_DESC_SIZE,
+			ntohl(mnl_attr_get_u32(tb[NFTA_SET_DESC_SIZE])));
 
 	return 0;
 }
@@ -439,65 +438,51 @@ int nftnl_set_nlmsg_parse(const struct nlmsghdr *nlh, struct nftnl_set *s)
 		return -1;
 
 	if (tb[NFTA_SET_TABLE]) {
-		if (s->flags & (1 << NFTNL_SET_TABLE))
-			xfree(s->table);
-		s->table = strdup(mnl_attr_get_str(tb[NFTA_SET_TABLE]));
-		if (!s->table)
-			return -1;
-		s->flags |= (1 << NFTNL_SET_TABLE);
+		ret = nftnl_set_set_str(s, NFTNL_SET_TABLE,
+			mnl_attr_get_str(tb[NFTA_SET_TABLE]));
+		if (ret)
+			return ret;
 	}
 	if (tb[NFTA_SET_NAME]) {
-		if (s->flags & (1 << NFTNL_SET_NAME))
-			xfree(s->name);
-		s->name = strdup(mnl_attr_get_str(tb[NFTA_SET_NAME]));
-		if (!s->name)
-			return -1;
-		s->flags |= (1 << NFTNL_SET_NAME);
-	}
-	if (tb[NFTA_SET_FLAGS]) {
-		s->set_flags = ntohl(mnl_attr_get_u32(tb[NFTA_SET_FLAGS]));
-		s->flags |= (1 << NFTNL_SET_FLAGS);
-	}
-	if (tb[NFTA_SET_KEY_TYPE]) {
-		s->key_type = ntohl(mnl_attr_get_u32(tb[NFTA_SET_KEY_TYPE]));
-		s->flags |= (1 << NFTNL_SET_KEY_TYPE);
-	}
-	if (tb[NFTA_SET_KEY_LEN]) {
-		s->key_len = ntohl(mnl_attr_get_u32(tb[NFTA_SET_KEY_LEN]));
-		s->flags |= (1 << NFTNL_SET_KEY_LEN);
-	}
-	if (tb[NFTA_SET_DATA_TYPE]) {
-		s->data_type = ntohl(mnl_attr_get_u32(tb[NFTA_SET_DATA_TYPE]));
-		s->flags |= (1 << NFTNL_SET_DATA_TYPE);
-	}
-	if (tb[NFTA_SET_DATA_LEN]) {
-		s->data_len = ntohl(mnl_attr_get_u32(tb[NFTA_SET_DATA_LEN]));
-		s->flags |= (1 << NFTNL_SET_DATA_LEN);
-	}
-	if (tb[NFTA_SET_ID]) {
-		s->id = ntohl(mnl_attr_get_u32(tb[NFTA_SET_ID]));
-		s->flags |= (1 << NFTNL_SET_ID);
-	}
-	if (tb[NFTA_SET_POLICY]) {
-		s->policy = ntohl(mnl_attr_get_u32(tb[NFTA_SET_POLICY]));
-		s->flags |= (1 << NFTNL_SET_POLICY);
-	}
-	if (tb[NFTA_SET_TIMEOUT]) {
-		s->timeout = be64toh(mnl_attr_get_u64(tb[NFTA_SET_TIMEOUT]));
-		s->flags |= (1 << NFTNL_SET_TIMEOUT);
-	}
-	if (tb[NFTA_SET_GC_INTERVAL]) {
-		s->gc_interval = ntohl(mnl_attr_get_u32(tb[NFTA_SET_GC_INTERVAL]));
-		s->flags |= (1 << NFTNL_SET_GC_INTERVAL);
+		ret = nftnl_set_set_str(s, NFTNL_SET_NAME,
+			mnl_attr_get_str(tb[NFTA_SET_NAME]));
+		if (ret)
+			return ret;
 	}
+	if (tb[NFTA_SET_FLAGS])
+		nftnl_set_set_u32(s, NFTNL_SET_FLAGS,
+			ntohl(mnl_attr_get_u32(tb[NFTA_SET_FLAGS])));
+	if (tb[NFTA_SET_KEY_TYPE])
+		nftnl_set_set_u32(s, NFTNL_SET_KEY_TYPE,
+			ntohl(mnl_attr_get_u32(tb[NFTA_SET_KEY_TYPE])));
+	if (tb[NFTA_SET_KEY_LEN])
+		nftnl_set_set_u32(s, NFTNL_SET_KEY_LEN,
+			ntohl(mnl_attr_get_u32(tb[NFTA_SET_KEY_LEN])));
+	if (tb[NFTA_SET_DATA_TYPE])
+		nftnl_set_set_u32(s, NFTNL_SET_DATA_TYPE,
+			ntohl(mnl_attr_get_u32(tb[NFTA_SET_DATA_TYPE])));
+	if (tb[NFTA_SET_DATA_LEN])
+		nftnl_set_set_u32(s, NFTNL_SET_DATA_LEN,
+			ntohl(mnl_attr_get_u32(tb[NFTA_SET_DATA_LEN])));
+	if (tb[NFTA_SET_ID])
+		nftnl_set_set_u32(s, NFTNL_SET_ID,
+			ntohl(mnl_attr_get_u32(tb[NFTA_SET_ID])));
+	if (tb[NFTA_SET_POLICY])
+		nftnl_set_set_u32(s, NFTNL_SET_POLICY,
+			ntohl(mnl_attr_get_u32(tb[NFTA_SET_POLICY])));
+	if (tb[NFTA_SET_TIMEOUT])
+		nftnl_set_set_u64(s, NFTNL_SET_TIMEOUT,
+			be64toh(mnl_attr_get_u64(tb[NFTA_SET_TIMEOUT])));
+	if (tb[NFTA_SET_GC_INTERVAL])
+		nftnl_set_set_u32(s, NFTNL_SET_GC_INTERVAL,
+			ntohl(mnl_attr_get_u32(tb[NFTA_SET_GC_INTERVAL])));
 	if (tb[NFTA_SET_DESC]) {
 		ret = nftnl_set_desc_parse(s, tb[NFTA_SET_DESC]);
 		if (ret)
 			return ret;
 	}
 
-	s->family = nfg->nfgen_family;
-	s->flags |= (1 << NFTNL_SET_FAMILY);
+	nftnl_set_set_u32(s, NFTNL_SET_FAMILY, nfg->nfgen_family);
 
 	return 0;
 }
diff --git a/src/set_elem.c b/src/set_elem.c
index e4dd313..a3467b4 100644
--- a/src/set_elem.c
+++ b/src/set_elem.c
@@ -119,6 +119,9 @@ int nftnl_set_elem_set(struct nftnl_set_elem *s, uint16_t attr,
 	case NFTNL_SET_ELEM_TIMEOUT:	/* NFTA_SET_ELEM_TIMEOUT */
 		s->timeout = *((uint64_t *)data);
 		break;
+	case NFTNL_SET_ELEM_EXPIRATION:	/* NFTA_SET_ELEM_EXPIRATION */
+		s->expiration = *((uint64_t *)data);
+		break;
 	case NFTNL_SET_ELEM_USERDATA: /* NFTA_SET_ELEM_USERDATA */
 		if (s->flags & (1 << NFTNL_SET_ELEM_USERDATA))
 			xfree(s->user.data);
@@ -355,19 +358,15 @@ static int nftnl_set_elems_parse2(struct nftnl_set *s, const struct nlattr *nest
 	if (ret)
 		goto out_set_elem;
 
-	if (tb[NFTA_SET_ELEM_FLAGS]) {
-		e->set_elem_flags =
-			ntohl(mnl_attr_get_u32(tb[NFTA_SET_ELEM_FLAGS]));
-		e->flags |= (1 << NFTNL_SET_ELEM_FLAGS);
-	}
-	if (tb[NFTA_SET_ELEM_TIMEOUT]) {
-		e->timeout = be64toh(mnl_attr_get_u64(tb[NFTA_SET_ELEM_TIMEOUT]));
-		e->flags |= (1 << NFTNL_SET_ELEM_TIMEOUT);
-	}
-	if (tb[NFTA_SET_ELEM_EXPIRATION]) {
-		e->expiration = be64toh(mnl_attr_get_u64(tb[NFTA_SET_ELEM_EXPIRATION]));
-		e->flags |= (1 << NFTNL_SET_ELEM_EXPIRATION);
-	}
+	if (tb[NFTA_SET_ELEM_FLAGS])
+		nftnl_set_elem_set_u32(e, NFTNL_SET_ELEM_FLAGS,
+			ntohl(mnl_attr_get_u32(tb[NFTA_SET_ELEM_FLAGS])));
+	if (tb[NFTA_SET_ELEM_TIMEOUT])
+		nftnl_set_elem_set_u64(e, NFTNL_SET_ELEM_TIMEOUT,
+			be64toh(mnl_attr_get_u64(tb[NFTA_SET_ELEM_TIMEOUT])));
+	if (tb[NFTA_SET_ELEM_EXPIRATION])
+		nftnl_set_elem_set_u64(e, NFTNL_SET_ELEM_EXPIRATION,
+			be64toh(mnl_attr_get_u64(tb[NFTA_SET_ELEM_EXPIRATION])));
         if (tb[NFTA_SET_ELEM_KEY]) {
 		ret = nftnl_parse_data(&e->key, tb[NFTA_SET_ELEM_KEY], &type);
 		if (ret)
@@ -398,18 +397,11 @@ static int nftnl_set_elems_parse2(struct nftnl_set *s, const struct nlattr *nest
 		e->flags |= (1 << NFTNL_SET_ELEM_EXPR);
 	}
 	if (tb[NFTA_SET_ELEM_USERDATA]) {
-		const void *udata =
-			mnl_attr_get_payload(tb[NFTA_SET_ELEM_USERDATA]);
-
-		if (e->flags & (1 << NFTNL_RULE_USERDATA))
-			xfree(e->user.data);
-
-		e->user.len  = mnl_attr_get_payload_len(tb[NFTA_SET_ELEM_USERDATA]);
-		e->user.data = malloc(e->user.len);
-		if (e->user.data == NULL)
+		ret = nftnl_set_elem_set(e, NFTNL_SET_ELEM_USERDATA,
+			mnl_attr_get_payload(tb[NFTA_SET_ELEM_USERDATA]),
+			mnl_attr_get_payload_len(tb[NFTA_SET_ELEM_USERDATA]));
+		if (ret)
 			goto out_expr;
-		memcpy(e->user.data, udata, e->user.len);
-		e->flags |= (1 << NFTNL_RULE_USERDATA);
 	}
 
 	/* Add this new element to this set */
@@ -475,35 +467,27 @@ int nftnl_set_elems_nlmsg_parse(const struct nlmsghdr *nlh, struct nftnl_set *s)
 		return -1;
 
 	if (tb[NFTA_SET_ELEM_LIST_TABLE]) {
-		if (s->flags & (1 << NFTNL_SET_TABLE))
-			xfree(s->table);
-		s->table =
-			strdup(mnl_attr_get_str(tb[NFTA_SET_ELEM_LIST_TABLE]));
-		if (!s->table)
-			return -1;
-		s->flags |= (1 << NFTNL_SET_TABLE);
+		ret = nftnl_set_set_str(s, NFTNL_SET_TABLE,
+			mnl_attr_get_str(tb[NFTA_SET_ELEM_LIST_TABLE]));
+		if (ret)
+			return ret;
 	}
 	if (tb[NFTA_SET_ELEM_LIST_SET]) {
-		if (s->flags & (1 << NFTNL_SET_NAME))
-			xfree(s->name);
-		s->name =
-			strdup(mnl_attr_get_str(tb[NFTA_SET_ELEM_LIST_SET]));
-		if (!s->name)
-			return -1;
-		s->flags |= (1 << NFTNL_SET_NAME);
-	}
-	if (tb[NFTA_SET_ELEM_LIST_SET_ID]) {
-		s->id = ntohl(mnl_attr_get_u32(tb[NFTA_SET_ELEM_LIST_SET_ID]));
-		s->flags |= (1 << NFTNL_SET_ID);
+		ret = nftnl_set_set_str(s, NFTNL_SET_NAME,
+			mnl_attr_get_str(tb[NFTA_SET_ELEM_LIST_SET]));
+		if (ret)
+			return ret;
 	}
+	if (tb[NFTA_SET_ELEM_LIST_SET_ID])
+		nftnl_set_set_u32(s, NFTNL_SET_ID,
+			mnl_attr_get_u32(tb[NFTA_SET_ELEM_LIST_SET_ID]));
         if (tb[NFTA_SET_ELEM_LIST_ELEMENTS]) {
 	 	ret = nftnl_set_elems_parse(s, tb[NFTA_SET_ELEM_LIST_ELEMENTS]);
 		if (ret)
 			return ret;
 	}
 
-	s->family = nfg->nfgen_family;
-	s->flags |= (1 << NFTNL_SET_FAMILY);
+	nftnl_set_set_u32(s, NFTNL_SET_FAMILY, nfg->nfgen_family);
 
 	return 0;
 }
diff --git a/src/table.c b/src/table.c
index 32d119f..8b72a69 100644
--- a/src/table.c
+++ b/src/table.c
@@ -222,29 +222,25 @@ int nftnl_table_nlmsg_parse(const struct nlmsghdr *nlh, struct nftnl_table *t)
 {
 	struct nlattr *tb[NFTA_TABLE_MAX+1] = {};
 	struct nfgenmsg *nfg = mnl_nlmsg_get_payload(nlh);
+	int err;
 
 	if (mnl_attr_parse(nlh, sizeof(*nfg), nftnl_table_parse_attr_cb, tb) < 0)
 		return -1;
 
 	if (tb[NFTA_TABLE_NAME]) {
-		if (t->flags & (1 << NFTNL_TABLE_NAME))
-			xfree(t->name);
-		t->name = strdup(mnl_attr_get_str(tb[NFTA_TABLE_NAME]));
-		if (!t->name)
-			return -1;
-		t->flags |= (1 << NFTNL_TABLE_NAME);
+		err = nftnl_table_set_str(t, NFTNL_TABLE_NAME,
+			mnl_attr_get_str(tb[NFTA_TABLE_NAME]));
+		if (err)
+			return err;
 	}
-	if (tb[NFTA_TABLE_FLAGS]) {
-		t->table_flags = ntohl(mnl_attr_get_u32(tb[NFTA_TABLE_FLAGS]));
-		t->flags |= (1 << NFTNL_TABLE_FLAGS);
-	}
-	if (tb[NFTA_TABLE_USE]) {
-		t->use = ntohl(mnl_attr_get_u32(tb[NFTA_TABLE_USE]));
-		t->flags |= (1 << NFTNL_TABLE_USE);
-	}
-
-	t->family = nfg->nfgen_family;
-	t->flags |= (1 << NFTNL_TABLE_FAMILY);
+	if (tb[NFTA_TABLE_FLAGS])
+		nftnl_table_set_u32(t, NFTNL_TABLE_FLAGS,
+			ntohl(mnl_attr_get_u32(tb[NFTA_TABLE_FLAGS])));
+	if (tb[NFTA_TABLE_USE])
+		nftnl_table_set_u32(t, NFTNL_TABLE_USE,
+			ntohl(mnl_attr_get_u32(tb[NFTA_TABLE_USE])));
+
+	nftnl_table_set_u32(t, NFTNL_TABLE_FAMILY, nfg->nfgen_family);
 
 	return 0;
 }
-- 
2.8.3

--
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: [PATCH 1/3 libnftnl] chain: Fix bug. Check correct attribute

[Pablo Neira Ayuso]

Applied, thanks for catching up this.

Please, include descriptions in your future patches.

Moreover, I'd suggest you use a title with a single sentence.

I have rewritten your patch title and description.

http://git.netfilter.org/libnftnl/commit/?id=2fee091b0dd1741a8a87cafceaa0091adadd2b46

Re: [PATCH 2/3 libnftnl] fix some error checking in parser functions

[Pablo Neira Ayuso]

On Thu, Jun 16, 2016 at 12:20:11PM +0200, Carlos Falgueras García wrote:
> diff --git a/src/rule.c b/src/rule.c
> index b009c37..930ecc0 100644
> --- a/src/rule.c
> +++ b/src/rule.c
> @@ -427,7 +427,7 @@ int nftnl_rule_nlmsg_parse(const struct nlmsghdr *nlh, struct nftnl_rule *r)
>  {
>  	struct nlattr *tb[NFTA_RULE_MAX+1] = {};
>  	struct nfgenmsg *nfg = mnl_nlmsg_get_payload(nlh);
> -	int ret = 0;
> +	int ret;
>  
>  	if (mnl_attr_parse(nlh, sizeof(*nfg), nftnl_rule_parse_attr_cb, tb) < 0)
>  		return -1;
> @@ -452,10 +452,16 @@ int nftnl_rule_nlmsg_parse(const struct nlmsghdr *nlh, struct nftnl_rule *r)
>  		r->handle = be64toh(mnl_attr_get_u64(tb[NFTA_RULE_HANDLE]));
>  		r->flags |= (1 << NFTNL_RULE_HANDLE);
>  	}
> -	if (tb[NFTA_RULE_EXPRESSIONS])
> +	if (tb[NFTA_RULE_EXPRESSIONS]) {
>  		ret = nftnl_rule_parse_expr(tb[NFTA_RULE_EXPRESSIONS], r);
> -	if (tb[NFTA_RULE_COMPAT])
> +		if (ret)
> +			return ret;

Please use:
                if (ret < 0)
                        return ret;

instead. I think this is the common idiom we're using all around this
library.

Thanks.
--
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