From 5a54b99859fd52d8c7e866b57f6d8fac3e20a78f Mon Sep 17 00:00:00 2001 From: Phil Sutter Date: Wed, 2 Oct 2024 16:44:49 +0200 Subject: [PATCH] utils: Introduce nftnl_parse_str_attr() 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 Acked-by: Florian Westphal --- include/utils.h | 3 +++ src/chain.c | 33 +++++++++------------------------ src/expr/dynset.c | 12 +++++------- src/expr/flow_offload.c | 12 +++++------- src/expr/log.c | 13 ++++--------- src/expr/lookup.c | 12 +++++------- src/expr/objref.c | 18 ++++++++---------- src/flowtable.c | 24 ++++++++---------------- src/object.c | 14 ++++++-------- src/rule.c | 22 ++++++---------------- src/set.c | 22 ++++++---------------- src/set_elem.c | 38 +++++++++++++------------------------- src/table.c | 11 +++-------- src/trace.c | 28 ++++++++++------------------ src/utils.c | 15 +++++++++++++++ 15 files changed, 106 insertions(+), 171 deletions(-) diff --git a/include/utils.h b/include/utils.h index 5a3379fb..10492893 100644 --- a/include/utils.h +++ b/include/utils.h @@ -88,4 +88,7 @@ struct nlattr; void nftnl_attr_put_ifname(struct nlmsghdr *nlh, const char *ifname); char *nftnl_attr_get_ifname(const struct nlattr *attr); +int nftnl_parse_str_attr(const struct nlattr *tb, int attr, + const char **field, uint32_t *flags); + #endif diff --git a/src/chain.c b/src/chain.c index 83961144..8433f7db 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 40f9136a..dc74fbbe 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 5f209a63..ce22ec41 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 d4b53e6c..ead24379 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 7f85ecca..4f76c5b7 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 5fe09c24..1b27e94a 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 59991d69..27af51c6 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 275a202e..3d358ccf 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 3948a740..cd3041e5 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 412bdac6..54674bca 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 848adf1d..05220e79 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 9870dcaf..61382838 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 d67e1140..5dc4cfab 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 d73c5f61..bbe44b4d 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; +} -- 2.47.3