From mboxrd@z Thu Jan 1 00:00:00 1970 From: Pablo Neira Ayuso Subject: Re: [PATCH 4/4 v3] nftables: rule: Change the field "rule->comment" for an nftnl_udata_buf MIME-Version: 1.0 Date: Tue, 8 Mar 2016 14:13:51 +0100 Message-ID: <20160308131351.GC5125@salvia> References: <1457370643-14408-1-git-send-email-carlosfg@riseup.net> <1457370643-14408-2-git-send-email-carlosfg@riseup.net> Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: netfilter-devel@vger.kernel.org, kaber@trash.net To: Carlos Falgueras =?iso-8859-1?Q?Garc=EDa?= Return-path: Received: from mail.us.es ([193.147.175.20]:43511 "EHLO mail.us.es" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750974AbcCHNOD (ORCPT ); Tue, 8 Mar 2016 08:14:03 -0500 Received: from antivirus1-rhel7.int (unknown [192.168.2.11]) by mail.us.es (Postfix) with ESMTP id 7F135842A for ; Tue, 8 Mar 2016 14:14:01 +0100 (CET) Received: from antivirus1-rhel7.int (localhost [127.0.0.1]) by antivirus1-rhel7.int (Postfix) with ESMTP id 6A11FDA38E for ; Tue, 8 Mar 2016 14:14:01 +0100 (CET) Received: from antivirus1-rhel7.int (localhost [127.0.0.1]) by antivirus1-rhel7.int (Postfix) with ESMTP id 7DA00DA396 for ; Tue, 8 Mar 2016 14:13:55 +0100 (CET) Content-Disposition: inline In-Reply-To: <1457370643-14408-2-git-send-email-carlosfg@riseup.net> Sender: netfilter-devel-owner@vger.kernel.org List-ID: On Mon, Mar 07, 2016 at 06:10:41PM +0100, Carlos Falgueras Garc=EDa wro= te: > Now it is possible to store multiple variable length user data into r= ule. > Modify the parser in order to fill the nftnl_udata with the comment, = and the > print function for extract these commentary and print it to user. >=20 > Signed-off-by: Carlos Falgueras Garc=EDa > --- > include/rule.h | 11 +++++++++-- > src/netlink_delinearize.c | 7 +++++-- > src/netlink_linearize.c | 6 ++++-- > src/parser_bison.y | 15 ++++++++++++++- > src/rule.c | 41 +++++++++++++++++++++++++++++++++++++= +--- > 5 files changed, 70 insertions(+), 10 deletions(-) >=20 > diff --git a/include/rule.h b/include/rule.h > index c848f0f..c500e88 100644 > --- a/include/rule.h > +++ b/include/rule.h > @@ -4,6 +4,7 @@ > #include > #include > #include > +#include > =20 > /** > * struct handle - handle for tables, chains, rules and sets > @@ -155,7 +156,7 @@ extern void chain_print_plain(const struct chain = *chain); > * @location: location the rule was defined at > * @stmt: list of statements > * @num_stmts: number of statements in stmts list > - * @comment: comment > + * @udata: user data > */ > struct rule { > struct list_head list; > @@ -163,7 +164,7 @@ struct rule { > struct location location; > struct list_head stmts; > unsigned int num_stmts; > - const char *comment; > + struct nftnl_udata_buf *udata; > }; > =20 > extern struct rule *rule_alloc(const struct location *loc, > @@ -396,4 +397,10 @@ extern int do_command(struct netlink_ctx *ctx, s= truct cmd *cmd); > extern int cache_update(enum cmd_ops cmd, struct list_head *msgs); > extern void cache_release(void); > =20 > +enum udata_type { > + UDATA_TYPE_COMMENT, > + __UDATA_TYPE_MAX, > +}; > +#define UDATA_TYPE_MAX (__UDATA_TYPE_MAX - 1) > + > #endif /* NFTABLES_RULE_H */ > diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c > index ae6abb0..2f8c512 100644 > --- a/src/netlink_delinearize.c > +++ b/src/netlink_delinearize.c > @@ -25,6 +25,7 @@ > #include > #include > #include > +#include > =20 > struct netlink_parse_ctx { > struct list_head *msgs; > @@ -1738,8 +1739,10 @@ struct rule *netlink_delinearize_rule(struct n= etlink_ctx *ctx, > uint32_t len; > =20 > data =3D nftnl_rule_get_data(nlr, NFTNL_RULE_USERDATA, &len); > - pctx->rule->comment =3D xmalloc(len); > - memcpy((char *)pctx->rule->comment, data, len); > + pctx->rule->udata =3D nftnl_udata_alloc(len); > + if (!pctx->rule->udata) > + memory_allocation_error(); > + nftnl_udata_copy_data(pctx->rule->udata, data, len); > } > =20 > nftnl_expr_foreach((struct nftnl_rule *)nlr, netlink_parse_expr, pc= tx); > diff --git a/src/netlink_linearize.c b/src/netlink_linearize.c > index 86b49c6..00f81ea 100644 > --- a/src/netlink_linearize.c > +++ b/src/netlink_linearize.c > @@ -21,6 +21,7 @@ > #include > =20 > #include > +#include > =20 > =20 > struct netlink_linearize_ctx { > @@ -1108,9 +1109,10 @@ void netlink_linearize_rule(struct netlink_ctx= *ctx, struct nftnl_rule *nlr, > list_for_each_entry(stmt, &rule->stmts, list) > netlink_gen_stmt(&lctx, stmt); > =20 > - if (rule->comment) > + if (rule->udata) > nftnl_rule_set_data(nlr, NFTNL_RULE_USERDATA, > - rule->comment, strlen(rule->comment) + 1); > + nftnl_udata_data(rule->udata), > + nftnl_udata_len(rule->udata)); > =20 > netlink_dump_rule(nlr); > } > diff --git a/src/parser_bison.y b/src/parser_bison.y > index 05ade0f..ed1b63a 100644 > --- a/src/parser_bison.y > +++ b/src/parser_bison.y > @@ -24,6 +24,7 @@ > #include > #include > #include > +#include > =20 > #include > #include > @@ -1304,7 +1305,19 @@ rule : stmt_list comment_spec > struct stmt *i; > =20 > $$ =3D rule_alloc(&@$, NULL); > - $$->comment =3D $2; > + > + if ($2) { > + if (!($$->udata =3D nftnl_udata_alloc(NFT_USERDATA_MAXLEN))) > + memory_allocation_error(); > + > + if (!nftnl_udata_put_strz($$->udata, > + UDATA_TYPE_COMMENT, $2)) { Mind coding style: if (!nftnl_udata_put_strz($$->udata, DATA_= TYPE_COMMENT, $2)) { You have to align the parameters to the parens. > + erec_queue(error(&@2, "Comment too long: \"%s\"", $2), > + state->msgs); > + YYERROR; > + } > + } > + > list_for_each_entry(i, $1, list) > $$->num_stmts++; > list_splice_tail($1, &$$->stmts); > diff --git a/src/rule.c b/src/rule.c > index 18ff592..60d9b38 100644 > --- a/src/rule.c > +++ b/src/rule.c > @@ -23,6 +23,7 @@ > =20 > #include > #include > +#include > #include > #include > #include > @@ -366,6 +367,7 @@ struct rule *rule_alloc(const struct location *lo= c, const struct handle *h) > rule->location =3D *loc; > init_list_head(&rule->list); > init_list_head(&rule->stmts); > + rule->udata =3D NULL; No need to set this to null. IIRC the rule object is already allocated in an already initialized memory area, right? > if (h !=3D NULL) > rule->handle =3D *h; > return rule; > @@ -375,21 +377,54 @@ void rule_free(struct rule *rule) > { > stmt_list_free(&rule->stmts); > handle_free(&rule->handle); > - xfree(rule->comment); > + nftnl_udata_free(rule->udata); > xfree(rule); > } > =20 > +static int rule_parse_userdata_cb(const struct nftnl_udata *attr, > + void *data) > +{ > + const struct nftnl_udata **tb =3D data; > + uint8_t type =3D nftnl_udata_attr_type(attr); > + uint8_t len =3D nftnl_udata_attr_len(attr); > + unsigned char *value =3D nftnl_udata_attr_value(attr); It is prefered to define variables in this way: unsigned char *value =3D nftnl_udata_attr_value(attr); uint8_t type =3D nftnl_udata_attr_type(attr); uint8_t len =3D nftnl_udata_attr_len(attr); const struct nftnl_udata **tb =3D data; ie. longest line first. It's easier to read the code in this way. > + > + /* Validation */ Remove this comment, it's obvious what this is doing ;-) > + switch (type) { > + case UDATA_TYPE_COMMENT: > + if (value[len-1] !=3D '\0') > + return NFTNL_CB_ERROR; You can remove the NFTNL_CB_* definitions. I know you're based on libmnl, but in this case I think it's fine to return the values. > + break; > + default: > + break; > + }; > + > + tb[type] =3D attr; > + return NFTNL_CB_OK; > +} > + > + > void rule_print(const struct rule *rule) > { > const struct stmt *stmt; > + const struct nftnl_udata *tb[UDATA_TYPE_MAX + 1] =3D {}; > + const struct nftnl_udata *attr; > =20 > list_for_each_entry(stmt, &rule->stmts, list) { > stmt->ops->print(stmt); > printf(" "); > } > =20 > - if (rule->comment) > - printf("comment \"%s\" ", rule->comment); > + if (rule->udata) { > + if (nftnl_udata_parse(rule->udata, rule_parse_userdata_cb, tb) > + !=3D NFTNL_CB_ERROR > + ) { ^^^ This coding style is not correct. > + attr =3D tb[UDATA_TYPE_COMMENT]; > + if (attr) You can do this instead: if (tb[UDATA_TYPE_COMMENT]) { printf("comment \"%s\" ", (char *)nftnl_udata_attr_value(= tb[UDATA_TYPE_COMMENT])); > + printf("comment \"%s\" ", > + (char *)nftnl_udata_attr_value(attr)); > + } > + } > =20 > if (handle_output > 0) > printf("# handle %" PRIu64, rule->handle.handle); > --=20 > 2.7.2 >=20 -- To unsubscribe from this list: send the line "unsubscribe netfilter-dev= el" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html