* [PATCH v2 0/2] Do not print unset value in xml file @ 2014-06-02 10:11 Ana Rey 2014-06-02 10:11 ` [PATCH v2 1/2] src: expr: log: Code refactoring to use nft_rule_expr_set_* functions Ana Rey 2014-06-02 10:11 ` [PATCH v2 2/2] src: expr: log: Do not print unset values in xml Ana Rey 0 siblings, 2 replies; 5+ messages in thread From: Ana Rey @ 2014-06-02 10:11 UTC (permalink / raw) To: netfilter-devel; +Cc: Ana Rey Hi, After some advises of Arturo Borrero, I send a previous patch to 'Code refactoring to use nft_rule_expr_set_* functions' patch and v2 of "Do not print unset values in xml" patch. Ana Rey (2): src: expr: log: Code refactoring to use nft_rule_expr_set_* functions src: expr: log: Do not print unset values in xml. src/expr/log.c | 81 ++++++++++++++++++++++++++++++++++------------------------ 1 file changed, 47 insertions(+), 34 deletions(-) -- 2.0.0.rc2 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 1/2] src: expr: log: Code refactoring to use nft_rule_expr_set_* functions 2014-06-02 10:11 [PATCH v2 0/2] Do not print unset value in xml file Ana Rey @ 2014-06-02 10:11 ` Ana Rey 2014-06-02 10:19 ` Pablo Neira Ayuso 2014-06-02 10:11 ` [PATCH v2 2/2] src: expr: log: Do not print unset values in xml Ana Rey 1 sibling, 1 reply; 5+ messages in thread From: Ana Rey @ 2014-06-02 10:11 UTC (permalink / raw) To: netfilter-devel; +Cc: Ana Rey Code refactoring to use nft_rule_expr_set_* in parse functions. This change was supported by Arturo Borrero Gonzalez. Signed-off-by: Ana Rey <anarey@gmail.com> --- src/expr/log.c | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/src/expr/log.c b/src/expr/log.c index a61a8d3..bf18b42 100644 --- a/src/expr/log.c +++ b/src/expr/log.c @@ -133,6 +133,9 @@ nft_rule_expr_log_parse(struct nft_rule_expr *e, struct nlattr *attr) { struct nft_expr_log *log = nft_expr_data(e); struct nlattr *tb[NFTA_LOG_MAX+1] = {}; + const char *prefix; + uint32_t snaplen; + uint16_t uval16; if (mnl_attr_parse_nested(attr, nft_rule_expr_log_cb, tb) < 0) return -1; @@ -141,20 +144,20 @@ nft_rule_expr_log_parse(struct nft_rule_expr *e, struct nlattr *attr) if (log->prefix) xfree(log->prefix); - log->prefix = strdup(mnl_attr_get_str(tb[NFTA_LOG_PREFIX])); - e->flags |= (1 << NFT_EXPR_LOG_PREFIX); + prefix = strdup(mnl_attr_get_str(tb[NFTA_LOG_PREFIX])); + nft_rule_expr_set_str(e, NFT_EXPR_LOG_PREFIX, prefix); } if (tb[NFTA_LOG_GROUP]) { - log->group = ntohs(mnl_attr_get_u16(tb[NFTA_LOG_GROUP])); - e->flags |= (1 << NFT_EXPR_LOG_GROUP); + uval16 = ntohs(mnl_attr_get_u16(tb[NFTA_LOG_GROUP])); + nft_rule_expr_set_u16(e, NFT_EXPR_LOG_GROUP, uval16); } if (tb[NFTA_LOG_SNAPLEN]) { - log->snaplen = ntohl(mnl_attr_get_u32(tb[NFTA_LOG_SNAPLEN])); - e->flags |= (1 << NFT_EXPR_LOG_SNAPLEN); + snaplen = ntohl(mnl_attr_get_u32(tb[NFTA_LOG_SNAPLEN])); + nft_rule_expr_set_u32(e, NFT_EXPR_LOG_SNAPLEN, snaplen); } if (tb[NFTA_LOG_QTHRESHOLD]) { - log->qthreshold = ntohs(mnl_attr_get_u16(tb[NFTA_LOG_QTHRESHOLD])); - e->flags |= (1 << NFT_EXPR_LOG_QTHRESHOLD); + uval16 = ntohs(mnl_attr_get_u16(tb[NFTA_LOG_QTHRESHOLD])); + nft_rule_expr_set_u16(e, NFT_EXPR_LOG_QTHRESHOLD, uval16); } return 0; @@ -211,30 +214,25 @@ static int nft_rule_expr_log_xml_parse(struct nft_rule_expr *e, NFT_XML_MAND, err); if (prefix == NULL) return -1; - - log->prefix = strdup(prefix); - e->flags |= (1 << NFT_EXPR_LOG_PREFIX); + nft_rule_expr_set_str(e, NFT_EXPR_LOG_PREFIX, prefix); if (nft_mxml_num_parse(tree, "group", MXML_DESCEND_FIRST, BASE_DEC, &log->group, NFT_TYPE_U16, NFT_XML_MAND, err) != 0) return -1; - - e->flags |= (1 << NFT_EXPR_LOG_GROUP); + nft_rule_expr_set_u16(e, NFT_EXPR_LOG_GROUP, log->group); if (nft_mxml_num_parse(tree, "snaplen", MXML_DESCEND_FIRST, BASE_DEC, &log->snaplen, NFT_TYPE_U32, NFT_XML_MAND, err) != 0) return -1; - - e->flags |= (1 << NFT_EXPR_LOG_SNAPLEN); + nft_rule_expr_set_u32(e, NFT_EXPR_LOG_SNAPLEN, log->snaplen); if (nft_mxml_num_parse(tree, "qthreshold", MXML_DESCEND_FIRST, BASE_DEC, &log->qthreshold, NFT_TYPE_U16, NFT_XML_MAND, err) != 0) return -1; - - e->flags |= (1 << NFT_EXPR_LOG_QTHRESHOLD); + nft_rule_expr_set_u16(e, NFT_EXPR_LOG_QTHRESHOLD, log->qthreshold); return 0; #else -- 2.0.0.rc2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/2] src: expr: log: Code refactoring to use nft_rule_expr_set_* functions 2014-06-02 10:11 ` [PATCH v2 1/2] src: expr: log: Code refactoring to use nft_rule_expr_set_* functions Ana Rey @ 2014-06-02 10:19 ` Pablo Neira Ayuso 0 siblings, 0 replies; 5+ messages in thread From: Pablo Neira Ayuso @ 2014-06-02 10:19 UTC (permalink / raw) To: Ana Rey; +Cc: netfilter-devel Hi Ana, No need to include "src: " in the tag, "expr: log: ..." should be enough. The "src: " is sort of wildcard, when the change applies to different aspects of the tree. On Mon, Jun 02, 2014 at 12:11:32PM +0200, Ana Rey wrote: > Code refactoring to use nft_rule_expr_set_* in parse functions. > > This change was supported by Arturo Borrero Gonzalez. ^-------^ suggested Or you could just include: Suggested-by: Arturo Borrero Gonzalez <...> > Signed-off-by: Ana Rey <anarey@gmail.com> > --- > src/expr/log.c | 32 +++++++++++++++----------------- > 1 file changed, 15 insertions(+), 17 deletions(-) > > diff --git a/src/expr/log.c b/src/expr/log.c > index a61a8d3..bf18b42 100644 > --- a/src/expr/log.c > +++ b/src/expr/log.c > @@ -133,6 +133,9 @@ nft_rule_expr_log_parse(struct nft_rule_expr *e, struct nlattr *attr) > { > struct nft_expr_log *log = nft_expr_data(e); > struct nlattr *tb[NFTA_LOG_MAX+1] = {}; > + const char *prefix; > + uint32_t snaplen; > + uint16_t uval16; Please, no uval16 or tmp variable names for code readability reasons. > > if (mnl_attr_parse_nested(attr, nft_rule_expr_log_cb, tb) < 0) > return -1; > @@ -141,20 +144,20 @@ nft_rule_expr_log_parse(struct nft_rule_expr *e, struct nlattr *attr) > if (log->prefix) > xfree(log->prefix); > > - log->prefix = strdup(mnl_attr_get_str(tb[NFTA_LOG_PREFIX])); > - e->flags |= (1 << NFT_EXPR_LOG_PREFIX); > + prefix = strdup(mnl_attr_get_str(tb[NFTA_LOG_PREFIX])); > + nft_rule_expr_set_str(e, NFT_EXPR_LOG_PREFIX, prefix); > } > if (tb[NFTA_LOG_GROUP]) { > - log->group = ntohs(mnl_attr_get_u16(tb[NFTA_LOG_GROUP])); > - e->flags |= (1 << NFT_EXPR_LOG_GROUP); > + uval16 = ntohs(mnl_attr_get_u16(tb[NFTA_LOG_GROUP])); I'd suggest here: if (tb[NFTA_LOG_GROUP]) { uint16_t group = ntohs(mnl_attr_get_u16(tb[NFTA_LOG_GROUP])); nft_rule_expr_set_u16(e, NFT_EXPR_LOG_GROUP, group); } > + nft_rule_expr_set_u16(e, NFT_EXPR_LOG_GROUP, uval16); > } > if (tb[NFTA_LOG_SNAPLEN]) { > - log->snaplen = ntohl(mnl_attr_get_u32(tb[NFTA_LOG_SNAPLEN])); > - e->flags |= (1 << NFT_EXPR_LOG_SNAPLEN); > + snaplen = ntohl(mnl_attr_get_u32(tb[NFTA_LOG_SNAPLEN])); > + nft_rule_expr_set_u32(e, NFT_EXPR_LOG_SNAPLEN, snaplen); > } > if (tb[NFTA_LOG_QTHRESHOLD]) { > - log->qthreshold = ntohs(mnl_attr_get_u16(tb[NFTA_LOG_QTHRESHOLD])); > - e->flags |= (1 << NFT_EXPR_LOG_QTHRESHOLD); > + uval16 = ntohs(mnl_attr_get_u16(tb[NFTA_LOG_QTHRESHOLD])); > + nft_rule_expr_set_u16(e, NFT_EXPR_LOG_QTHRESHOLD, uval16); > } > > return 0; > @@ -211,30 +214,25 @@ static int nft_rule_expr_log_xml_parse(struct nft_rule_expr *e, > NFT_XML_MAND, err); > if (prefix == NULL) > return -1; > - > - log->prefix = strdup(prefix); > - e->flags |= (1 << NFT_EXPR_LOG_PREFIX); > + nft_rule_expr_set_str(e, NFT_EXPR_LOG_PREFIX, prefix); > > if (nft_mxml_num_parse(tree, "group", MXML_DESCEND_FIRST, BASE_DEC, > &log->group, NFT_TYPE_U16, NFT_XML_MAND, > err) != 0) > return -1; > - > - e->flags |= (1 << NFT_EXPR_LOG_GROUP); > + nft_rule_expr_set_u16(e, NFT_EXPR_LOG_GROUP, log->group); > > if (nft_mxml_num_parse(tree, "snaplen", MXML_DESCEND_FIRST, BASE_DEC, > &log->snaplen, NFT_TYPE_U32, NFT_XML_MAND, > err) != 0) > return -1; > - > - e->flags |= (1 << NFT_EXPR_LOG_SNAPLEN); > + nft_rule_expr_set_u32(e, NFT_EXPR_LOG_SNAPLEN, log->snaplen); > > if (nft_mxml_num_parse(tree, "qthreshold", MXML_DESCEND_FIRST, > BASE_DEC, &log->qthreshold, > NFT_TYPE_U16, NFT_XML_MAND, err) != 0) > return -1; > - > - e->flags |= (1 << NFT_EXPR_LOG_QTHRESHOLD); > + nft_rule_expr_set_u16(e, NFT_EXPR_LOG_QTHRESHOLD, log->qthreshold); > > return 0; > #else > -- > 2.0.0.rc2 > > -- > 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] 5+ messages in thread
* [PATCH v2 2/2] src: expr: log: Do not print unset values in xml. 2014-06-02 10:11 [PATCH v2 0/2] Do not print unset value in xml file Ana Rey 2014-06-02 10:11 ` [PATCH v2 1/2] src: expr: log: Code refactoring to use nft_rule_expr_set_* functions Ana Rey @ 2014-06-02 10:11 ` Ana Rey 2014-06-02 10:21 ` Pablo Neira Ayuso 1 sibling, 1 reply; 5+ messages in thread From: Ana Rey @ 2014-06-02 10:11 UTC (permalink / raw) To: netfilter-devel; +Cc: Ana Rey It changes the parse and the snprint functions to omit unset values. If we used this rule: ntt add rule ip test output log We got this xml file: <rule><family>ip</family> <table>test</table> <chain>output</chain> <handle>88</handle> <expr type="log"> <prefix>(null)</prefix> <group>0</group> <snaplen>0</snaplen> <qthreshold>0</qthreshold> </expr> </rule> And It was imposible import this file. Now, That rule creates this xml file without null values: <rule><family>ip</family> <table>test</table> <chain>output</chain> <handle>88</handle> <expr type="log"> </expr> </rule> and It's possible import this xml file. Signed-off-by: Ana Rey <anarey@gmail.com> --- [Changes in v2] This patch has some changes that derive from the previous "src: expr: log: Code refactoring to use nft_rule_expr_set_* functions" patch. src/expr/log.c | 57 ++++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 36 insertions(+), 21 deletions(-) diff --git a/src/expr/log.c b/src/expr/log.c index bf18b42..34c939f 100644 --- a/src/expr/log.c +++ b/src/expr/log.c @@ -212,27 +212,25 @@ static int nft_rule_expr_log_xml_parse(struct nft_rule_expr *e, prefix = nft_mxml_str_parse(tree, "prefix", MXML_DESCEND_FIRST, NFT_XML_MAND, err); - if (prefix == NULL) - return -1; - nft_rule_expr_set_str(e, NFT_EXPR_LOG_PREFIX, prefix); + if (prefix != NULL) + nft_rule_expr_set_str(e, NFT_EXPR_LOG_PREFIX, prefix); if (nft_mxml_num_parse(tree, "group", MXML_DESCEND_FIRST, BASE_DEC, &log->group, NFT_TYPE_U16, NFT_XML_MAND, - err) != 0) - return -1; - nft_rule_expr_set_u16(e, NFT_EXPR_LOG_GROUP, log->group); + err) == 0) + nft_rule_expr_set_u16(e, NFT_EXPR_LOG_GROUP, log->group); if (nft_mxml_num_parse(tree, "snaplen", MXML_DESCEND_FIRST, BASE_DEC, &log->snaplen, NFT_TYPE_U32, NFT_XML_MAND, - err) != 0) - return -1; - nft_rule_expr_set_u32(e, NFT_EXPR_LOG_SNAPLEN, log->snaplen); - - if (nft_mxml_num_parse(tree, "qthreshold", MXML_DESCEND_FIRST, - BASE_DEC, &log->qthreshold, - NFT_TYPE_U16, NFT_XML_MAND, err) != 0) - return -1; - nft_rule_expr_set_u16(e, NFT_EXPR_LOG_QTHRESHOLD, log->qthreshold); + err) == 0) + nft_rule_expr_set_u32(e, NFT_EXPR_LOG_SNAPLEN, log->snaplen); + + if (nft_mxml_num_parse(tree, "qthreshold", MXML_DESCEND_FIRST, BASE_DEC, + &log->qthreshold, NFT_TYPE_U16, NFT_XML_MAND, + err) == 0) { + nft_rule_expr_set_u16(e, NFT_EXPR_LOG_QTHRESHOLD, + log->qthreshold); + } return 0; #else @@ -254,14 +252,31 @@ static int nft_rule_expr_log_snprintf_default(char *buf, size_t len, static int nft_rule_expr_log_snprintf_xml(char *buf, size_t size, struct nft_rule_expr *e) { + int ret, len = size, offset = 0; struct nft_expr_log *log = nft_expr_data(e); - return snprintf(buf, size, "<prefix>%s</prefix>" - "<group>%u</group>" - "<snaplen>%u</snaplen>" - "<qthreshold>%u</qthreshold>", - log->prefix, log->group, - log->snaplen, log->qthreshold); + if (e->flags & (1 << NFT_EXPR_LOG_PREFIX)) { + ret = snprintf(buf+offset, len, "<prefix>%s</prefix>", + log->prefix); + SNPRINTF_BUFFER_SIZE(ret, size, len, offset); + } + if (e->flags & (1 << NFT_EXPR_LOG_GROUP)) { + ret = snprintf(buf+offset, len, "<group>%u</group>", + log->group); + SNPRINTF_BUFFER_SIZE(ret, size, len, offset); + } + if (e->flags & (1 << NFT_EXPR_LOG_SNAPLEN)) { + ret = snprintf(buf+offset, len, "<snaplen>%u</snaplen>", + log->snaplen); + SNPRINTF_BUFFER_SIZE(ret, size, len, offset); + } + if (e->flags & (1 << NFT_EXPR_LOG_QTHRESHOLD)) { + ret = snprintf(buf+offset, len, "<qthreshold>%u</qthreshold>", + log->qthreshold); + SNPRINTF_BUFFER_SIZE(ret, size, len, offset); + } + + return offset; } static int nft_rule_expr_log_snprintf_json(char *buf, size_t len, -- 2.0.0.rc2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/2] src: expr: log: Do not print unset values in xml. 2014-06-02 10:11 ` [PATCH v2 2/2] src: expr: log: Do not print unset values in xml Ana Rey @ 2014-06-02 10:21 ` Pablo Neira Ayuso 0 siblings, 0 replies; 5+ messages in thread From: Pablo Neira Ayuso @ 2014-06-02 10:21 UTC (permalink / raw) To: Ana Rey; +Cc: netfilter-devel On Mon, Jun 02, 2014 at 12:11:33PM +0200, Ana Rey wrote: > It changes the parse and the snprint functions to omit unset values. > > If we used this rule: > ntt add rule ip test output log > > We got this xml file: > > <rule><family>ip</family> > <table>test</table> > <chain>output</chain> > <handle>88</handle> > <expr type="log"> > <prefix>(null)</prefix> > <group>0</group> > <snaplen>0</snaplen> > <qthreshold>0</qthreshold> > </expr> > </rule> > > And It was imposible import this file. > > Now, That rule creates this xml file without null values: > > <rule><family>ip</family> > <table>test</table> > <chain>output</chain> > <handle>88</handle> > <expr type="log"> > </expr> > </rule> > > and It's possible import this xml file. > > Signed-off-by: Ana Rey <anarey@gmail.com> > --- > [Changes in v2] > This patch has some changes that derive from the previous > "src: expr: log: Code refactoring to use nft_rule_expr_set_* functions" > patch. > > > src/expr/log.c | 57 ++++++++++++++++++++++++++++++++++++--------------------- > 1 file changed, 36 insertions(+), 21 deletions(-) > > + if (nft_mxml_num_parse(tree, "qthreshold", MXML_DESCEND_FIRST, BASE_DEC, > + &log->qthreshold, NFT_TYPE_U16, NFT_XML_MAND, > + err) == 0) { > + nft_rule_expr_set_u16(e, NFT_EXPR_LOG_QTHRESHOLD, > + log->qthreshold); > + } ^------^ wrong indentation. > > return 0; > #else > @@ -254,14 +252,31 @@ static int nft_rule_expr_log_snprintf_default(char *buf, size_t len, > static int nft_rule_expr_log_snprintf_xml(char *buf, size_t size, > struct nft_rule_expr *e) > { > + int ret, len = size, offset = 0; > struct nft_expr_log *log = nft_expr_data(e); > > - return snprintf(buf, size, "<prefix>%s</prefix>" > - "<group>%u</group>" > - "<snaplen>%u</snaplen>" > - "<qthreshold>%u</qthreshold>", > - log->prefix, log->group, > - log->snaplen, log->qthreshold); > + if (e->flags & (1 << NFT_EXPR_LOG_PREFIX)) { > + ret = snprintf(buf+offset, len, "<prefix>%s</prefix>", > + log->prefix); > + SNPRINTF_BUFFER_SIZE(ret, size, len, offset); > + } > + if (e->flags & (1 << NFT_EXPR_LOG_GROUP)) { > + ret = snprintf(buf+offset, len, "<group>%u</group>", > + log->group); > + SNPRINTF_BUFFER_SIZE(ret, size, len, offset); > + } > + if (e->flags & (1 << NFT_EXPR_LOG_SNAPLEN)) { > + ret = snprintf(buf+offset, len, "<snaplen>%u</snaplen>", > + log->snaplen); > + SNPRINTF_BUFFER_SIZE(ret, size, len, offset); > + } > + if (e->flags & (1 << NFT_EXPR_LOG_QTHRESHOLD)) { > + ret = snprintf(buf+offset, len, "<qthreshold>%u</qthreshold>", > + log->qthreshold); > + SNPRINTF_BUFFER_SIZE(ret, size, len, offset); > + } Same wrong indentation here. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-06-02 10:21 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-06-02 10:11 [PATCH v2 0/2] Do not print unset value in xml file Ana Rey 2014-06-02 10:11 ` [PATCH v2 1/2] src: expr: log: Code refactoring to use nft_rule_expr_set_* functions Ana Rey 2014-06-02 10:19 ` Pablo Neira Ayuso 2014-06-02 10:11 ` [PATCH v2 2/2] src: expr: log: Do not print unset values in xml Ana Rey 2014-06-02 10:21 ` Pablo Neira Ayuso
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).