utils: Introduce nftnl_parse_str_attr()HEADmaster

Wrap the common parsing of string attributes in a function. Apart from
slightly reducing code size, this unifies callers in conditional freeing
of the field in case it was set before (missing in twelve spots) and
error checking for failing strdup()-calls (missing in four spots).

Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Florian Westphal <fw@strlen.de>

14 files changed, 103 insertions, 171 deletions

diff --git a/src/chain.c b/src/chain.c
index 8396114..8433f7d 100644
--- a/src/chain.c
+++ b/src/chain.c
@@ -675,22 +675,12 @@ int nftnl_chain_nlmsg_parse(const struct nlmsghdr *nlh, struct nftnl_chain *c)
 	if (mnl_attr_parse(nlh, sizeof(*nfg), nftnl_chain_parse_attr_cb, tb) < 0)
 		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);
-	}
-	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);
-	}
+	if (nftnl_parse_str_attr(tb[NFTA_CHAIN_NAME], NFTNL_CHAIN_NAME,
+				 &c->name, &c->flags) < 0)
+		return -1;
+	if (nftnl_parse_str_attr(tb[NFTA_CHAIN_TABLE], NFTNL_CHAIN_TABLE,
+				 &c->table, &c->flags) < 0)
+		return -1;
 	if (tb[NFTA_CHAIN_HOOK]) {
 		ret = nftnl_chain_parse_hook(tb[NFTA_CHAIN_HOOK], c);
 		if (ret < 0)
@@ -713,14 +703,9 @@ int nftnl_chain_nlmsg_parse(const struct nlmsghdr *nlh, struct nftnl_chain *c)
 		c->handle = be64toh(mnl_attr_get_u64(tb[NFTA_CHAIN_HANDLE]));
 		c->flags |= (1 << NFTNL_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);
-	}
+	if (nftnl_parse_str_attr(tb[NFTA_CHAIN_TYPE], NFTNL_CHAIN_TYPE,
+				 &c->type, &c->flags) < 0)
+		return -1;
 	if (tb[NFTA_CHAIN_FLAGS]) {
 		c->chain_flags = ntohl(mnl_attr_get_u32(tb[NFTA_CHAIN_FLAGS]));
 		c->flags |= (1 << NFTNL_CHAIN_FLAGS);
diff --git a/src/expr/dynset.c b/src/expr/dynset.c
index 40f9136..dc74fbb 100644
--- a/src/expr/dynset.c
+++ b/src/expr/dynset.c
@@ -245,13 +245,11 @@ nftnl_expr_dynset_parse(struct nftnl_expr *e, struct nlattr *attr)
 		dynset->timeout = be64toh(mnl_attr_get_u64(tb[NFTA_DYNSET_TIMEOUT]));
 		e->flags |= (1 << NFTNL_EXPR_DYNSET_TIMEOUT);
 	}
-	if (tb[NFTA_DYNSET_SET_NAME]) {
-		dynset->set_name =
-			strdup(mnl_attr_get_str(tb[NFTA_DYNSET_SET_NAME]));
-		if (!dynset->set_name)
-			return -1;
-		e->flags |= (1 << NFTNL_EXPR_DYNSET_SET_NAME);
-	}
+	if (nftnl_parse_str_attr(tb[NFTA_DYNSET_SET_NAME],
+				 NFTNL_EXPR_DYNSET_SET_NAME,
+				 (const char **)&dynset->set_name,
+				 &e->flags) < 0)
+		return -1;
 	if (tb[NFTA_DYNSET_SET_ID]) {
 		dynset->set_id = ntohl(mnl_attr_get_u32(tb[NFTA_DYNSET_SET_ID]));
 		e->flags |= (1 << NFTNL_EXPR_DYNSET_SET_ID);
diff --git a/src/expr/flow_offload.c b/src/expr/flow_offload.c
index 5f209a6..ce22ec4 100644
--- a/src/expr/flow_offload.c
+++ b/src/expr/flow_offload.c
@@ -79,13 +79,11 @@ static int nftnl_expr_flow_parse(struct nftnl_expr *e, struct nlattr *attr)
 	if (mnl_attr_parse_nested(attr, nftnl_expr_flow_cb, tb) < 0)
 		return -1;
 
-	if (tb[NFTA_FLOW_TABLE_NAME]) {
-		flow->table_name =
-			strdup(mnl_attr_get_str(tb[NFTA_FLOW_TABLE_NAME]));
-		if (!flow->table_name)
-			return -1;
-		e->flags |= (1 << NFTNL_EXPR_FLOW_TABLE_NAME);
-	}
+	if (nftnl_parse_str_attr(tb[NFTA_FLOW_TABLE_NAME],
+				 NFTNL_EXPR_FLOW_TABLE_NAME,
+				 (const char **)&flow->table_name,
+				 &e->flags) < 0)
+		return -1;
 
 	return ret;
 }
diff --git a/src/expr/log.c b/src/expr/log.c
index d4b53e6..ead2437 100644
--- a/src/expr/log.c
+++ b/src/expr/log.c
@@ -147,15 +147,10 @@ nftnl_expr_log_parse(struct nftnl_expr *e, struct nlattr *attr)
 	if (mnl_attr_parse_nested(attr, nftnl_expr_log_cb, tb) < 0)
 		return -1;
 
-	if (tb[NFTA_LOG_PREFIX]) {
-		if (log->prefix)
-			xfree(log->prefix);
-
-		log->prefix = strdup(mnl_attr_get_str(tb[NFTA_LOG_PREFIX]));
-		if (!log->prefix)
-			return -1;
-		e->flags |= (1 << NFTNL_EXPR_LOG_PREFIX);
-	}
+	if (nftnl_parse_str_attr(tb[NFTA_LOG_PREFIX],
+				 NFTNL_EXPR_LOG_PREFIX,
+				 &log->prefix, &e->flags) < 0)
+		return -1;
 	if (tb[NFTA_LOG_GROUP]) {
 		log->group = ntohs(mnl_attr_get_u16(tb[NFTA_LOG_GROUP]));
 		e->flags |= (1 << NFTNL_EXPR_LOG_GROUP);
diff --git a/src/expr/lookup.c b/src/expr/lookup.c
index 7f85ecc..4f76c5b 100644
--- a/src/expr/lookup.c
+++ b/src/expr/lookup.c
@@ -141,13 +141,11 @@ nftnl_expr_lookup_parse(struct nftnl_expr *e, struct nlattr *attr)
 		lookup->dreg = ntohl(mnl_attr_get_u32(tb[NFTA_LOOKUP_DREG]));
 		e->flags |= (1 << NFTNL_EXPR_LOOKUP_DREG);
 	}
-	if (tb[NFTA_LOOKUP_SET]) {
-		lookup->set_name =
-			strdup(mnl_attr_get_str(tb[NFTA_LOOKUP_SET]));
-		if (!lookup->set_name)
-			return -1;
-		e->flags |= (1 << NFTNL_EXPR_LOOKUP_SET);
-	}
+	if (nftnl_parse_str_attr(tb[NFTA_LOOKUP_SET],
+				 NFTNL_EXPR_LOOKUP_SET,
+				 (const char **)&lookup->set_name,
+				 &e->flags) < 0)
+		return -1;
 	if (tb[NFTA_LOOKUP_SET_ID]) {
 		lookup->set_id =
 			ntohl(mnl_attr_get_u32(tb[NFTA_LOOKUP_SET_ID]));
diff --git a/src/expr/objref.c b/src/expr/objref.c
index 5fe09c2..1b27e94 100644
--- a/src/expr/objref.c
+++ b/src/expr/objref.c
@@ -144,21 +144,19 @@ static int nftnl_expr_objref_parse(struct nftnl_expr *e, struct nlattr *attr)
 			ntohl(mnl_attr_get_u32(tb[NFTA_OBJREF_IMM_TYPE]));
 		e->flags |= (1 << NFTNL_EXPR_OBJREF_IMM_TYPE);
 	}
-	if (tb[NFTA_OBJREF_IMM_NAME]) {
-		objref->imm.name =
-			strdup(mnl_attr_get_str(tb[NFTA_OBJREF_IMM_NAME]));
-		e->flags |= (1 << NFTNL_EXPR_OBJREF_IMM_NAME);
-	}
+	if (nftnl_parse_str_attr(tb[NFTA_OBJREF_IMM_NAME],
+				 NFTNL_EXPR_OBJREF_IMM_NAME,
+				 &objref->imm.name, &e->flags) < 0)
+		return -1;
 	if (tb[NFTA_OBJREF_SET_SREG]) {
 		objref->set.sreg =
 			ntohl(mnl_attr_get_u32(tb[NFTA_OBJREF_SET_SREG]));
 		e->flags |= (1 << NFTNL_EXPR_OBJREF_SET_SREG);
 	}
-	if (tb[NFTA_OBJREF_SET_NAME]) {
-		objref->set.name =
-			strdup(mnl_attr_get_str(tb[NFTA_OBJREF_SET_NAME]));
-		e->flags |= (1 << NFTNL_EXPR_OBJREF_SET_NAME);
-	}
+	if (nftnl_parse_str_attr(tb[NFTA_OBJREF_SET_NAME],
+				 NFTNL_EXPR_OBJREF_SET_NAME,
+				 &objref->set.name, &e->flags) < 0)
+		return -1;
 	if (tb[NFTA_OBJREF_SET_ID]) {
 		objref->set.id =
 			ntohl(mnl_attr_get_u32(tb[NFTA_OBJREF_SET_ID]));
diff --git a/src/flowtable.c b/src/flowtable.c
index 59991d6..27af51c 100644
--- a/src/flowtable.c
+++ b/src/flowtable.c
@@ -408,22 +408,14 @@ int nftnl_flowtable_nlmsg_parse(const struct nlmsghdr *nlh, struct nftnl_flowtab
 	if (mnl_attr_parse(nlh, sizeof(*nfg), nftnl_flowtable_parse_attr_cb, tb) < 0)
 		return -1;
 
-	if (tb[NFTA_FLOWTABLE_NAME]) {
-		if (c->flags & (1 << NFTNL_FLOWTABLE_NAME))
-			xfree(c->name);
-		c->name = strdup(mnl_attr_get_str(tb[NFTA_FLOWTABLE_NAME]));
-		if (!c->name)
-			return -1;
-		c->flags |= (1 << NFTNL_FLOWTABLE_NAME);
-	}
-	if (tb[NFTA_FLOWTABLE_TABLE]) {
-		if (c->flags & (1 << NFTNL_FLOWTABLE_TABLE))
-			xfree(c->table);
-		c->table = strdup(mnl_attr_get_str(tb[NFTA_FLOWTABLE_TABLE]));
-		if (!c->table)
-			return -1;
-		c->flags |= (1 << NFTNL_FLOWTABLE_TABLE);
-	}
+	if (nftnl_parse_str_attr(tb[NFTA_FLOWTABLE_NAME],
+				 NFTNL_FLOWTABLE_NAME,
+				 &c->name, &c->flags) < 0)
+		return -1;
+	if (nftnl_parse_str_attr(tb[NFTA_FLOWTABLE_TABLE],
+				 NFTNL_FLOWTABLE_TABLE,
+				 &c->table, &c->flags) < 0)
+		return -1;
 	if (tb[NFTA_FLOWTABLE_HOOK]) {
 		ret = nftnl_flowtable_parse_hook(tb[NFTA_FLOWTABLE_HOOK], c);
 		if (ret < 0)
diff --git a/src/object.c b/src/object.c
index 275a202..3d358cc 100644
--- a/src/object.c
+++ b/src/object.c
@@ -344,14 +344,12 @@ int nftnl_obj_nlmsg_parse(const struct nlmsghdr *nlh, struct nftnl_obj *obj)
 	if (mnl_attr_parse(nlh, sizeof(*nfg), nftnl_obj_parse_attr_cb, tb) < 0)
 		return -1;
 
-	if (tb[NFTA_OBJ_TABLE]) {
-		obj->table = strdup(mnl_attr_get_str(tb[NFTA_OBJ_TABLE]));
-		obj->flags |= (1 << NFTNL_OBJ_TABLE);
-	}
-	if (tb[NFTA_OBJ_NAME]) {
-		obj->name = strdup(mnl_attr_get_str(tb[NFTA_OBJ_NAME]));
-		obj->flags |= (1 << NFTNL_OBJ_NAME);
-	}
+	if (nftnl_parse_str_attr(tb[NFTA_OBJ_TABLE], NFTNL_OBJ_TABLE,
+				 &obj->table, &obj->flags) < 0)
+		return -1;
+	if (nftnl_parse_str_attr(tb[NFTA_OBJ_NAME], NFTNL_OBJ_NAME,
+				 &obj->name, &obj->flags) < 0)
+		return -1;
 	if (tb[NFTA_OBJ_TYPE]) {
 		uint32_t type = ntohl(mnl_attr_get_u32(tb[NFTA_OBJ_TYPE]));
 
diff --git a/src/rule.c b/src/rule.c
index 3948a74..cd3041e 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -431,22 +431,12 @@ int nftnl_rule_nlmsg_parse(const struct nlmsghdr *nlh, struct nftnl_rule *r)
 	if (mnl_attr_parse(nlh, sizeof(*nfg), nftnl_rule_parse_attr_cb, tb) < 0)
 		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);
-	}
-	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 (nftnl_parse_str_attr(tb[NFTA_RULE_TABLE], NFTNL_RULE_TABLE,
+				 &r->table, &r->flags) < 0)
+		return -1;
+	if (nftnl_parse_str_attr(tb[NFTA_RULE_CHAIN], NFTNL_RULE_CHAIN,
+				 &r->chain, &r->flags) < 0)
+		return -1;
 	if (tb[NFTA_RULE_HANDLE]) {
 		r->handle = be64toh(mnl_attr_get_u64(tb[NFTA_RULE_HANDLE]));
 		r->flags |= (1 << NFTNL_RULE_HANDLE);
diff --git a/src/set.c b/src/set.c
index 412bdac..54674bc 100644
--- a/src/set.c
+++ b/src/set.c
@@ -657,22 +657,12 @@ int nftnl_set_nlmsg_parse(const struct nlmsghdr *nlh, struct nftnl_set *s)
 	if (mnl_attr_parse(nlh, sizeof(*nfg), nftnl_set_parse_attr_cb, tb) < 0)
 		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);
-	}
-	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 (nftnl_parse_str_attr(tb[NFTA_SET_TABLE], NFTNL_SET_TABLE,
+				 &s->table, &s->flags) < 0)
+		return -1;
+	if (nftnl_parse_str_attr(tb[NFTA_SET_NAME], NFTNL_SET_NAME,
+				 &s->name, &s->flags) < 0)
+		return -1;
 	if (tb[NFTA_SET_HANDLE]) {
 		s->handle = be64toh(mnl_attr_get_u64(tb[NFTA_SET_HANDLE]));
 		s->flags |= (1 << NFTNL_SET_HANDLE);
diff --git a/src/set_elem.c b/src/set_elem.c
index 848adf1..05220e7 100644
--- a/src/set_elem.c
+++ b/src/set_elem.c
@@ -576,13 +576,11 @@ static int nftnl_set_elems_parse2(struct nftnl_set *s, const struct nlattr *nest
 		memcpy(e->user.data, udata, e->user.len);
 		e->flags |= (1 << NFTNL_RULE_USERDATA);
 	}
-	if (tb[NFTA_SET_ELEM_OBJREF]) {
-		e->objref = strdup(mnl_attr_get_str(tb[NFTA_SET_ELEM_OBJREF]));
-		if (e->objref == NULL) {
-			ret = -1;
-			goto out_set_elem;
-		}
-		e->flags |= (1 << NFTNL_SET_ELEM_OBJREF);
+	if (nftnl_parse_str_attr(tb[NFTA_SET_ELEM_OBJREF],
+				 NFTNL_SET_ELEM_OBJREF,
+				 &e->objref, &e->flags) < 0) {
+		ret = -1;
+		goto out_set_elem;
 	}
 
 	/* Add this new element to this set */
@@ -646,24 +644,14 @@ int nftnl_set_elems_nlmsg_parse(const struct nlmsghdr *nlh, struct nftnl_set *s)
 			   nftnl_set_elem_list_parse_attr_cb, tb) < 0)
 		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);
-	}
-	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 (nftnl_parse_str_attr(tb[NFTA_SET_ELEM_LIST_TABLE],
+				 NFTNL_SET_TABLE,
+				 &s->table, &s->flags) < 0)
+		return -1;
+	if (nftnl_parse_str_attr(tb[NFTA_SET_ELEM_LIST_SET],
+				 NFTNL_SET_NAME,
+				 &s->name, &s->flags) < 0)
+		return -1;
 	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);
diff --git a/src/table.c b/src/table.c
index 9870dca..6138283 100644
--- a/src/table.c
+++ b/src/table.c
@@ -285,14 +285,9 @@ int nftnl_table_nlmsg_parse(const struct nlmsghdr *nlh, struct nftnl_table *t)
 	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);
-	}
+	if (nftnl_parse_str_attr(tb[NFTA_TABLE_NAME], NFTNL_TABLE_NAME,
+				 &t->name, &t->flags) < 0)
+		return -1;
 	if (tb[NFTA_TABLE_FLAGS]) {
 		t->table_flags = ntohl(mnl_attr_get_u32(tb[NFTA_TABLE_FLAGS]));
 		t->flags |= (1 << NFTNL_TABLE_FLAGS);
diff --git a/src/trace.c b/src/trace.c
index d67e114..5dc4cfa 100644
--- a/src/trace.c
+++ b/src/trace.c
@@ -336,11 +336,11 @@ static int nftnl_trace_parse_verdict(const struct nlattr *attr,
 	case NFT_JUMP:
 		if (!tb[NFTA_VERDICT_CHAIN])
 			abi_breakage();
-		t->jump_target = strdup(mnl_attr_get_str(tb[NFTA_VERDICT_CHAIN]));
-		if (!t->jump_target)
+		if (nftnl_parse_str_attr(tb[NFTA_VERDICT_CHAIN],
+					 NFTNL_TRACE_JUMP_TARGET,
+					 (const char **)&t->jump_target,
+					 &t->flags) < 0)
 			return -1;
-
-		t->flags |= (1 << NFTNL_TRACE_JUMP_TARGET);
 		break;
 	}
 	return 0;
@@ -370,21 +370,13 @@ int nftnl_trace_nlmsg_parse(const struct nlmsghdr *nlh, struct nftnl_trace *t)
 	t->id = ntohl(mnl_attr_get_u32(tb[NFTA_TRACE_ID]));
 	t->flags |= (1 << NFTNL_TRACE_ID);
 
-	if (tb[NFTA_TRACE_TABLE]) {
-		t->table = strdup(mnl_attr_get_str(tb[NFTA_TRACE_TABLE]));
-		if (!t->table)
-			return -1;
-
-		t->flags |= (1 << NFTNL_TRACE_TABLE);
-	}
-
-	if (tb[NFTA_TRACE_CHAIN]) {
-		t->chain = strdup(mnl_attr_get_str(tb[NFTA_TRACE_CHAIN]));
-		if (!t->chain)
-			return -1;
+	if (nftnl_parse_str_attr(tb[NFTA_TRACE_TABLE], NFTNL_TRACE_TABLE,
+				 (const char **)&t->table, &t->flags) < 0)
+		return -1;
 
-		t->flags |= (1 << NFTNL_TRACE_CHAIN);
-	}
+	if (nftnl_parse_str_attr(tb[NFTA_TRACE_CHAIN], NFTNL_TRACE_CHAIN,
+				 (const char **)&t->chain, &t->flags) < 0)
+		return -1;
 
 	if (tb[NFTA_TRACE_IIFTYPE]) {
 		t->iiftype = ntohs(mnl_attr_get_u16(tb[NFTA_TRACE_IIFTYPE]));
diff --git a/src/utils.c b/src/utils.c
index d73c5f6..bbe44b4 100644
--- a/src/utils.c
+++ b/src/utils.c
@@ -192,3 +192,18 @@ char *nftnl_attr_get_ifname(const struct nlattr *attr)
 		return NULL;
 	}
 }
+
+int nftnl_parse_str_attr(const struct nlattr *tb, int attr,
+			 const char **field, uint32_t *flags)
+{
+	if (!tb)
+		return 0;
+
+	if (*flags & (1 << attr))
+		xfree(*field);
+	*field = strdup(mnl_attr_get_str(tb));
+	if (!*field)
+		return -1;
+	*flags |= (1 << attr);
+	return 0;
+}