All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH libnftnl] set: Fix nftnl_set_set_str
@ 2016-06-27 16:24 Carlos Falgueras García
  2016-06-27 16:29 ` Carlos Falgueras García
  2016-07-01 14:22 ` Pablo Neira Ayuso
  0 siblings, 2 replies; 19+ messages in thread
From: Carlos Falgueras García @ 2016-06-27 16:24 UTC (permalink / raw)
  To: netfilter-devel; +Cc: pablo

We need the string length

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

diff --git a/src/set.c b/src/set.c
index 879100c..edbcbe5 100644
--- a/src/set.c
+++ b/src/set.c
@@ -203,7 +203,7 @@ EXPORT_SYMBOL_ALIAS(nftnl_set_set_u64, nft_set_attr_set_u64);
 
 int nftnl_set_set_str(struct nftnl_set *s, uint16_t attr, const char *str)
 {
-	return nftnl_set_set(s, attr, str);
+	return nftnl_set_set_data(s, attr, str, strlen(str));
 }
 EXPORT_SYMBOL_ALIAS(nftnl_set_set_str, nft_set_attr_set_str);
 
-- 
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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH libnftnl] set: Fix nftnl_set_set_str
  2016-06-27 16:24 [PATCH libnftnl] set: Fix nftnl_set_set_str Carlos Falgueras García
@ 2016-06-27 16:29 ` Carlos Falgueras García
  2016-07-01 14:22 ` Pablo Neira Ayuso
  1 sibling, 0 replies; 19+ messages in thread
From: Carlos Falgueras García @ 2016-06-27 16:29 UTC (permalink / raw)
  To: netfilter-devel; +Cc: pablo

On 06/27/2016 06:24 PM, Carlos Falgueras García wrote:
> We need the string length
>
> Signed-off-by: Carlos Falgueras García <carlosfg@riseup.net>
> ---
>  src/set.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/set.c b/src/set.c
> index 879100c..edbcbe5 100644
> --- a/src/set.c
> +++ b/src/set.c
> @@ -203,7 +203,7 @@ EXPORT_SYMBOL_ALIAS(nftnl_set_set_u64, nft_set_attr_set_u64);
>
>  int nftnl_set_set_str(struct nftnl_set *s, uint16_t attr, const char *str)
>  {
> -	return nftnl_set_set(s, attr, str);
> +	return nftnl_set_set_data(s, attr, str, strlen(str));
>  }
>  EXPORT_SYMBOL_ALIAS(nftnl_set_set_str, nft_set_attr_set_str);

This bug has gone unnoticed because all tests check if the set attribute 
was equal to the parsed one, but not check if it was really set. If you 
try to set an string but it fail, the test compares two empty strings 
and says that it is correct.

Maybe we can impove the tests if it checks too if the gotten attribute 
is equal to the set one.
--
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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH libnftnl] set: Fix nftnl_set_set_str
  2016-06-27 16:24 [PATCH libnftnl] set: Fix nftnl_set_set_str Carlos Falgueras García
  2016-06-27 16:29 ` Carlos Falgueras García
@ 2016-07-01 14:22 ` Pablo Neira Ayuso
  2016-07-01 16:07   ` [PATCH libnfntl v2] " Carlos Falgueras García
                     ` (2 more replies)
  1 sibling, 3 replies; 19+ messages in thread
From: Pablo Neira Ayuso @ 2016-07-01 14:22 UTC (permalink / raw)
  To: Carlos Falgueras García; +Cc: netfilter-devel

On Mon, Jun 27, 2016 at 06:24:25PM +0200, Carlos Falgueras García wrote:
> We need the string length
> 
> Signed-off-by: Carlos Falgueras García <carlosfg@riseup.net>
> ---
>  src/set.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/set.c b/src/set.c
> index 879100c..edbcbe5 100644
> --- a/src/set.c
> +++ b/src/set.c
> @@ -203,7 +203,7 @@ EXPORT_SYMBOL_ALIAS(nftnl_set_set_u64, nft_set_attr_set_u64);
>  
>  int nftnl_set_set_str(struct nftnl_set *s, uint16_t attr, const char *str)
>  {
> -	return nftnl_set_set(s, attr, str);
> +	return nftnl_set_set_data(s, attr, str, strlen(str));

I think this should be strlen(str) + 1 so we make sure the string is
nul-terminated.
--
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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH libnfntl v2] set: Fix nftnl_set_set_str
  2016-07-01 14:22 ` Pablo Neira Ayuso
@ 2016-07-01 16:07   ` Carlos Falgueras García
  2016-07-01 16:11   ` [PATCH libnftnl] Fix string length calculations Carlos Falgueras García
  2016-07-01 16:13   ` [PATCH libnftnl] set: Fix nftnl_set_set_str Carlos Falgueras García
  2 siblings, 0 replies; 19+ messages in thread
From: Carlos Falgueras García @ 2016-07-01 16:07 UTC (permalink / raw)
  To: netfilter-devel; +Cc: pablo

We need the string length

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

diff --git a/src/set.c b/src/set.c
index 47e0c45..8a025ab 100644
--- a/src/set.c
+++ b/src/set.c
@@ -190,7 +190,7 @@ EXPORT_SYMBOL_ALIAS(nftnl_set_set_u64, nft_set_attr_set_u64);
 
 int nftnl_set_set_str(struct nftnl_set *s, uint16_t attr, const char *str)
 {
-	return nftnl_set_set(s, attr, str);
+	return nftnl_set_set_data(s, attr, str, strlen(str) + 1);
 }
 EXPORT_SYMBOL_ALIAS(nftnl_set_set_str, nft_set_attr_set_str);
 
-- 
2.5.1

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH libnftnl] Fix string length calculations
  2016-07-01 14:22 ` Pablo Neira Ayuso
  2016-07-01 16:07   ` [PATCH libnfntl v2] " Carlos Falgueras García
@ 2016-07-01 16:11   ` Carlos Falgueras García
  2016-07-02  6:54     ` Pablo Neira Ayuso
  2016-07-01 16:13   ` [PATCH libnftnl] set: Fix nftnl_set_set_str Carlos Falgueras García
  2 siblings, 1 reply; 19+ messages in thread
From: Carlos Falgueras García @ 2016-07-01 16:11 UTC (permalink / raw)
  To: netfilter-devel; +Cc: pablo

These lengths must be one character longer to take account the null
character

Signed-off-by: Carlos Falgueras García <carlosfg@riseup.net>
---
 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;
-- 
2.5.1

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH libnftnl] set: Fix nftnl_set_set_str
  2016-07-01 14:22 ` Pablo Neira Ayuso
  2016-07-01 16:07   ` [PATCH libnfntl v2] " Carlos Falgueras García
  2016-07-01 16:11   ` [PATCH libnftnl] Fix string length calculations Carlos Falgueras García
@ 2016-07-01 16:13   ` Carlos Falgueras García
  2 siblings, 0 replies; 19+ messages in thread
From: Carlos Falgueras García @ 2016-07-01 16:13 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel

On 01/07/16 16:22, Pablo Neira Ayuso wrote:
> On Mon, Jun 27, 2016 at 06:24:25PM +0200, Carlos Falgueras García wrote:
>> We need the string length
>>
>> Signed-off-by: Carlos Falgueras García <carlosfg@riseup.net>
>> ---
>>   src/set.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/set.c b/src/set.c
>> index 879100c..edbcbe5 100644
>> --- a/src/set.c
>> +++ b/src/set.c
>> @@ -203,7 +203,7 @@ EXPORT_SYMBOL_ALIAS(nftnl_set_set_u64, nft_set_attr_set_u64);
>>   
>>   int nftnl_set_set_str(struct nftnl_set *s, uint16_t attr, const char *str)
>>   {
>> -	return nftnl_set_set(s, attr, str);
>> +	return nftnl_set_set_data(s, attr, str, strlen(str));
> I think this should be strlen(str) + 1 so we make sure the string is
> nul-terminated.
Thanks Pablo. I have sent a v2 and another patch to fix this error in 
other code parts.
--
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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH libnftnl] Fix string length calculations
  2016-07-01 16:11   ` [PATCH libnftnl] Fix string length calculations Carlos Falgueras García
@ 2016-07-02  6:54     ` Pablo Neira Ayuso
  2016-07-03 10:13       ` Carlos Falgueras García
  0 siblings, 1 reply; 19+ messages in thread
From: Pablo Neira Ayuso @ 2016-07-02  6:54 UTC (permalink / raw)
  To: Carlos Falgueras García; +Cc: netfilter-devel

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.

Another comment below.

> Signed-off-by: Carlos Falgueras García <carlosfg@riseup.net>
> ---
>  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?
--
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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH libnftnl] Fix string length calculations
  2016-07-02  6:54     ` Pablo Neira Ayuso
@ 2016-07-03 10:13       ` Carlos Falgueras García
  2016-07-05 12:31         ` Pablo Neira Ayuso
  0 siblings, 1 reply; 19+ messages in thread
From: Carlos Falgueras García @ 2016-07-03 10:13 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel

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@riseup.net>
>> ---
>>   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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH libnftnl] Fix string length calculations
  2016-07-03 10:13       ` Carlos Falgueras García
@ 2016-07-05 12:31         ` Pablo Neira Ayuso
  2016-07-05 17:15           ` [PATCH 1/2 libnfntl] Fix nftnl_*_set_str Carlos Falgueras García
  0 siblings, 1 reply; 19+ messages in thread
From: Pablo Neira Ayuso @ 2016-07-05 12:31 UTC (permalink / raw)
  To: Carlos Falgueras García; +Cc: netfilter-devel

On Sun, Jul 03, 2016 at 12:13:18PM +0200, Carlos Falgueras García wrote:
> >>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.

_get() for strings functions should retain the strlen() semantics.

So I'd suggest you just fix the string setters.
--
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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 1/2 libnfntl] Fix nftnl_*_set_str
  2016-07-05 12:31         ` Pablo Neira Ayuso
@ 2016-07-05 17:15           ` Carlos Falgueras García
  2016-07-05 17:15             ` [PATCH 2/2 libnfntl] Fix nftnl_*_get to set data_len Carlos Falgueras García
  2016-07-06 15:19             ` [PATCH 1/2 libnfntl] Fix nftnl_*_set_str Pablo Neira Ayuso
  0 siblings, 2 replies; 19+ messages in thread
From: Carlos Falgueras García @ 2016-07-05 17:15 UTC (permalink / raw)
  To: netfilter-devel; +Cc: pablo

These lengths must be one character longer to take account the null
character

Signed-off-by: Carlos Falgueras García <carlosfg@riseup.net>
---
 src/chain.c    | 2 +-
 src/rule.c     | 2 +-
 src/set.c      | 2 +-
 src/set_elem.c | 2 +-
 src/table.c    | 2 +-
 5 files changed, 5 insertions(+), 5 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.c b/src/set.c
index 9315bf0..e48ff78 100644
--- a/src/set.c
+++ b/src/set.c
@@ -203,7 +203,7 @@ EXPORT_SYMBOL_ALIAS(nftnl_set_set_u64, nft_set_attr_set_u64);
 
 int nftnl_set_set_str(struct nftnl_set *s, uint16_t attr, const char *str)
 {
-	return nftnl_set_set(s, attr, str);
+	return nftnl_set_set_data(s, attr, str, strlen(str) + 1);
 }
 EXPORT_SYMBOL_ALIAS(nftnl_set_set_str, nft_set_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);
 
-- 
2.5.1

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 2/2 libnfntl] Fix nftnl_*_get to set data_len
  2016-07-05 17:15           ` [PATCH 1/2 libnfntl] Fix nftnl_*_set_str Carlos Falgueras García
@ 2016-07-05 17:15             ` Carlos Falgueras García
  2016-07-06 15:21               ` Pablo Neira Ayuso
  2016-07-11 10:24               ` [PATCH 2/2 libnfntl] " Pablo Neira Ayuso
  2016-07-06 15:19             ` [PATCH 1/2 libnfntl] Fix nftnl_*_set_str Pablo Neira Ayuso
  1 sibling, 2 replies; 19+ messages in thread
From: Carlos Falgueras García @ 2016-07-05 17:15 UTC (permalink / raw)
  To: netfilter-devel; +Cc: pablo

All getters must set the output parameter 'data_len'

Signed-off-by: Carlos Falgueras García <carlosfg@riseup.net>
---
 src/chain.c       | 3 +++
 src/expr.c        | 1 +
 src/expr/dynset.c | 3 +++
 src/expr/lookup.c | 3 +++
 src/gen.c         | 1 +
 src/rule.c        | 2 ++
 src/set.c         | 2 ++
 src/set_elem.c    | 6 ++++++
 src/table.c       | 1 +
 9 files changed, 22 insertions(+)

diff --git a/src/chain.c b/src/chain.c
index cab64b5..ccf0f92 100644
--- a/src/chain.c
+++ b/src/chain.c
@@ -272,8 +272,10 @@ const void *nftnl_chain_get_data(const struct nftnl_chain *c, uint16_t attr,
 
 	switch(attr) {
 	case NFTNL_CHAIN_NAME:
+		*data_len = strlen(c->name);
 		return c->name;
 	case NFTNL_CHAIN_TABLE:
+		*data_len = strlen(c->table);
 		return c->table;
 	case NFTNL_CHAIN_HOOKNUM:
 		*data_len = sizeof(uint32_t);
@@ -303,6 +305,7 @@ const void *nftnl_chain_get_data(const struct nftnl_chain *c, uint16_t attr,
 		*data_len = sizeof(uint32_t);
 		return c->type;
 	case NFTNL_CHAIN_DEV:
+		*data_len = strlen(c->dev);
 		return c->dev;
 	}
 	return NULL;
diff --git a/src/expr.c b/src/expr.c
index f802725..06afcd3 100644
--- a/src/expr.c
+++ b/src/expr.c
@@ -119,6 +119,7 @@ const void *nftnl_expr_get(const struct nftnl_expr *expr,
 
 	switch(type) {
 	case NFTNL_EXPR_NAME:
+		*data_len = strlen(expr->ops->name);
 		ret = expr->ops->name;
 		break;
 	default:
diff --git a/src/expr/dynset.c b/src/expr/dynset.c
index 0404359..6bf360e 100644
--- a/src/expr/dynset.c
+++ b/src/expr/dynset.c
@@ -88,10 +88,13 @@ nftnl_expr_dynset_get(const struct nftnl_expr *e, uint16_t type,
 		*data_len = sizeof(dynset->timeout);
 		return &dynset->timeout;
 	case NFTNL_EXPR_DYNSET_SET_NAME:
+		*data_len = strlen(dynset->set_name);
 		return dynset->set_name;
 	case NFTNL_EXPR_DYNSET_SET_ID:
+		*data_len = sizeof(dynset->set_id);
 		return &dynset->set_id;
 	case NFTNL_EXPR_DYNSET_EXPR:
+		*data_len = 0;
 		return dynset->expr;
 	}
 	return NULL;
diff --git a/src/expr/lookup.c b/src/expr/lookup.c
index 7f68f74..54d19d6 100644
--- a/src/expr/lookup.c
+++ b/src/expr/lookup.c
@@ -73,10 +73,13 @@ nftnl_expr_lookup_get(const struct nftnl_expr *e, uint16_t type,
 		*data_len = sizeof(lookup->dreg);
 		return &lookup->dreg;
 	case NFTNL_EXPR_LOOKUP_SET:
+		*data_len = strlen(lookup->set_name);
 		return lookup->set_name;
 	case NFTNL_EXPR_LOOKUP_SET_ID:
+		*data_len = sizeof(lookup->set_id);
 		return &lookup->set_id;
 	case NFTNL_EXPR_LOOKUP_FLAGS:
+		*data_len = sizeof(lookup->flags);
 		return &lookup->flags;
 	}
 	return NULL;
diff --git a/src/gen.c b/src/gen.c
index 37a9049..c69d2f8 100644
--- a/src/gen.c
+++ b/src/gen.c
@@ -100,6 +100,7 @@ const void *nftnl_gen_get_data(const struct nftnl_gen *gen, uint16_t attr,
 
 	switch(attr) {
 	case NFTNL_GEN_ID:
+		*data_len = sizeof(gen->id);
 		return &gen->id;
 	}
 	return NULL;
diff --git a/src/rule.c b/src/rule.c
index 2b23c8e..bf72595 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -213,8 +213,10 @@ const void *nftnl_rule_get_data(const struct nftnl_rule *r, uint16_t attr,
 		*data_len = sizeof(uint32_t);
 		return &r->family;
 	case NFTNL_RULE_TABLE:
+		*data_len = strlen(r->table);
 		return r->table;
 	case NFTNL_RULE_CHAIN:
+		*data_len = strlen(r->chain);
 		return r->chain;
 	case NFTNL_RULE_HANDLE:
 		*data_len = sizeof(uint64_t);
diff --git a/src/set.c b/src/set.c
index e48ff78..f728373 100644
--- a/src/set.c
+++ b/src/set.c
@@ -215,8 +215,10 @@ const void *nftnl_set_get_data(const struct nftnl_set *s, uint16_t attr,
 
 	switch(attr) {
 	case NFTNL_SET_TABLE:
+		*data_len = strlen(s->table);
 		return s->table;
 	case NFTNL_SET_NAME:
+		*data_len = strlen(s->name);
 		return s->name;
 	case NFTNL_SET_FLAGS:
 		*data_len = sizeof(uint32_t);
diff --git a/src/set_elem.c b/src/set_elem.c
index 40b5bfe..2056209 100644
--- a/src/set_elem.c
+++ b/src/set_elem.c
@@ -160,25 +160,31 @@ const void *nftnl_set_elem_get(struct nftnl_set_elem *s, uint16_t attr, uint32_t
 
 	switch(attr) {
 	case NFTNL_SET_ELEM_FLAGS:
+		*data_len = sizeof(s->set_elem_flags);
 		return &s->set_elem_flags;
 	case NFTNL_SET_ELEM_KEY:	/* NFTA_SET_ELEM_KEY */
 		*data_len = s->key.len;
 		return &s->key.val;
 	case NFTNL_SET_ELEM_VERDICT:	/* NFTA_SET_ELEM_DATA */
+		*data_len = sizeof(s->data.verdict);
 		return &s->data.verdict;
 	case NFTNL_SET_ELEM_CHAIN:	/* NFTA_SET_ELEM_DATA */
+		*data_len = strlen(s->data.chain);
 		return s->data.chain;
 	case NFTNL_SET_ELEM_DATA:	/* NFTA_SET_ELEM_DATA */
 		*data_len = s->data.len;
 		return &s->data.val;
 	case NFTNL_SET_ELEM_TIMEOUT:	/* NFTA_SET_ELEM_TIMEOUT */
+		*data_len = sizeof(s->timeout);
 		return &s->timeout;
 	case NFTNL_SET_ELEM_EXPIRATION:	/* NFTA_SET_ELEM_EXPIRATION */
+		*data_len = sizeof(s->expiration);
 		return &s->expiration;
 	case NFTNL_SET_ELEM_USERDATA:
 		*data_len = s->user.len;
 		return s->user.data;
 	case NFTNL_SET_ELEM_EXPR:
+		*data_len = 0;
 		return s->expr;
 	}
 	return NULL;
diff --git a/src/table.c b/src/table.c
index 966b923..78e27bc 100644
--- a/src/table.c
+++ b/src/table.c
@@ -143,6 +143,7 @@ const void *nftnl_table_get_data(const struct nftnl_table *t, uint16_t attr,
 
 	switch(attr) {
 	case NFTNL_TABLE_NAME:
+		*data_len = strlen(t->name);
 		return t->name;
 	case NFTNL_TABLE_FLAGS:
 		*data_len = sizeof(uint32_t);
-- 
2.5.1

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/2 libnfntl] Fix nftnl_*_set_str
  2016-07-05 17:15           ` [PATCH 1/2 libnfntl] Fix nftnl_*_set_str Carlos Falgueras García
  2016-07-05 17:15             ` [PATCH 2/2 libnfntl] Fix nftnl_*_get to set data_len Carlos Falgueras García
@ 2016-07-06 15:19             ` Pablo Neira Ayuso
  1 sibling, 0 replies; 19+ messages in thread
From: Pablo Neira Ayuso @ 2016-07-06 15:19 UTC (permalink / raw)
  To: Carlos Falgueras García; +Cc: netfilter-devel

On Tue, Jul 05, 2016 at 07:15:16PM +0200, Carlos Falgueras García wrote:
> These lengths must be one character longer to take account the null
> character

Applied, thanks Carlos.
--
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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/2 libnfntl] Fix nftnl_*_get to set data_len
  2016-07-05 17:15             ` [PATCH 2/2 libnfntl] Fix nftnl_*_get to set data_len Carlos Falgueras García
@ 2016-07-06 15:21               ` Pablo Neira Ayuso
  2016-07-11 11:41                 ` [PATCH v2, libnftnl] " Carlos Falgueras García
  2016-07-11 10:24               ` [PATCH 2/2 libnfntl] " Pablo Neira Ayuso
  1 sibling, 1 reply; 19+ messages in thread
From: Pablo Neira Ayuso @ 2016-07-06 15:21 UTC (permalink / raw)
  To: Carlos Falgueras García; +Cc: netfilter-devel

On Tue, Jul 05, 2016 at 07:15:17PM +0200, Carlos Falgueras García wrote:
> All getters must set the output parameter 'data_len'

Then, send me a v2 that also replaces strlen(x) + 1 to strlen(x)
from getters.

I can see several spots in src/expr/ that return strlen(x) + 1.  So we
leave this in consistent state in one go.

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/2 libnfntl] Fix nftnl_*_get to set data_len
  2016-07-05 17:15             ` [PATCH 2/2 libnfntl] Fix nftnl_*_get to set data_len Carlos Falgueras García
  2016-07-06 15:21               ` Pablo Neira Ayuso
@ 2016-07-11 10:24               ` Pablo Neira Ayuso
  2016-07-11 10:25                 ` Pablo Neira Ayuso
  1 sibling, 1 reply; 19+ messages in thread
From: Pablo Neira Ayuso @ 2016-07-11 10:24 UTC (permalink / raw)
  To: Carlos Falgueras García; +Cc: netfilter-devel

Carlos,

Habla con Laura para ver cómo lleva este cambio en la reunión:

http://patchwork.ozlabs.org/patch/639253/

Si ella no anda con tiempo, creo que tú tienes los conocimientos para
hacer este cambio que describo ahí.

No lo olvides. Gracias.
--
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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/2 libnfntl] Fix nftnl_*_get to set data_len
  2016-07-11 10:24               ` [PATCH 2/2 libnfntl] " Pablo Neira Ayuso
@ 2016-07-11 10:25                 ` Pablo Neira Ayuso
  0 siblings, 0 replies; 19+ messages in thread
From: Pablo Neira Ayuso @ 2016-07-11 10:25 UTC (permalink / raw)
  To: Carlos Falgueras García; +Cc: netfilter-devel

On Mon, Jul 11, 2016 at 12:24:27PM +0200, Pablo Neira Ayuso wrote:
> Carlos,
> 
> Habla con Laura para ver cómo lleva este cambio en la reunión:
> 
> http://patchwork.ozlabs.org/patch/639253/
> 
> Si ella no anda con tiempo, creo que tú tienes los conocimientos para
> hacer este cambio que describo ahí.
> 
> No lo olvides. Gracias.

Sorry, I accidentally sent this email to the mailing list in Spanish.
--
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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v2, libnftnl] Fix nftnl_*_get to set data_len
  2016-07-06 15:21               ` Pablo Neira Ayuso
@ 2016-07-11 11:41                 ` Carlos Falgueras García
  2016-07-11 11:48                   ` Pablo Neira Ayuso
  0 siblings, 1 reply; 19+ messages in thread
From: Carlos Falgueras García @ 2016-07-11 11:41 UTC (permalink / raw)
  To: netfilter-devel; +Cc: pablo

All getters must set the output parameter 'data_len'

Signed-off-by: Carlos Falgueras García <carlosfg@riseup.net>
---
 src/chain.c       | 3 +++
 src/expr.c        | 1 +
 src/expr/dynset.c | 3 +++
 src/expr/lookup.c | 3 +++
 src/gen.c         | 1 +
 src/rule.c        | 2 ++
 src/set.c         | 2 ++
 src/set_elem.c    | 6 ++++++
 src/table.c       | 1 +
 9 files changed, 22 insertions(+)

diff --git a/src/chain.c b/src/chain.c
index cab64b5..4c562fe 100644
--- a/src/chain.c
+++ b/src/chain.c
@@ -272,8 +272,10 @@ const void *nftnl_chain_get_data(const struct nftnl_chain *c, uint16_t attr,
 
 	switch(attr) {
 	case NFTNL_CHAIN_NAME:
+		*data_len = strlen(c->name) + 1;
 		return c->name;
 	case NFTNL_CHAIN_TABLE:
+		*data_len = strlen(c->table) + 1;
 		return c->table;
 	case NFTNL_CHAIN_HOOKNUM:
 		*data_len = sizeof(uint32_t);
@@ -303,6 +305,7 @@ const void *nftnl_chain_get_data(const struct nftnl_chain *c, uint16_t attr,
 		*data_len = sizeof(uint32_t);
 		return c->type;
 	case NFTNL_CHAIN_DEV:
+		*data_len = strlen(c->dev) + 1;
 		return c->dev;
 	}
 	return NULL;
diff --git a/src/expr.c b/src/expr.c
index f802725..e5c1dd3 100644
--- a/src/expr.c
+++ b/src/expr.c
@@ -119,6 +119,7 @@ const void *nftnl_expr_get(const struct nftnl_expr *expr,
 
 	switch(type) {
 	case NFTNL_EXPR_NAME:
+		*data_len = strlen(expr->ops->name) + 1;
 		ret = expr->ops->name;
 		break;
 	default:
diff --git a/src/expr/dynset.c b/src/expr/dynset.c
index 0404359..3f6fb08 100644
--- a/src/expr/dynset.c
+++ b/src/expr/dynset.c
@@ -88,10 +88,13 @@ nftnl_expr_dynset_get(const struct nftnl_expr *e, uint16_t type,
 		*data_len = sizeof(dynset->timeout);
 		return &dynset->timeout;
 	case NFTNL_EXPR_DYNSET_SET_NAME:
+		*data_len = strlen(dynset->set_name) + 1;
 		return dynset->set_name;
 	case NFTNL_EXPR_DYNSET_SET_ID:
+		*data_len = sizeof(dynset->set_id) + 1;
 		return &dynset->set_id;
 	case NFTNL_EXPR_DYNSET_EXPR:
+		*data_len = 0;
 		return dynset->expr;
 	}
 	return NULL;
diff --git a/src/expr/lookup.c b/src/expr/lookup.c
index 7f68f74..a29b7e5 100644
--- a/src/expr/lookup.c
+++ b/src/expr/lookup.c
@@ -73,10 +73,13 @@ nftnl_expr_lookup_get(const struct nftnl_expr *e, uint16_t type,
 		*data_len = sizeof(lookup->dreg);
 		return &lookup->dreg;
 	case NFTNL_EXPR_LOOKUP_SET:
+		*data_len = strlen(lookup->set_name) + 1;
 		return lookup->set_name;
 	case NFTNL_EXPR_LOOKUP_SET_ID:
+		*data_len = sizeof(lookup->set_id) + 1;
 		return &lookup->set_id;
 	case NFTNL_EXPR_LOOKUP_FLAGS:
+		*data_len = sizeof(lookup->flags) + 1;
 		return &lookup->flags;
 	}
 	return NULL;
diff --git a/src/gen.c b/src/gen.c
index 37a9049..ed815e5 100644
--- a/src/gen.c
+++ b/src/gen.c
@@ -100,6 +100,7 @@ const void *nftnl_gen_get_data(const struct nftnl_gen *gen, uint16_t attr,
 
 	switch(attr) {
 	case NFTNL_GEN_ID:
+		*data_len = sizeof(gen->id) + 1;
 		return &gen->id;
 	}
 	return NULL;
diff --git a/src/rule.c b/src/rule.c
index 2b23c8e..a0edca7 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -213,8 +213,10 @@ const void *nftnl_rule_get_data(const struct nftnl_rule *r, uint16_t attr,
 		*data_len = sizeof(uint32_t);
 		return &r->family;
 	case NFTNL_RULE_TABLE:
+		*data_len = strlen(r->table) + 1;
 		return r->table;
 	case NFTNL_RULE_CHAIN:
+		*data_len = strlen(r->chain) + 1;
 		return r->chain;
 	case NFTNL_RULE_HANDLE:
 		*data_len = sizeof(uint64_t);
diff --git a/src/set.c b/src/set.c
index e48ff78..8a592db 100644
--- a/src/set.c
+++ b/src/set.c
@@ -215,8 +215,10 @@ const void *nftnl_set_get_data(const struct nftnl_set *s, uint16_t attr,
 
 	switch(attr) {
 	case NFTNL_SET_TABLE:
+		*data_len = strlen(s->table) + 1;
 		return s->table;
 	case NFTNL_SET_NAME:
+		*data_len = strlen(s->name) + 1;
 		return s->name;
 	case NFTNL_SET_FLAGS:
 		*data_len = sizeof(uint32_t);
diff --git a/src/set_elem.c b/src/set_elem.c
index 40b5bfe..02808aa 100644
--- a/src/set_elem.c
+++ b/src/set_elem.c
@@ -160,25 +160,31 @@ const void *nftnl_set_elem_get(struct nftnl_set_elem *s, uint16_t attr, uint32_t
 
 	switch(attr) {
 	case NFTNL_SET_ELEM_FLAGS:
+		*data_len = sizeof(s->set_elem_flags) + 1;
 		return &s->set_elem_flags;
 	case NFTNL_SET_ELEM_KEY:	/* NFTA_SET_ELEM_KEY */
 		*data_len = s->key.len;
 		return &s->key.val;
 	case NFTNL_SET_ELEM_VERDICT:	/* NFTA_SET_ELEM_DATA */
+		*data_len = sizeof(s->data.verdict) + 1;
 		return &s->data.verdict;
 	case NFTNL_SET_ELEM_CHAIN:	/* NFTA_SET_ELEM_DATA */
+		*data_len = strlen(s->data.chain) + 1;
 		return s->data.chain;
 	case NFTNL_SET_ELEM_DATA:	/* NFTA_SET_ELEM_DATA */
 		*data_len = s->data.len;
 		return &s->data.val;
 	case NFTNL_SET_ELEM_TIMEOUT:	/* NFTA_SET_ELEM_TIMEOUT */
+		*data_len = sizeof(s->timeout) + 1;
 		return &s->timeout;
 	case NFTNL_SET_ELEM_EXPIRATION:	/* NFTA_SET_ELEM_EXPIRATION */
+		*data_len = sizeof(s->expiration) + 1;
 		return &s->expiration;
 	case NFTNL_SET_ELEM_USERDATA:
 		*data_len = s->user.len;
 		return s->user.data;
 	case NFTNL_SET_ELEM_EXPR:
+		*data_len = 0;
 		return s->expr;
 	}
 	return NULL;
diff --git a/src/table.c b/src/table.c
index 966b923..3d4d7b9 100644
--- a/src/table.c
+++ b/src/table.c
@@ -143,6 +143,7 @@ const void *nftnl_table_get_data(const struct nftnl_table *t, uint16_t attr,
 
 	switch(attr) {
 	case NFTNL_TABLE_NAME:
+		*data_len = strlen(t->name) + 1;
 		return t->name;
 	case NFTNL_TABLE_FLAGS:
 		*data_len = sizeof(uint32_t);
-- 
2.5.1

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v2, libnftnl] Fix nftnl_*_get to set data_len
  2016-07-11 11:41                 ` [PATCH v2, libnftnl] " Carlos Falgueras García
@ 2016-07-11 11:48                   ` Pablo Neira Ayuso
  2016-07-11 16:07                     ` [PATCH v3, " Carlos Falgueras García
  0 siblings, 1 reply; 19+ messages in thread
From: Pablo Neira Ayuso @ 2016-07-11 11:48 UTC (permalink / raw)
  To: Carlos Falgueras García; +Cc: netfilter-devel

On Mon, Jul 11, 2016 at 01:41:07PM +0200, Carlos Falgueras García wrote:
> diff --git a/src/expr/lookup.c b/src/expr/lookup.c
> index 7f68f74..a29b7e5 100644
> --- a/src/expr/lookup.c
> +++ b/src/expr/lookup.c
> @@ -73,10 +73,13 @@ nftnl_expr_lookup_get(const struct nftnl_expr *e, uint16_t type,
>  		*data_len = sizeof(lookup->dreg);
>  		return &lookup->dreg;
>  	case NFTNL_EXPR_LOOKUP_SET:
> +		*data_len = strlen(lookup->set_name) + 1;
>  		return lookup->set_name;
>  	case NFTNL_EXPR_LOOKUP_SET_ID:
> +		*data_len = sizeof(lookup->set_id) + 1;
>  		return &lookup->set_id;
>  	case NFTNL_EXPR_LOOKUP_FLAGS:
> +		*data_len = sizeof(lookup->flags) + 1;

This + 1 here doesn't make any sense.

Please, send v3 without all these + 1.

And as I said in the previous version, you have to return strlen() as is.
--
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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v3, libnftnl] Fix nftnl_*_get to set data_len
  2016-07-11 11:48                   ` Pablo Neira Ayuso
@ 2016-07-11 16:07                     ` Carlos Falgueras García
  2016-07-11 17:18                       ` Pablo Neira Ayuso
  0 siblings, 1 reply; 19+ messages in thread
From: Carlos Falgueras García @ 2016-07-11 16:07 UTC (permalink / raw)
  To: netfilter-devel; +Cc: pablo

All getters must set the output parameter 'data_len'

Signed-off-by: Carlos Falgueras García <carlosfg@riseup.net>
---
 src/chain.c       | 3 +++
 src/expr.c        | 1 +
 src/expr/dynset.c | 3 +++
 src/expr/lookup.c | 3 +++
 src/gen.c         | 1 +
 src/rule.c        | 2 ++
 src/set.c         | 2 ++
 src/set_elem.c    | 6 ++++++
 src/table.c       | 1 +
 src/trace.c       | 6 +++---
 10 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/src/chain.c b/src/chain.c
index cab64b5..4c562fe 100644
--- a/src/chain.c
+++ b/src/chain.c
@@ -272,8 +272,10 @@ const void *nftnl_chain_get_data(const struct nftnl_chain *c, uint16_t attr,
 
 	switch(attr) {
 	case NFTNL_CHAIN_NAME:
+		*data_len = strlen(c->name) + 1;
 		return c->name;
 	case NFTNL_CHAIN_TABLE:
+		*data_len = strlen(c->table) + 1;
 		return c->table;
 	case NFTNL_CHAIN_HOOKNUM:
 		*data_len = sizeof(uint32_t);
@@ -303,6 +305,7 @@ const void *nftnl_chain_get_data(const struct nftnl_chain *c, uint16_t attr,
 		*data_len = sizeof(uint32_t);
 		return c->type;
 	case NFTNL_CHAIN_DEV:
+		*data_len = strlen(c->dev) + 1;
 		return c->dev;
 	}
 	return NULL;
diff --git a/src/expr.c b/src/expr.c
index f802725..e5c1dd3 100644
--- a/src/expr.c
+++ b/src/expr.c
@@ -119,6 +119,7 @@ const void *nftnl_expr_get(const struct nftnl_expr *expr,
 
 	switch(type) {
 	case NFTNL_EXPR_NAME:
+		*data_len = strlen(expr->ops->name) + 1;
 		ret = expr->ops->name;
 		break;
 	default:
diff --git a/src/expr/dynset.c b/src/expr/dynset.c
index 0404359..111bf8c 100644
--- a/src/expr/dynset.c
+++ b/src/expr/dynset.c
@@ -88,10 +88,13 @@ nftnl_expr_dynset_get(const struct nftnl_expr *e, uint16_t type,
 		*data_len = sizeof(dynset->timeout);
 		return &dynset->timeout;
 	case NFTNL_EXPR_DYNSET_SET_NAME:
+		*data_len = strlen(dynset->set_name) + 1;
 		return dynset->set_name;
 	case NFTNL_EXPR_DYNSET_SET_ID:
+		*data_len = sizeof(dynset->set_id);
 		return &dynset->set_id;
 	case NFTNL_EXPR_DYNSET_EXPR:
+		*data_len = 0;
 		return dynset->expr;
 	}
 	return NULL;
diff --git a/src/expr/lookup.c b/src/expr/lookup.c
index 7f68f74..16cfce2 100644
--- a/src/expr/lookup.c
+++ b/src/expr/lookup.c
@@ -73,10 +73,13 @@ nftnl_expr_lookup_get(const struct nftnl_expr *e, uint16_t type,
 		*data_len = sizeof(lookup->dreg);
 		return &lookup->dreg;
 	case NFTNL_EXPR_LOOKUP_SET:
+		*data_len = strlen(lookup->set_name) + 1;
 		return lookup->set_name;
 	case NFTNL_EXPR_LOOKUP_SET_ID:
+		*data_len = sizeof(lookup->set_id);
 		return &lookup->set_id;
 	case NFTNL_EXPR_LOOKUP_FLAGS:
+		*data_len = sizeof(lookup->flags);
 		return &lookup->flags;
 	}
 	return NULL;
diff --git a/src/gen.c b/src/gen.c
index 37a9049..c69d2f8 100644
--- a/src/gen.c
+++ b/src/gen.c
@@ -100,6 +100,7 @@ const void *nftnl_gen_get_data(const struct nftnl_gen *gen, uint16_t attr,
 
 	switch(attr) {
 	case NFTNL_GEN_ID:
+		*data_len = sizeof(gen->id);
 		return &gen->id;
 	}
 	return NULL;
diff --git a/src/rule.c b/src/rule.c
index 2b23c8e..a0edca7 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -213,8 +213,10 @@ const void *nftnl_rule_get_data(const struct nftnl_rule *r, uint16_t attr,
 		*data_len = sizeof(uint32_t);
 		return &r->family;
 	case NFTNL_RULE_TABLE:
+		*data_len = strlen(r->table) + 1;
 		return r->table;
 	case NFTNL_RULE_CHAIN:
+		*data_len = strlen(r->chain) + 1;
 		return r->chain;
 	case NFTNL_RULE_HANDLE:
 		*data_len = sizeof(uint64_t);
diff --git a/src/set.c b/src/set.c
index e48ff78..8a592db 100644
--- a/src/set.c
+++ b/src/set.c
@@ -215,8 +215,10 @@ const void *nftnl_set_get_data(const struct nftnl_set *s, uint16_t attr,
 
 	switch(attr) {
 	case NFTNL_SET_TABLE:
+		*data_len = strlen(s->table) + 1;
 		return s->table;
 	case NFTNL_SET_NAME:
+		*data_len = strlen(s->name) + 1;
 		return s->name;
 	case NFTNL_SET_FLAGS:
 		*data_len = sizeof(uint32_t);
diff --git a/src/set_elem.c b/src/set_elem.c
index 40b5bfe..4e89210 100644
--- a/src/set_elem.c
+++ b/src/set_elem.c
@@ -160,25 +160,31 @@ const void *nftnl_set_elem_get(struct nftnl_set_elem *s, uint16_t attr, uint32_t
 
 	switch(attr) {
 	case NFTNL_SET_ELEM_FLAGS:
+		*data_len = sizeof(s->set_elem_flags);
 		return &s->set_elem_flags;
 	case NFTNL_SET_ELEM_KEY:	/* NFTA_SET_ELEM_KEY */
 		*data_len = s->key.len;
 		return &s->key.val;
 	case NFTNL_SET_ELEM_VERDICT:	/* NFTA_SET_ELEM_DATA */
+		*data_len = sizeof(s->data.verdict);
 		return &s->data.verdict;
 	case NFTNL_SET_ELEM_CHAIN:	/* NFTA_SET_ELEM_DATA */
+		*data_len = strlen(s->data.chain) + 1;
 		return s->data.chain;
 	case NFTNL_SET_ELEM_DATA:	/* NFTA_SET_ELEM_DATA */
 		*data_len = s->data.len;
 		return &s->data.val;
 	case NFTNL_SET_ELEM_TIMEOUT:	/* NFTA_SET_ELEM_TIMEOUT */
+		*data_len = sizeof(s->timeout);
 		return &s->timeout;
 	case NFTNL_SET_ELEM_EXPIRATION:	/* NFTA_SET_ELEM_EXPIRATION */
+		*data_len = sizeof(s->expiration);
 		return &s->expiration;
 	case NFTNL_SET_ELEM_USERDATA:
 		*data_len = s->user.len;
 		return s->user.data;
 	case NFTNL_SET_ELEM_EXPR:
+		*data_len = 0;
 		return s->expr;
 	}
 	return NULL;
diff --git a/src/table.c b/src/table.c
index 966b923..3d4d7b9 100644
--- a/src/table.c
+++ b/src/table.c
@@ -143,6 +143,7 @@ const void *nftnl_table_get_data(const struct nftnl_table *t, uint16_t attr,
 
 	switch(attr) {
 	case NFTNL_TABLE_NAME:
+		*data_len = strlen(t->name) + 1;
 		return t->name;
 	case NFTNL_TABLE_FLAGS:
 		*data_len = sizeof(uint32_t);
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;
-- 
2.5.1

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v3, libnftnl] Fix nftnl_*_get to set data_len
  2016-07-11 16:07                     ` [PATCH v3, " Carlos Falgueras García
@ 2016-07-11 17:18                       ` Pablo Neira Ayuso
  0 siblings, 0 replies; 19+ messages in thread
From: Pablo Neira Ayuso @ 2016-07-11 17:18 UTC (permalink / raw)
  To: Carlos Falgueras García; +Cc: netfilter-devel

On Mon, Jul 11, 2016 at 06:07:40PM +0200, Carlos Falgueras García wrote:
> All getters must set the output parameter 'data_len'

Applied.

Carlos, I have enhanced this description. Please, include more
detailed justifications on your follow up patches. 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

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2016-07-11 17:19 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-27 16:24 [PATCH libnftnl] set: Fix nftnl_set_set_str Carlos Falgueras García
2016-06-27 16:29 ` Carlos Falgueras García
2016-07-01 14:22 ` Pablo Neira Ayuso
2016-07-01 16:07   ` [PATCH libnfntl v2] " Carlos Falgueras García
2016-07-01 16:11   ` [PATCH libnftnl] Fix string length calculations Carlos Falgueras García
2016-07-02  6:54     ` Pablo Neira Ayuso
2016-07-03 10:13       ` Carlos Falgueras García
2016-07-05 12:31         ` Pablo Neira Ayuso
2016-07-05 17:15           ` [PATCH 1/2 libnfntl] Fix nftnl_*_set_str Carlos Falgueras García
2016-07-05 17:15             ` [PATCH 2/2 libnfntl] Fix nftnl_*_get to set data_len Carlos Falgueras García
2016-07-06 15:21               ` Pablo Neira Ayuso
2016-07-11 11:41                 ` [PATCH v2, libnftnl] " Carlos Falgueras García
2016-07-11 11:48                   ` Pablo Neira Ayuso
2016-07-11 16:07                     ` [PATCH v3, " Carlos Falgueras García
2016-07-11 17:18                       ` Pablo Neira Ayuso
2016-07-11 10:24               ` [PATCH 2/2 libnfntl] " Pablo Neira Ayuso
2016-07-11 10:25                 ` Pablo Neira Ayuso
2016-07-06 15:19             ` [PATCH 1/2 libnfntl] Fix nftnl_*_set_str Pablo Neira Ayuso
2016-07-01 16:13   ` [PATCH libnftnl] set: Fix nftnl_set_set_str Carlos Falgueras García

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.