Re: [PATCH libnftnl] Fix string length calculations

[Carlos Falgueras García <carlosfg@xxxxxxxxxx>]

On 02/07/16 08:54, Pablo Neira Ayuso wrote:
> On Fri, Jul 01, 2016 at 06:11:43PM +0200, Carlos Falgueras García wrote:
> > These lengths must be one character longer to take account the null
> > character
> Please, place the change for src/set.c in this patch so I only need to
> apply one patch.

I will send it now.

> Another comment below.
>
> > Signed-off-by: Carlos Falgueras García <carlosfg@xxxxxxxxxx>
> > ---
> >   src/chain.c    | 2 +-
> >   src/rule.c     | 2 +-
> >   src/set_elem.c | 2 +-
> >   src/table.c    | 2 +-
> >   src/trace.c    | 6 +++---
> >   5 files changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/src/chain.c b/src/chain.c
> > index bfffbe0..cab64b5 100644
> > --- a/src/chain.c
> > +++ b/src/chain.c
> > @@ -260,7 +260,7 @@ EXPORT_SYMBOL_ALIAS(nftnl_chain_set_u8, nft_chain_attr_set_u8);
> >   
> >   int nftnl_chain_set_str(struct nftnl_chain *c, uint16_t attr, const char *str)
> >   {
> > -	return nftnl_chain_set_data(c, attr, str, strlen(str));
> > +	return nftnl_chain_set_data(c, attr, str, strlen(str) + 1);
> >   }
> >   EXPORT_SYMBOL_ALIAS(nftnl_chain_set_str, nft_chain_attr_set_str);
> >   
> > diff --git a/src/rule.c b/src/rule.c
> > index c87fea7..2b23c8e 100644
> > --- a/src/rule.c
> > +++ b/src/rule.c
> > @@ -198,7 +198,7 @@ EXPORT_SYMBOL_ALIAS(nftnl_rule_set_u64, nft_rule_attr_set_u64);
> >   
> >   int nftnl_rule_set_str(struct nftnl_rule *r, uint16_t attr, const char *str)
> >   {
> > -	return nftnl_rule_set_data(r, attr, str, strlen(str));
> > +	return nftnl_rule_set_data(r, attr, str, strlen(str) + 1);
> >   }
> >   EXPORT_SYMBOL_ALIAS(nftnl_rule_set_str, nft_rule_attr_set_str);
> >   
> > diff --git a/src/set_elem.c b/src/set_elem.c
> > index 00b7327..40b5bfe 100644
> > --- a/src/set_elem.c
> > +++ b/src/set_elem.c
> > @@ -149,7 +149,7 @@ EXPORT_SYMBOL_ALIAS(nftnl_set_elem_set_u64, nft_set_elem_attr_set_u64);
> >   
> >   int nftnl_set_elem_set_str(struct nftnl_set_elem *s, uint16_t attr, const char *str)
> >   {
> > -	return nftnl_set_elem_set(s, attr, str, strlen(str));
> > +	return nftnl_set_elem_set(s, attr, str, strlen(str) + 1);
> >   }
> >   EXPORT_SYMBOL_ALIAS(nftnl_set_elem_set_str, nft_set_elem_attr_set_str);
> >   
> > diff --git a/src/table.c b/src/table.c
> > index 32d119f..966b923 100644
> > --- a/src/table.c
> > +++ b/src/table.c
> > @@ -131,7 +131,7 @@ EXPORT_SYMBOL_ALIAS(nftnl_table_set_u8, nft_table_attr_set_u8);
> >   
> >   int nftnl_table_set_str(struct nftnl_table *t, uint16_t attr, const char *str)
> >   {
> > -	return nftnl_table_set_data(t, attr, str, 0);
> > +	return nftnl_table_set_data(t, attr, str, strlen(str) + 1);
> >   }
> >   EXPORT_SYMBOL_ALIAS(nftnl_table_set_str, nft_table_attr_set_str);
> >   
> > diff --git a/src/trace.c b/src/trace.c
> > index d8f561d..1a50390 100644
> > --- a/src/trace.c
> > +++ b/src/trace.c
> > @@ -165,13 +165,13 @@ const void *nftnl_trace_get_data(const struct nftnl_trace *trace,
> >   		*data_len = sizeof(uint32_t);
> >   		return &trace->type;
> >   	case NFTNL_TRACE_CHAIN:
> > -		*data_len = strlen(trace->chain);
> > +		*data_len = strlen(trace->chain) + 1;
> >   		return trace->chain;
> >   	case NFTNL_TRACE_TABLE:
> > -		*data_len = strlen(trace->table);
> > +		*data_len = strlen(trace->table) + 1;
> >   		return trace->table;
> >   	case NFTNL_TRACE_JUMP_TARGET:
> > -		*data_len = strlen(trace->jump_target);
> > +		*data_len = strlen(trace->jump_target) + 1;
> >   		return trace->jump_target;
> >   	case NFTNL_TRACE_TRANSPORT_HEADER:
> >   		*data_len = trace->th.len;
> Are you really sure we need this chunk too?
Yes, I think the user would expect that 'data_len' means data length 
instead of data length minus one.

When checking other getters I realized that in most cases we keep the 
parameter 'data_len' unset when the user ask for a string. Maybe we can 
fix these behavior, I think it make more sense if our getters always 
return the data length instead of keep an inconstant behavior.

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