* [libnftnl PATCH 1/2] src/rule: Removed mandatory attribute printing in rules
@ 2014-03-09 12:59 Alvaro Neira Ayuso
2014-03-09 13:00 ` [libnftnl PATCH 2/2] src/rule: Changed parser support for having consistent with the new print support Alvaro Neira Ayuso
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Alvaro Neira Ayuso @ 2014-03-09 12:59 UTC (permalink / raw)
To: netfilter-devel
From: Álvaro Neira Ayuso <alvaroneay@gmail.com>
Before this patch, the program tried to print some attribute
that maybe the user hasn't defined for printing. We can't
assume that the user want to print some attribute that we have put
mandatory in the rules. Example:
Before this patch, it's mandatory have a rule with family
and this is the output:
{"rule":{"family":"ip","handle":4...
<rule><family>ip</family><handle>4</handle>...
Now, we can print rule without some attribute:
{"rule":{"handle":4...
<rule><handle>4</handle>...
Signed-off-by: Alvaro Neira Ayuso <alvaroneay@gmail.com>
---
src/rule.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++++--------
1 file changed, 79 insertions(+), 13 deletions(-)
diff --git a/src/rule.c b/src/rule.c
index 162a0a1..f0cafd3 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -775,12 +775,32 @@ static int nft_rule_snprintf_json(char *buf, size_t size, struct nft_rule *r,
int ret, len = size, offset = 0;
struct nft_rule_expr *expr;
- ret = snprintf(buf, len, "{\"rule\":{\"family\":\"%s\",\"table\":\"%s\","
- "\"chain\":\"%s\",\"handle\":%llu,",
- nft_family2str(r->family), r->table, r->chain,
- (unsigned long long)r->handle);
+ ret = snprintf(buf, len, "{\"rule\":{");
SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+ if (r->flags & (1 << NFT_RULE_ATTR_FAMILY)) {
+ ret = snprintf(buf+offset, len, "\"family\":\"%s\",",
+ nft_family2str(r->family));
+ SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+ }
+
+ if (r->flags & (1 << NFT_RULE_ATTR_TABLE)) {
+ ret = snprintf(buf+offset, len, "\"table\":\"%s\",",
+ r->table);
+ SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+ }
+
+ if (r->flags & (1 << NFT_RULE_ATTR_CHAIN)) {
+ ret = snprintf(buf+offset, len, "\"chain\":\"%s\",",
+ r->chain);
+ SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+ }
+ if (r->flags & (1 << NFT_RULE_ATTR_HANDLE)) {
+ ret = snprintf(buf+offset, len, "\"handle\":%llu,",
+ (unsigned long long)r->handle);
+ SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+ }
+
if (r->flags & (1 << NFT_RULE_ATTR_COMPAT_PROTO) ||
r->flags & (1 << NFT_RULE_ATTR_COMPAT_FLAGS)) {
ret = snprintf(buf+offset, len, "\"compat_flags\":%u,"
@@ -824,13 +844,32 @@ static int nft_rule_snprintf_xml(char *buf, size_t size, struct nft_rule *r,
int ret, len = size, offset = 0;
struct nft_rule_expr *expr;
- ret = snprintf(buf, len, "<rule><family>%s</family>"
- "<table>%s</table><chain>%s</chain>"
- "<handle>%llu</handle>",
- nft_family2str(r->family), r->table, r->chain,
- (unsigned long long)r->handle);
+ ret = snprintf(buf, len, "<rule>");
SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+ if (r->flags & (1 << NFT_RULE_ATTR_FAMILY)) {
+ ret = snprintf(buf+offset, len, "<family>%s</family>",
+ nft_family2str(r->family));
+ SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+ }
+
+ if (r->flags & (1 << NFT_RULE_ATTR_TABLE)) {
+ ret = snprintf(buf+offset, len, "<table>%s</table>",
+ r->table);
+ SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+ }
+
+ if (r->flags & (1 << NFT_RULE_ATTR_CHAIN)) {
+ ret = snprintf(buf+offset, len, "<chain>%s</chain>",
+ r->chain);
+ SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+ }
+ if (r->flags & (1 << NFT_RULE_ATTR_HANDLE)) {
+ ret = snprintf(buf+offset, len, "<handle>%llu</handle>",
+ (unsigned long long)r->handle);
+ SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+ }
+
if (r->compat.flags != 0 || r->compat.proto != 0) {
ret = snprintf(buf+offset, len,
"<compat_flags>%u</compat_flags>"
@@ -865,15 +904,42 @@ static int nft_rule_snprintf_xml(char *buf, size_t size, struct nft_rule *r,
return offset;
}
-static int nft_rule_snprintf_default(char *buf, size_t size, struct nft_rule *r,
+static int nft_rule_snprintf_default(char *buf, size_t size, struct nft_rule *r,
uint32_t type, uint32_t flags)
{
struct nft_rule_expr *expr;
int ret, len = size, offset = 0, i;
- ret = snprintf(buf, len, "%s %s %s %"PRIu64" %"PRIu64"\n",
- nft_family2str(r->family), r->table, r->chain,
- r->handle, r->position);
+ if (r->flags & (1 << NFT_RULE_ATTR_FAMILY)) {
+ ret = snprintf(buf+offset, len, "%s ",
+ nft_family2str(r->family));
+ SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+ }
+
+ if (r->flags & (1 << NFT_RULE_ATTR_TABLE)) {
+ ret = snprintf(buf+offset, len, "%s ",
+ r->table);
+ SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+ }
+
+ if (r->flags & (1 << NFT_RULE_ATTR_CHAIN)) {
+ ret = snprintf(buf+offset, len, "%s ",
+ r->chain);
+ SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+ }
+ if (r->flags & (1 << NFT_RULE_ATTR_TABLE)) {
+ ret = snprintf(buf+offset, len, "%llu ",
+ (unsigned long long)r->handle);
+ SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+ }
+
+ if (r->flags & (1 << NFT_RULE_ATTR_POSITION)) {
+ ret = snprintf(buf+offset, len, "%llu ",
+ (unsigned long long)r->position);
+ SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+ }
+
+ ret = snprintf(buf+offset, len, "\n");
SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
list_for_each_entry(expr, &r->expr_list, head) {
--
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] 7+ messages in thread
* [libnftnl PATCH 2/2] src/rule: Changed parser support for having consistent with the new print support
2014-03-09 12:59 [libnftnl PATCH 1/2] src/rule: Removed mandatory attribute printing in rules Alvaro Neira Ayuso
@ 2014-03-09 13:00 ` Alvaro Neira Ayuso
2014-03-11 20:16 ` Arturo Borrero Gonzalez
2014-03-09 13:14 ` [libnftnl PATCH 1/2] src/rule: Removed mandatory attribute printing in rules Arturo Borrero Gonzalez
2014-03-11 19:59 ` Arturo Borrero Gonzalez
2 siblings, 1 reply; 7+ messages in thread
From: Alvaro Neira Ayuso @ 2014-03-09 13:00 UTC (permalink / raw)
To: netfilter-devel
From: Álvaro Neira Ayuso <alvaroneay@gmail.com>
Signed-off-by: Alvaro Neira Ayuso <alvaroneay@gmail.com>
---
src/rule.c | 77 +++++++++++++++++++++++++++++++-----------------------------
1 file changed, 40 insertions(+), 37 deletions(-)
diff --git a/src/rule.c b/src/rule.c
index f0cafd3..e7335f8 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -540,28 +540,36 @@ int nft_jansson_parse_rule(struct nft_rule *r, json_t *tree,
if (root == NULL)
return -1;
- if (nft_jansson_parse_family(root, &family, err) != 0)
- goto err;
+ if (nft_jansson_node_exist(root, "family")) {
+ if (nft_jansson_parse_family(root, &family, err) != 0)
+ goto err;
nft_rule_attr_set_u32(r, NFT_RULE_ATTR_FAMILY, family);
+ }
- str = nft_jansson_parse_str(root, "table", err);
- if (str == NULL)
- goto err;
+ if (nft_jansson_node_exist(root, "table")) {
+ str = nft_jansson_parse_str(root, "table", err);
+ if (str == NULL)
+ goto err;
- nft_rule_attr_set_str(r, NFT_RULE_ATTR_TABLE, str);
+ nft_rule_attr_set_str(r, NFT_RULE_ATTR_TABLE, str);
+ }
- str = nft_jansson_parse_str(root, "chain", err);
- if (str == NULL)
- goto err;
+ if (nft_jansson_node_exist(root, "chain")) {
+ str = nft_jansson_parse_str(root, "chain", err);
+ if (str == NULL)
+ goto err;
- nft_rule_attr_set_str(r, NFT_RULE_ATTR_CHAIN, str);
+ nft_rule_attr_set_str(r, NFT_RULE_ATTR_CHAIN, str);
+ }
- if (nft_jansson_parse_val(root, "handle", NFT_TYPE_U64, &uval64,
- err) < 0)
- goto err;
+ if (nft_jansson_node_exist(root, "handle")) {
+ if (nft_jansson_parse_val(root, "handle", NFT_TYPE_U64, &uval64,
+ err) < 0)
+ goto err;
- nft_rule_attr_set_u64(r, NFT_RULE_ATTR_HANDLE, uval64);
+ nft_rule_attr_set_u64(r, NFT_RULE_ATTR_HANDLE, uval64);
+ }
if (nft_jansson_node_exist(root, "compat_proto") ||
nft_jansson_node_exist(root, "compat_flags")) {
@@ -640,39 +648,34 @@ int nft_mxml_rule_parse(mxml_node_t *tree, struct nft_rule *r,
family = nft_mxml_family_parse(tree, "family", MXML_DESCEND_FIRST,
NFT_XML_MAND, err);
- if (family < 0)
- return -1;
-
- r->family = family;
- r->flags |= (1 << NFT_RULE_ATTR_FAMILY);
+ if (family >= 0) {
+ r->family = family;
+ r->flags |= (1 << NFT_RULE_ATTR_FAMILY);
+ }
table = nft_mxml_str_parse(tree, "table", MXML_DESCEND_FIRST,
NFT_XML_MAND, err);
- if (table == NULL)
- return -1;
-
- if (r->table)
- xfree(r->table);
+ if (table != NULL) {
+ if (r->table)
+ xfree(r->table);
- r->table = strdup(table);
- r->flags |= (1 << NFT_RULE_ATTR_TABLE);
+ r->table = strdup(table);
+ r->flags |= (1 << NFT_RULE_ATTR_TABLE);
+ }
chain = nft_mxml_str_parse(tree, "chain", MXML_DESCEND_FIRST,
NFT_XML_MAND, err);
- if (chain == NULL)
- return -1;
-
- if (r->chain)
- xfree(r->chain);
+ if (chain != NULL) {
+ if (r->chain)
+ xfree(r->chain);
- r->chain = strdup(chain);
- r->flags |= (1 << NFT_RULE_ATTR_CHAIN);
+ r->chain = strdup(chain);
+ r->flags |= (1 << NFT_RULE_ATTR_CHAIN);
+ }
if (nft_mxml_num_parse(tree, "handle", MXML_DESCEND_FIRST, BASE_DEC,
- &r->handle, NFT_TYPE_U64, NFT_XML_MAND, err) != 0)
- return -1;
-
- r->flags |= (1 << NFT_RULE_ATTR_HANDLE);
+ &r->handle, NFT_TYPE_U64, NFT_XML_MAND, err) >= 0)
+ r->flags |= (1 << NFT_RULE_ATTR_HANDLE);
if (nft_mxml_num_parse(tree, "compat_proto", MXML_DESCEND_FIRST,
BASE_DEC, &r->compat.proto, NFT_TYPE_U32,
--
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] 7+ messages in thread
* Re: [libnftnl PATCH 1/2] src/rule: Removed mandatory attribute printing in rules
2014-03-09 12:59 [libnftnl PATCH 1/2] src/rule: Removed mandatory attribute printing in rules Alvaro Neira Ayuso
2014-03-09 13:00 ` [libnftnl PATCH 2/2] src/rule: Changed parser support for having consistent with the new print support Alvaro Neira Ayuso
@ 2014-03-09 13:14 ` Arturo Borrero Gonzalez
2014-03-12 12:50 ` Pablo Neira Ayuso
2014-03-11 19:59 ` Arturo Borrero Gonzalez
2 siblings, 1 reply; 7+ messages in thread
From: Arturo Borrero Gonzalez @ 2014-03-09 13:14 UTC (permalink / raw)
To: Alvaro Neira Ayuso; +Cc: Netfilter Development Mailing list, Pablo Neira Ayuso
On 9 March 2014 13:59, Alvaro Neira Ayuso <alvaroneay@gmail.com> wrote:
> From: Álvaro Neira Ayuso <alvaroneay@gmail.com>
>
> Before this patch, the program tried to print some attribute
> that maybe the user hasn't defined for printing. We can't
> assume that the user want to print some attribute that we have put
> mandatory in the rules. Example:
>
> Before this patch, it's mandatory have a rule with family
> and this is the output:
>
> {"rule":{"family":"ip","handle":4...
> <rule><family>ip</family><handle>4</handle>...
>
> Now, we can print rule without some attribute:
>
> {"rule":{"handle":4...
> <rule><handle>4</handle>...
>
> Signed-off-by: Alvaro Neira Ayuso <alvaroneay@gmail.com>
> ---
Hi there!
These attributes are no longer mandatory? What is the idea behind this?
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [libnftnl PATCH 1/2] src/rule: Removed mandatory attribute printing in rules
2014-03-09 12:59 [libnftnl PATCH 1/2] src/rule: Removed mandatory attribute printing in rules Alvaro Neira Ayuso
2014-03-09 13:00 ` [libnftnl PATCH 2/2] src/rule: Changed parser support for having consistent with the new print support Alvaro Neira Ayuso
2014-03-09 13:14 ` [libnftnl PATCH 1/2] src/rule: Removed mandatory attribute printing in rules Arturo Borrero Gonzalez
@ 2014-03-11 19:59 ` Arturo Borrero Gonzalez
2 siblings, 0 replies; 7+ messages in thread
From: Arturo Borrero Gonzalez @ 2014-03-11 19:59 UTC (permalink / raw)
To: Alvaro Neira Ayuso; +Cc: Netfilter Development Mailing list
Hi Alvaro,
I found a small issue in your patch, see below.
Except for this, the patch looks good to me.
On 9 March 2014 13:59, Alvaro Neira Ayuso <alvaroneay@gmail.com> wrote:
> @@ -865,15 +904,42 @@ static int nft_rule_snprintf_xml(char *buf, size_t size, struct nft_rule *r,
> return offset;
> }
>
> -static int nft_rule_snprintf_default(char *buf, size_t size, struct nft_rule *r,
> +static int nft_rule_snprintf_default(char *buf, size_t size, struct nft_rule *r,
> uint32_t type, uint32_t flags)
> {
> struct nft_rule_expr *expr;
> int ret, len = size, offset = 0, i;
>
> - ret = snprintf(buf, len, "%s %s %s %"PRIu64" %"PRIu64"\n",
> - nft_family2str(r->family), r->table, r->chain,
> - r->handle, r->position);
> + if (r->flags & (1 << NFT_RULE_ATTR_FAMILY)) {
> + ret = snprintf(buf+offset, len, "%s ",
> + nft_family2str(r->family));
> + SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
> + }
> +
> + if (r->flags & (1 << NFT_RULE_ATTR_TABLE)) {
> + ret = snprintf(buf+offset, len, "%s ",
> + r->table);
> + SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
> + }
> +
> + if (r->flags & (1 << NFT_RULE_ATTR_CHAIN)) {
> + ret = snprintf(buf+offset, len, "%s ",
> + r->chain);
> + SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
> + }
> + if (r->flags & (1 << NFT_RULE_ATTR_TABLE)) {
> + ret = snprintf(buf+offset, len, "%llu ",
> + (unsigned long long)r->handle);
> + SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
> + }
Here you check the table attr but print handle.
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [libnftnl PATCH 2/2] src/rule: Changed parser support for having consistent with the new print support
2014-03-09 13:00 ` [libnftnl PATCH 2/2] src/rule: Changed parser support for having consistent with the new print support Alvaro Neira Ayuso
@ 2014-03-11 20:16 ` Arturo Borrero Gonzalez
2014-03-12 11:37 ` Álvaro Neira Ayuso
0 siblings, 1 reply; 7+ messages in thread
From: Arturo Borrero Gonzalez @ 2014-03-11 20:16 UTC (permalink / raw)
To: Alvaro Neira Ayuso; +Cc: Netfilter Development Mailing list
Hi Alvaro,
a few coments below about this patch.
On 9 March 2014 14:00, Alvaro Neira Ayuso <alvaroneay@gmail.com> wrote:
> From: Álvaro Neira Ayuso <alvaroneay@gmail.com>
>
> Signed-off-by: Alvaro Neira Ayuso <alvaroneay@gmail.com>
> ---
> src/rule.c | 77 +++++++++++++++++++++++++++++++-----------------------------
> 1 file changed, 40 insertions(+), 37 deletions(-)
>
I would suggest to describe this parser change.
> diff --git a/src/rule.c b/src/rule.c
> index f0cafd3..e7335f8 100644
> --- a/src/rule.c
> +++ b/src/rule.c
> @@ -540,28 +540,36 @@ int nft_jansson_parse_rule(struct nft_rule *r, json_t *tree,
> if (root == NULL)
> return -1;
>
> - if (nft_jansson_parse_family(root, &family, err) != 0)
> - goto err;
> + if (nft_jansson_node_exist(root, "family")) {
> + if (nft_jansson_parse_family(root, &family, err) != 0)
> + goto err;
>
> nft_rule_attr_set_u32(r, NFT_RULE_ATTR_FAMILY, family);
> + }
>
Fix indentation. The nft_rule_attr_set_u32() is now inside the if statement.
> - if (nft_jansson_parse_val(root, "handle", NFT_TYPE_U64, &uval64,
> - err) < 0)
> - goto err;
> + if (nft_jansson_node_exist(root, "handle")) {
> + if (nft_jansson_parse_val(root, "handle", NFT_TYPE_U64, &uval64,
> + err) < 0)
Mixed tab/spaces indentation before 'err) < 0)'
> @@ -640,39 +648,34 @@ int nft_mxml_rule_parse(mxml_node_t *tree, struct nft_rule *r,
>
> family = nft_mxml_family_parse(tree, "family", MXML_DESCEND_FIRST,
> NFT_XML_MAND, err);
> - if (family < 0)
> - return -1;
> -
> - r->family = family;
> - r->flags |= (1 << NFT_RULE_ATTR_FAMILY);
> + if (family >= 0) {
> + r->family = family;
> + r->flags |= (1 << NFT_RULE_ATTR_FAMILY);
> + }
>
I would suggest to, while at it, switch to nft_rule_attr_set_xx() functions.
IMHO, we will save some LOCs, among other benefits.
> - r->table = strdup(table);
> - r->flags |= (1 << NFT_RULE_ATTR_TABLE);
> + r->table = strdup(table);
> + r->flags |= (1 << NFT_RULE_ATTR_TABLE);
> + }
Just for completeness:
remember that for string attributes, the strdup() is done internally,
inside nft_rule_attr_set_str().
> if (nft_mxml_num_parse(tree, "handle", MXML_DESCEND_FIRST, BASE_DEC,
> - &r->handle, NFT_TYPE_U64, NFT_XML_MAND, err) != 0)
> - return -1;
> -
> - r->flags |= (1 << NFT_RULE_ATTR_HANDLE);
> + &r->handle, NFT_TYPE_U64, NFT_XML_MAND, err) >= 0)
The line above is 82 characters long.
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [libnftnl PATCH 2/2] src/rule: Changed parser support for having consistent with the new print support
2014-03-11 20:16 ` Arturo Borrero Gonzalez
@ 2014-03-12 11:37 ` Álvaro Neira Ayuso
0 siblings, 0 replies; 7+ messages in thread
From: Álvaro Neira Ayuso @ 2014-03-12 11:37 UTC (permalink / raw)
To: Arturo Borrero Gonzalez; +Cc: Netfilter Development Mailing list
El 11/03/14 21:16, Arturo Borrero Gonzalez escribió:
> Hi Alvaro,
>
> a few coments below about this patch.
>
> On 9 March 2014 14:00, Alvaro Neira Ayuso <alvaroneay@gmail.com> wrote:
>> From: Álvaro Neira Ayuso <alvaroneay@gmail.com>
>>
>> Signed-off-by: Alvaro Neira Ayuso <alvaroneay@gmail.com>
>> ---
>> src/rule.c | 77 +++++++++++++++++++++++++++++++-----------------------------
>> 1 file changed, 40 insertions(+), 37 deletions(-)
>>
>
> I would suggest to describe this parser change.
>
>> diff --git a/src/rule.c b/src/rule.c
>> index f0cafd3..e7335f8 100644
>> --- a/src/rule.c
>> +++ b/src/rule.c
>> @@ -540,28 +540,36 @@ int nft_jansson_parse_rule(struct nft_rule *r, json_t *tree,
>> if (root == NULL)
>> return -1;
>>
>> - if (nft_jansson_parse_family(root, &family, err) != 0)
>> - goto err;
>> + if (nft_jansson_node_exist(root, "family")) {
>> + if (nft_jansson_parse_family(root, &family, err) != 0)
>> + goto err;
>>
>> nft_rule_attr_set_u32(r, NFT_RULE_ATTR_FAMILY, family);
>> + }
>>
>
> Fix indentation. The nft_rule_attr_set_u32() is now inside the if statement.
>
>
>> - if (nft_jansson_parse_val(root, "handle", NFT_TYPE_U64, &uval64,
>> - err) < 0)
>> - goto err;
>> + if (nft_jansson_node_exist(root, "handle")) {
>> + if (nft_jansson_parse_val(root, "handle", NFT_TYPE_U64, &uval64,
>> + err) < 0)
>
> Mixed tab/spaces indentation before 'err) < 0)'
>
>
>> @@ -640,39 +648,34 @@ int nft_mxml_rule_parse(mxml_node_t *tree, struct nft_rule *r,
>>
>> family = nft_mxml_family_parse(tree, "family", MXML_DESCEND_FIRST,
>> NFT_XML_MAND, err);
>> - if (family < 0)
>> - return -1;
>> -
>> - r->family = family;
>> - r->flags |= (1 << NFT_RULE_ATTR_FAMILY);
>> + if (family >= 0) {
>> + r->family = family;
>> + r->flags |= (1 << NFT_RULE_ATTR_FAMILY);
>> + }
>>
>
> I would suggest to, while at it, switch to nft_rule_attr_set_xx() functions.
> IMHO, we will save some LOCs, among other benefits.
Yes, i have put this like that for following the same format that you
have had. The next time, i'm going to ask before :D. I'm going to change
that.
>
>> - r->table = strdup(table);
>> - r->flags |= (1 << NFT_RULE_ATTR_TABLE);
>> + r->table = strdup(table);
>> + r->flags |= (1 << NFT_RULE_ATTR_TABLE);
>> + }
>
> Just for completeness:
> remember that for string attributes, the strdup() is done internally,
> inside nft_rule_attr_set_str().
I know that ^^.
>
>> if (nft_mxml_num_parse(tree, "handle", MXML_DESCEND_FIRST, BASE_DEC,
>> - &r->handle, NFT_TYPE_U64, NFT_XML_MAND, err) != 0)
>> - return -1;
>> -
>> - r->flags |= (1 << NFT_RULE_ATTR_HANDLE);
>> + &r->handle, NFT_TYPE_U64, NFT_XML_MAND, err) >= 0)
>
> The line above is 82 characters long.
Thanks Arturo for helping me. I'm going to fix all the stuff.
Regards
Álvaro
--
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] 7+ messages in thread
* Re: [libnftnl PATCH 1/2] src/rule: Removed mandatory attribute printing in rules
2014-03-09 13:14 ` [libnftnl PATCH 1/2] src/rule: Removed mandatory attribute printing in rules Arturo Borrero Gonzalez
@ 2014-03-12 12:50 ` Pablo Neira Ayuso
0 siblings, 0 replies; 7+ messages in thread
From: Pablo Neira Ayuso @ 2014-03-12 12:50 UTC (permalink / raw)
To: Arturo Borrero Gonzalez
Cc: Alvaro Neira Ayuso, Netfilter Development Mailing list
On Sun, Mar 09, 2014 at 02:14:23PM +0100, Arturo Borrero Gonzalez wrote:
> On 9 March 2014 13:59, Alvaro Neira Ayuso <alvaroneay@gmail.com> wrote:
> > From: Álvaro Neira Ayuso <alvaroneay@gmail.com>
> >
> > Before this patch, the program tried to print some attribute
> > that maybe the user hasn't defined for printing. We can't
> > assume that the user want to print some attribute that we have put
> > mandatory in the rules. Example:
> >
> > Before this patch, it's mandatory have a rule with family
> > and this is the output:
> >
> > {"rule":{"family":"ip","handle":4...
> > <rule><family>ip</family><handle>4</handle>...
> >
> > Now, we can print rule without some attribute:
> >
> > {"rule":{"handle":4...
> > <rule><handle>4</handle>...
> >
> > Signed-off-by: Alvaro Neira Ayuso <alvaroneay@gmail.com>
> > ---
>
> Hi there!
>
> These attributes are no longer mandatory? What is the idea behind this?
I think we already discussed this time ago.
I'd like that the parsers don't enforce the occurrence of any
attribute, we should just let the kernel bail out if the configuration
that the user provides doesn't make sense.
This provides us more flexibility in case that we change any aspect
from the kernel side.
--
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] 7+ messages in thread
end of thread, other threads:[~2014-03-12 12:50 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-03-09 12:59 [libnftnl PATCH 1/2] src/rule: Removed mandatory attribute printing in rules Alvaro Neira Ayuso
2014-03-09 13:00 ` [libnftnl PATCH 2/2] src/rule: Changed parser support for having consistent with the new print support Alvaro Neira Ayuso
2014-03-11 20:16 ` Arturo Borrero Gonzalez
2014-03-12 11:37 ` Álvaro Neira Ayuso
2014-03-09 13:14 ` [libnftnl PATCH 1/2] src/rule: Removed mandatory attribute printing in rules Arturo Borrero Gonzalez
2014-03-12 12:50 ` Pablo Neira Ayuso
2014-03-11 19:59 ` Arturo Borrero Gonzalez
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).