On Thu, Jun 02, 2016 at 12:40:24PM +0200, Carlos Falgueras García wrote:
> When you set an object attribute the memory is copied, sometimes an
> allocations is needed and it must be checked. By now all setters methods
> returns void, so the policy adopted in case of error is keep the object
> unchanged.
>
> What this patch makes:
> * All memory allocations inside setters are checked
> * The object remains unchanged in case of error
> * Unsetters are used if is possible in order to consolidate
>
> Signed-off-by: Carlos Falgueras García <carlosfg@xxxxxxxxxx>
> ---
> src/chain.c | 26 +++++++++++++++++---------
> src/expr/dynset.c | 8 +++++++-
> src/expr/immediate.c | 9 ++++++---
> src/expr/log.c | 12 +++++++++---
> src/expr/lookup.c | 8 +++++++-
> src/rule.c | 28 +++++++++++++++++-----------
> src/set.c | 18 ++++++++++++------
> src/set_elem.c | 21 +++++++++++++--------
> 8 files changed, 88 insertions(+), 42 deletions(-)
>
> diff --git a/src/chain.c b/src/chain.c
> index 990c576..c4c4ff7 100644
> --- a/src/chain.c
> +++ b/src/chain.c
> @@ -168,6 +168,8 @@ static uint32_t nftnl_chain_validate[NFTNL_CHAIN_MAX + 1] = {
> void nftnl_chain_set_data(struct nftnl_chain *c, uint16_t attr,
> const void *data, uint32_t data_len)
> {
> + char *newstr;
> +
> if (attr > NFTNL_CHAIN_MAX)
> return;
>
> @@ -178,10 +180,12 @@ void nftnl_chain_set_data(struct nftnl_chain *c, uint16_t attr,
> strncpy(c->name, data, NFT_CHAIN_MAXNAMELEN);
> break;
> case NFTNL_CHAIN_TABLE:
> - if (c->table)
> - xfree(c->table);
> + newstr = strdup(data);
> + if (!newstr)
> + return;
>
> - c->table = strdup(data);
> + nftnl_chain_unset(c, attr);
> + c->table = newstr;
This looks a bit tangled. Probably something more simple, like this
below?
xfree(c->table);
c->table = strdup(data);
if (!c->table)
return;
Another comment below.
[...]
> diff --git a/src/expr/log.c b/src/expr/log.c
> index c3dc0a6..369174f 100644
> --- a/src/expr/log.c
> +++ b/src/expr/log.c
> @@ -34,13 +34,17 @@ static int nftnl_expr_log_set(struct nftnl_expr *e, uint16_t type,
> const void *data, uint32_t data_len)
> {
> struct nftnl_expr_log *log = nftnl_expr_data(e);
> + char *newstr;
>
> switch(type) {
> case NFTNL_EXPR_LOG_PREFIX:
> - if (log->prefix)
> - xfree(log->prefix);
> + newstr = strdup(data);
> + if (!newstr)
> + return -1;
>
> - log->prefix = strdup(data);
> + if (log->flags & (1 << NFTNL_EXPR_LOG_PREFIX))
> + xfree(log->prefix);
> + log->prefix = newstr;
> break;
> case NFTNL_EXPR_LOG_GROUP:
> log->group = *((uint16_t *)data);
> @@ -60,6 +64,8 @@ static int nftnl_expr_log_set(struct nftnl_expr *e, uint16_t type,
> default:
> return -1;
> }
> +
> + log->flags |= (1 << type);
Do we need this here?
> return 0;
> }
>
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
