[libnftnl PATCH 0/5] queue: Do not print unset value

[libnftnl PATCH 0/5] queue: Do not print unset value

[Ana Rey]

This patchset does tasks about does not print unset value in xml and json file
and about refactoring code: "Add nft_rule_expr_queue_snprinf_* functions"
and "Use nft_rule_expr_* in the xml".

Also, It fixes some data type of variables.

Ana Rey (5):
  expr: queue: Add nft_rule_expr_queue_snprinf_* functions
  expr: queue: Use nft_rule_expr_* in the xml
  expr: queue: Do not print unset values in xml
  expr: queue: Do not print unset values in json
  expr: queue: Use the correct data type.

 src/expr/queue.c | 161 ++++++++++++++++++++++++++++++++++---------------------
 1 file changed, 101 insertions(+), 60 deletions(-)

-- 
2.0.0

[libnftnl PATCH 1/5] expr: queue: Add nft_rule_expr_queue_snprinf_* functions

[Ana Rey]

Code refactoring in nft_rule_expr_queue_snprinf functions. This process
adds three new functions:

* nft_rule_expr_queue_snprinf_default
* nft_rule_expr_queue_snprinf_xml
* nft_rule_expr_queue_snprinf_json

Also, It is delete an unnecesary whitespace as the CodingStyle recommends.

Signed-off-by: Ana Rey <anarey@gmail.com>
---
 src/expr/queue.c | 82 +++++++++++++++++++++++++++++++++++---------------------
 1 file changed, 51 insertions(+), 31 deletions(-)

diff --git a/src/expr/queue.c b/src/expr/queue.c
index 9f269e7..5dad84a 100644
--- a/src/expr/queue.c
+++ b/src/expr/queue.c
@@ -194,46 +194,66 @@ nft_rule_expr_queue_xml_parse(struct nft_rule_expr *e, mxml_node_t *tree,
 #endif
 }
 
-static int
-nft_rule_expr_queue_snprintf(char *buf, size_t len, uint32_t type,
-			      uint32_t flags, struct nft_rule_expr *e)
+static int nft_rule_expr_queue_snprintf_default(char *buf, size_t len,
+						struct nft_rule_expr *e)
 {
 	struct nft_expr_queue *queue = nft_expr_data(e);
 	int ret;
 	int one = 0;
 
+	ret = snprintf(buf, len, "num %u total %u", queue->queuenum,
+		       queue->queues_total);
+	if (queue->flags) {
+		ret += snprintf(buf + ret, len - ret, " options ");
+		if (queue->flags & NFT_QUEUE_FLAG_BYPASS) {
+			ret += snprintf(buf + ret, len - ret, "bypass");
+			one = 1;
+		}
+		if (queue->flags & NFT_QUEUE_FLAG_CPU_FANOUT) {
+			if (one)
+				ret += snprintf(buf + ret, len - ret, ",");
+			ret += snprintf(buf + ret, len - ret, "fanout");
+		}
+	}
+	return ret;
+}
+
+static int nft_rule_expr_queue_snprintf_xml(char *buf, size_t len,
+					    struct nft_rule_expr *e)
+{
+	struct nft_expr_queue *queue = nft_expr_data(e);
+
+	return snprintf(buf, len, "<num>%u</num>"
+				  "<total>%u</total>"
+				  "<flags>%u</flags>",
+			queue->queuenum, queue->queues_total,
+			queue->flags);
+}
+
+static int nft_rule_expr_queue_snprintf_json(char *buf, size_t len,
+					     struct nft_rule_expr *e)
+{
+	struct nft_expr_queue *queue = nft_expr_data(e);
+
+	return snprintf(buf, len, "\"num\":%u,"
+				  "\"total\":%u,"
+				  "\"flags\":%u,",
+			queue->queuenum, queue->queues_total,
+			queue->flags);
+}
+
+static int
+nft_rule_expr_queue_snprintf(char *buf, size_t len, uint32_t type,
+			      uint32_t flags, struct nft_rule_expr *e)
+{
+
 	switch(type) {
 	case NFT_OUTPUT_DEFAULT:
-		ret = snprintf(buf, len, "num %u total %u",
-				queue->queuenum, queue->queues_total);
-		if (queue->flags) {
-			ret += snprintf(buf + ret , len - ret, " options ");
-			if (queue->flags & NFT_QUEUE_FLAG_BYPASS) {
-				ret += snprintf(buf + ret ,
-						len - ret, "bypass");
-				one = 1;
-			}
-			if (queue->flags & NFT_QUEUE_FLAG_CPU_FANOUT) {
-				if (one)
-					ret += snprintf(buf + ret ,
-							len - ret, ",");
-				ret += snprintf(buf + ret ,
-						len - ret, "fanout");
-			}
-		}
-		return ret;
+		return nft_rule_expr_queue_snprintf_default(buf, len, e);
 	case NFT_OUTPUT_XML:
-		return snprintf(buf, len, "<num>%u</num>"
-					  "<total>%u</total>"
-					  "<flags>%u</flags>",
-				queue->queuenum, queue->queues_total,
-				queue->flags);
+		return nft_rule_expr_queue_snprintf_xml(buf, len, e);
 	case NFT_OUTPUT_JSON:
-		return snprintf(buf, len, "\"num\":%u,"
-					  "\"total\":%u,"
-					  "\"flags\":%u,",
-				queue->queuenum, queue->queues_total,
-				queue->flags);
+		return nft_rule_expr_queue_snprintf_json(buf, len, e);
 	default:
 		break;
 	}
-- 
2.0.0

[libnftnl PATCH 2/5] expr: queue: Use nft_rule_expr_* in the xml

[Ana Rey]

Code refactoring to use nft_rule_expr_* in parse xml functions.

Signed-off-by: Ana Rey <anarey@gmail.com>
---
 src/expr/queue.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/src/expr/queue.c b/src/expr/queue.c
index 5dad84a..1d47d48 100644
--- a/src/expr/queue.c
+++ b/src/expr/queue.c
@@ -164,28 +164,25 @@ nft_rule_expr_queue_xml_parse(struct nft_rule_expr *e, mxml_node_t *tree,
 			      struct nft_parse_err *err)
 {
 #ifdef XML_PARSING
-	struct nft_expr_queue *queue = nft_expr_data(e);
+	uint16_t queue_num, queue_total, flags;
 
 	if (nft_mxml_num_parse(tree, "num", MXML_DESCEND_FIRST, BASE_DEC,
-			       &queue->queuenum, NFT_TYPE_U16, NFT_XML_MAND,
+			       &queue_num, NFT_TYPE_U16, NFT_XML_MAND,
 			       err) != 0)
 		return -1;
-
-	e->flags |= (1 << NFT_EXPR_QUEUE_NUM);
+	nft_rule_expr_set_u16(e, NFT_EXPR_QUEUE_NUM, queue_num);
 
 	if (nft_mxml_num_parse(tree, "total", MXML_DESCEND_FIRST, BASE_DEC,
-			       &queue->queues_total, NFT_TYPE_U8,
+			       &queue_total, NFT_TYPE_U8,
 			       NFT_XML_MAND, err) != 0)
 		return -1;
-
-	e->flags |= (1 << NFT_EXPR_QUEUE_TOTAL);
+	nft_rule_expr_set_u16(e, NFT_EXPR_QUEUE_TOTAL, queue_total);
 
 	if (nft_mxml_num_parse(tree, "flags", MXML_DESCEND_FIRST, BASE_DEC,
-			       &queue->flags, NFT_TYPE_U8,
+			       &flags, NFT_TYPE_U8,
 			       NFT_XML_MAND, err) != 0)
 		return -1;
-
-	e->flags |= (1 << NFT_EXPR_QUEUE_FLAGS);
+	nft_rule_expr_set_u16(e, NFT_EXPR_QUEUE_FLAGS, flags);
 
 	return 0;
 #else
-- 
2.0.0

[libnftnl PATCH 3/5] expr: queue: Do not print unset values in xml

[Ana Rey]

It changes the parse and the snprint functions to omit unset values.

This xml file is gotten for a queue:

[...]
<expr type="queue">
	<num>0</num>
	<total>1</total>
	<flags>0</flags>
</expr>
[...]

Signed-off-by: Ana Rey <anarey@gmail.com>
---
 src/expr/queue.c | 39 +++++++++++++++++++++++++--------------
 1 file changed, 25 insertions(+), 14 deletions(-)

diff --git a/src/expr/queue.c b/src/expr/queue.c
index 1d47d48..5235027 100644
--- a/src/expr/queue.c
+++ b/src/expr/queue.c
@@ -168,21 +168,18 @@ nft_rule_expr_queue_xml_parse(struct nft_rule_expr *e, mxml_node_t *tree,
 
 	if (nft_mxml_num_parse(tree, "num", MXML_DESCEND_FIRST, BASE_DEC,
 			       &queue_num, NFT_TYPE_U16, NFT_XML_MAND,
-			       err) != 0)
-		return -1;
-	nft_rule_expr_set_u16(e, NFT_EXPR_QUEUE_NUM, queue_num);
+			       err) == 0)
+		nft_rule_expr_set_u16(e, NFT_EXPR_QUEUE_NUM, queue_num);
 
 	if (nft_mxml_num_parse(tree, "total", MXML_DESCEND_FIRST, BASE_DEC,
 			       &queue_total, NFT_TYPE_U8,
-			       NFT_XML_MAND, err) != 0)
-		return -1;
-	nft_rule_expr_set_u16(e, NFT_EXPR_QUEUE_TOTAL, queue_total);
+			       NFT_XML_MAND, err) == 0)
+		nft_rule_expr_set_u16(e, NFT_EXPR_QUEUE_TOTAL, queue_total);
 
 	if (nft_mxml_num_parse(tree, "flags", MXML_DESCEND_FIRST, BASE_DEC,
 			       &flags, NFT_TYPE_U8,
-			       NFT_XML_MAND, err) != 0)
-		return -1;
-	nft_rule_expr_set_u16(e, NFT_EXPR_QUEUE_FLAGS, flags);
+			       NFT_XML_MAND, err) == 0)
+		nft_rule_expr_set_u16(e, NFT_EXPR_QUEUE_FLAGS, flags);
 
 	return 0;
 #else
@@ -218,13 +215,27 @@ static int nft_rule_expr_queue_snprintf_default(char *buf, size_t len,
 static int nft_rule_expr_queue_snprintf_xml(char *buf, size_t len,
 					    struct nft_rule_expr *e)
 {
+	int ret, size = len, offset = 0;
 	struct nft_expr_queue *queue = nft_expr_data(e);
 
-	return snprintf(buf, len, "<num>%u</num>"
-				  "<total>%u</total>"
-				  "<flags>%u</flags>",
-			queue->queuenum, queue->queues_total,
-			queue->flags);
+
+	if (e->flags & (1 << NFT_EXPR_QUEUE_NUM)) {
+		ret = snprintf(buf + offset, len, "<num>%u</num>",
+			       queue->queuenum);
+		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+	}
+
+	if (e->flags & (1 << NFT_EXPR_QUEUE_TOTAL)) {
+		ret = snprintf(buf + offset, len, "<total>%u</total>",
+			       queue->queues_total);
+		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+	}
+	if (e->flags & (1 << NFT_EXPR_QUEUE_FLAGS)) {
+		ret = snprintf(buf + offset, len, "<flags>%u</flags>",
+			       queue->flags);
+		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+	}
+	return offset;
 }
 
 static int nft_rule_expr_queue_snprintf_json(char *buf, size_t len,
-- 
2.0.0

[libnftnl PATCH 4/5] expr: queue: Do not print unset values in json

[Ana Rey]

It changes the parse and the snprint functions to omit unset values.

This json file is gotten for a queue:

{"expr":[{"type":"queue","num":0,"total":1,"flags":0}]}

Signed-off-by: Ana Rey <anarey@gmail.com>
---
 src/expr/queue.c | 46 +++++++++++++++++++++++++++++-----------------
 1 file changed, 29 insertions(+), 17 deletions(-)

diff --git a/src/expr/queue.c b/src/expr/queue.c
index 5235027..55d2ab0 100644
--- a/src/expr/queue.c
+++ b/src/expr/queue.c
@@ -137,20 +137,14 @@ nft_rule_expr_queue_json_parse(struct nft_rule_expr *e, json_t *root,
 	uint32_t type;
 	uint16_t code;
 
-	if (nft_jansson_parse_val(root, "num", NFT_TYPE_U16, &type, err) < 0)
-		return -1;
-
-	nft_rule_expr_set_u32(e, NFT_EXPR_QUEUE_NUM, type);
-
-	if (nft_jansson_parse_val(root, "total", NFT_TYPE_U16, &code, err) < 0)
-		return -1;
-
-	nft_rule_expr_set_u16(e, NFT_EXPR_QUEUE_TOTAL, code);
+	if (nft_jansson_parse_val(root, "num", NFT_TYPE_U16, &type, err) == 0)
+		nft_rule_expr_set_u32(e, NFT_EXPR_QUEUE_NUM, type);
 
-	if (nft_jansson_parse_val(root, "flags", NFT_TYPE_U16, &code, err) < 0)
-		return -1;
+	if (nft_jansson_parse_val(root, "total", NFT_TYPE_U16, &code, err) == 0)
+		nft_rule_expr_set_u16(e, NFT_EXPR_QUEUE_TOTAL, code);
 
-	nft_rule_expr_set_u16(e, NFT_EXPR_QUEUE_FLAGS, code);
+	if (nft_jansson_parse_val(root, "flags", NFT_TYPE_U16, &code, err) == 0)
+		nft_rule_expr_set_u16(e, NFT_EXPR_QUEUE_FLAGS, code);
 
 	return 0;
 #else
@@ -241,13 +235,31 @@ static int nft_rule_expr_queue_snprintf_xml(char *buf, size_t len,
 static int nft_rule_expr_queue_snprintf_json(char *buf, size_t len,
 					     struct nft_rule_expr *e)
 {
+	int ret, size = len, offset = 0;
 	struct nft_expr_queue *queue = nft_expr_data(e);
 
-	return snprintf(buf, len, "\"num\":%u,"
-				  "\"total\":%u,"
-				  "\"flags\":%u,",
-			queue->queuenum, queue->queues_total,
-			queue->flags);
+	if (e->flags & (1 << NFT_EXPR_QUEUE_NUM)) {
+		ret = snprintf(buf + offset, len, "\"num\":%u,",
+			       queue->queuenum);
+		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+	}
+
+	if (e->flags & (1 << NFT_EXPR_QUEUE_TOTAL)) {
+		ret = snprintf(buf + offset, len, "\"total\":%u,",
+			       queue->queues_total);
+		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+	}
+	if (e->flags & (1 << NFT_EXPR_QUEUE_FLAGS)) {
+		ret = snprintf(buf + offset, len, "\"flags\":%u,",
+			       queue->flags);
+		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+	}
+
+	/* Remove the last comma characther */
+	if (offset > 0)
+		offset--;
+
+	return offset;
 }
 
 static int
-- 
2.0.0

[libnftnl PATCH 5/5] expr: queue: Use the correct data type.

[Ana Rey]

It changes some data type for their correct type.

Signed-off-by: Ana Rey <anarey@gmail.com>
---
 src/expr/queue.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/expr/queue.c b/src/expr/queue.c
index 55d2ab0..78540b6 100644
--- a/src/expr/queue.c
+++ b/src/expr/queue.c
@@ -134,11 +134,12 @@ nft_rule_expr_queue_json_parse(struct nft_rule_expr *e, json_t *root,
 			       struct nft_parse_err *err)
 {
 #ifdef JSON_PARSING
-	uint32_t type;
+	uint16_t type;
 	uint16_t code;
 
 	if (nft_jansson_parse_val(root, "num", NFT_TYPE_U16, &type, err) == 0)
-		nft_rule_expr_set_u32(e, NFT_EXPR_QUEUE_NUM, type);
+		nft_rule_expr_set_u16(e, NFT_EXPR_QUEUE_NUM, type);
+	nft_rule_expr_set_u16(e, NFT_EXPR_QUEUE_NUM, type);
 
 	if (nft_jansson_parse_val(root, "total", NFT_TYPE_U16, &code, err) == 0)
 		nft_rule_expr_set_u16(e, NFT_EXPR_QUEUE_TOTAL, code);
@@ -166,12 +167,12 @@ nft_rule_expr_queue_xml_parse(struct nft_rule_expr *e, mxml_node_t *tree,
 		nft_rule_expr_set_u16(e, NFT_EXPR_QUEUE_NUM, queue_num);
 
 	if (nft_mxml_num_parse(tree, "total", MXML_DESCEND_FIRST, BASE_DEC,
-			       &queue_total, NFT_TYPE_U8,
+			       &queue_total, NFT_TYPE_U16,
 			       NFT_XML_MAND, err) == 0)
 		nft_rule_expr_set_u16(e, NFT_EXPR_QUEUE_TOTAL, queue_total);
 
 	if (nft_mxml_num_parse(tree, "flags", MXML_DESCEND_FIRST, BASE_DEC,
-			       &flags, NFT_TYPE_U8,
+			       &flags, NFT_TYPE_U16,
 			       NFT_XML_MAND, err) == 0)
 		nft_rule_expr_set_u16(e, NFT_EXPR_QUEUE_FLAGS, flags);
 
-- 
2.0.0

Re: [libnftnl PATCH 1/5] expr: queue: Add nft_rule_expr_queue_snprinf_* functions

[Arturo Borrero Gonzalez]

On 13 June 2014 11:31, Ana Rey <anarey@gmail.com> wrote:
[...]
> diff --git a/src/expr/queue.c b/src/expr/queue.c
> index 9f269e7..5dad84a 100644
> --- a/src/expr/queue.c
> +++ b/src/expr/queue.c
> @@ -194,46 +194,66 @@ nft_rule_expr_queue_xml_parse(struct nft_rule_expr *e, mxml_node_t *tree,
>  #endif
>  }
>
> -static int
> -nft_rule_expr_queue_snprintf(char *buf, size_t len, uint32_t type,
> -                             uint32_t flags, struct nft_rule_expr *e)
> +static int nft_rule_expr_queue_snprintf_default(char *buf, size_t len,
> +                                               struct nft_rule_expr *e)

As I see this in my email client, It seems that this is not correctly
aligned. But I don't really know, as I'm reviewing without pushing the
patch to my stack.

>  {
>         struct nft_expr_queue *queue = nft_expr_data(e);
>         int ret;
>         int one = 0;
>
> +       ret = snprintf(buf, len, "num %u total %u", queue->queuenum,
> +                      queue->queues_total);
> +       if (queue->flags) {
> +               ret += snprintf(buf + ret, len - ret, " options ");
> +               if (queue->flags & NFT_QUEUE_FLAG_BYPASS) {
> +                       ret += snprintf(buf + ret, len - ret, "bypass");
> +                       one = 1;
> +               }
> +               if (queue->flags & NFT_QUEUE_FLAG_CPU_FANOUT) {
> +                       if (one)
> +                               ret += snprintf(buf + ret, len - ret, ",");
> +                       ret += snprintf(buf + ret, len - ret, "fanout");
> +               }
> +       }
> +       return ret;
> +}

I would recommend to use the SNPRINTF_BUFFER_SIZE macro, so we keep
this consistent with all the other code.

regards
-- 
Arturo Borrero González
--
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

Re: [libnftnl PATCH 0/5] queue: Do not print unset value

[Arturo Borrero Gonzalez]

On 13 June 2014 11:31, Ana Rey <anarey@gmail.com> wrote:
> This patchset does tasks about does not print unset value in xml and json file
> and about refactoring code: "Add nft_rule_expr_queue_snprinf_* functions"
> and "Use nft_rule_expr_* in the xml".
>
> Also, It fixes some data type of variables.
>
> Ana Rey (5):
>   expr: queue: Add nft_rule_expr_queue_snprinf_* functions
>   expr: queue: Use nft_rule_expr_* in the xml
>   expr: queue: Do not print unset values in xml
>   expr: queue: Do not print unset values in json
>   expr: queue: Use the correct data type.
>
>  src/expr/queue.c | 161 ++++++++++++++++++++++++++++++++++---------------------
>  1 file changed, 101 insertions(+), 60 deletions(-)
>

Hi Ana,

I've reviewed this series, and except the small thing in 1/5, they
seem OK to me.

thanks, regards.

-- 
Arturo Borrero González
--
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

Re: [libnftnl PATCH 0/5] queue: Do not print unset value

[Pablo Neira Ayuso]

On Fri, Jun 13, 2014 at 11:31:48AM +0200, Ana Rey wrote:
> This patchset does tasks about does not print unset value in xml and json file
> and about refactoring code: "Add nft_rule_expr_queue_snprinf_* functions"
> and "Use nft_rule_expr_* in the xml".
> 
> Also, It fixes some data type of variables.
> 
> Ana Rey (5):
>   expr: queue: Add nft_rule_expr_queue_snprinf_* functions
>   expr: queue: Use nft_rule_expr_* in the xml
>   expr: queue: Do not print unset values in xml
>   expr: queue: Do not print unset values in json
>   expr: queue: Use the correct data type.

Series applied, thanks Ana.