Re: [PATCH 2/4 v3] libnftnl: rule: Change the "userdata" attribute to use new TLV buffer
Thanks for the feedback.
On 08/03/16 14:04, Pablo Neira Ayuso wrote:
On Mon, Mar 07, 2016 at 06:10:42PM +0100, Carlos Falgueras García wrote:
@@ -75,6 +81,8 @@ void nftnl_rule_free(struct nftnl_rule *r)
xfree(r->table);
if (r->chain != NULL)
xfree(r->chain);
+ if (r->flags & (1 << NFTNL_RULE_USERDATA))
+ nftnl_udata_free(r->userdata);
xfree(r);
}
@@ -162,8 +170,12 @@ void nftnl_rule_set_data(struct nftnl_rule *r, uint16_t
attr,
r->position = *((uint64_t *)data);
break;
case NFTNL_RULE_USERDATA:
- r->user.data = (void *)data;
- r->user.len = data_len;
You have to check here if r->userdata is already set, if so, release
it in first place.
+ (r->userdata = nftnl_udata_alloc(data_len));
^ ^
You don't need these parens.
Sorry, I'll remove them.
+ if (!r->userdata) {
+ perror("nftnl_rule_set_data - userdata");
+ return;
+ }
+ nftnl_udata_copy_data(r->userdata, data, data_len);
break;
}
r->flags |= (1 << attr);
@@ -221,8 +233,8 @@ const void *nftnl_rule_get_data(const struct nftnl_rule *r,
uint16_t attr,
*data_len = sizeof(uint64_t);
return >position;
case NFTNL_RULE_USERDATA:
- *data_len = r->user.len;
- return r->user.data;
+ *data_len = nftnl_udata_len(r->userdata);
+ return (void *)nftnl_udata_data(r->userdata);
}
return NULL;
}
@@ -288,8 +300,9 @@ void nftnl_rule_nlmsg_build_payload(struct nlmsghdr *nlh,
struct nftnl_rule *r)
if (r->flags & (1 << NFTNL_RULE_POSITION))
mnl_attr_put_u64(nlh, NFTA_RULE_POSITION, htobe64(r->position));
if (r->flags & (1 << NFTNL_RULE_USERDATA)) {
- mnl_attr_put(nlh, NFTA_RULE_USERDATA, r->user.len,
-r->user.data);
+ mnl_attr_put(nlh, NFTA_RULE_USERDATA,
+nftnl_udata_len(r->userdata),
+nftnl_udata_data(r->userdata));
}
if (!list_empty(>expr_list)) {
@@ -447,19 +460,17 @@ int nftnl_rule_nlmsg_parse(const struct nlmsghdr *nlh,
struct nftnl_rule *r)
r->flags |= (1 << NFTNL_RULE_POSITION);
}
if (tb[NFTA_RULE_USERDATA]) {
+ uint16_t udata_size;
Missing line break after this.
Thanks, I'll correct it.
const void *udata =
mnl_attr_get_payload(tb[NFTA_RULE_USERDATA]);
- if (r->user.data)
- xfree(r->user.data);
These lines above are now gone, they avoid a memory leak.
I'll check these two cases using the flags. Is it ok?
if (r->flags & (1 << NFTNL_RULE_USERDATA))
nftnl_udata_free(r->userdata);
-
- r->user.len = mnl_attr_get_payload_len(tb[NFTA_RULE_USERDATA]);
+ udata_size = mnl_attr_get_payload_len(tb[NFTA_RULE_USERDATA]);
- r->user.data = malloc(r->user.len);
- if (r->user.data == NULL)
+ (r->userdata = nftnl_udata_alloc(udata_size));
^ ^
No need for parens.
Sorry, I'll fix it.
+ if (!r->userdata)
return -1;
+ nftnl_udata_copy_data(r->userdata, udata, udata_size);
- memcpy(r->user.data, udata, r->user.len);
r->flags |= (1 << NFTNL_RULE_USERDATA);
}
@@ -481,6 +492,7 @@ int nftnl_jansson_parse_rule(struct nftnl_rule *r, json_t
*tree,
uint64_t uval64;
uint32_t uval32;
int i, family;
+ struct nftnl_udata_buf *buf;
root = nftnl_jansson_get_node(tree, "rule", err);
if (root == NULL)
@@ -557,6 +569,27 @@ int nftnl_jansson_parse_rule(struct nftnl_rule *r, json_t
*tree,
nftnl_rule_add_expr(r, e);
}
+ array = json_object_get(root, "userdata");
+ if (array != NULL) {
+ buf = nftnl_udata_alloc(NFT_USERDATA_MAXLEN);
+ if (!buf) {
+ perror("nftnl_jansson_parse_rule");
+ goto err;
+ }
+
+ for (i = 0; i < json_array_size(array); ++i) {
+ if (nftnl_jansson_udata_parse(buf,
+ json_array_get(array, i),
+ err,
+ set_list) < 0)
+ goto err;
+ }
+
+ nftnl_rule_set_data(r, NFTNL_RULE_USERDATA,
+ nftnl_udata_data(buf),
+ nftnl_udata_len(buf));
+ }
+
Re: [PATCH 2/4 v3] libnftnl: rule: Change the "userdata" attribute to use new TLV buffer
On Mon, Mar 07, 2016 at 06:10:42PM +0100, Carlos Falgueras García wrote:
> @@ -75,6 +81,8 @@ void nftnl_rule_free(struct nftnl_rule *r)
> xfree(r->table);
> if (r->chain != NULL)
> xfree(r->chain);
> + if (r->flags & (1 << NFTNL_RULE_USERDATA))
> + nftnl_udata_free(r->userdata);
>
> xfree(r);
> }
> @@ -162,8 +170,12 @@ void nftnl_rule_set_data(struct nftnl_rule *r, uint16_t
> attr,
> r->position = *((uint64_t *)data);
> break;
> case NFTNL_RULE_USERDATA:
> - r->user.data = (void *)data;
> - r->user.len = data_len;
You have to check here if r->userdata is already set, if so, release
it in first place.
> + (r->userdata = nftnl_udata_alloc(data_len));
^ ^
You don't need these parens.
> + if (!r->userdata) {
> + perror("nftnl_rule_set_data - userdata");
> + return;
> + }
> + nftnl_udata_copy_data(r->userdata, data, data_len);
> break;
> }
> r->flags |= (1 << attr);
> @@ -221,8 +233,8 @@ const void *nftnl_rule_get_data(const struct nftnl_rule
> *r, uint16_t attr,
> *data_len = sizeof(uint64_t);
> return >position;
> case NFTNL_RULE_USERDATA:
> - *data_len = r->user.len;
> - return r->user.data;
> + *data_len = nftnl_udata_len(r->userdata);
> + return (void *)nftnl_udata_data(r->userdata);
> }
> return NULL;
> }
> @@ -288,8 +300,9 @@ void nftnl_rule_nlmsg_build_payload(struct nlmsghdr *nlh,
> struct nftnl_rule *r)
> if (r->flags & (1 << NFTNL_RULE_POSITION))
> mnl_attr_put_u64(nlh, NFTA_RULE_POSITION, htobe64(r->position));
> if (r->flags & (1 << NFTNL_RULE_USERDATA)) {
> - mnl_attr_put(nlh, NFTA_RULE_USERDATA, r->user.len,
> - r->user.data);
> + mnl_attr_put(nlh, NFTA_RULE_USERDATA,
> + nftnl_udata_len(r->userdata),
> + nftnl_udata_data(r->userdata));
> }
>
> if (!list_empty(>expr_list)) {
> @@ -447,19 +460,17 @@ int nftnl_rule_nlmsg_parse(const struct nlmsghdr *nlh,
> struct nftnl_rule *r)
> r->flags |= (1 << NFTNL_RULE_POSITION);
> }
> if (tb[NFTA_RULE_USERDATA]) {
> + uint16_t udata_size;
Missing line break after this.
> const void *udata =
> mnl_attr_get_payload(tb[NFTA_RULE_USERDATA]);
>
> - if (r->user.data)
> - xfree(r->user.data);
These lines above are now gone, they avoid a memory leak.
> -
> - r->user.len = mnl_attr_get_payload_len(tb[NFTA_RULE_USERDATA]);
> + udata_size = mnl_attr_get_payload_len(tb[NFTA_RULE_USERDATA]);
>
> - r->user.data = malloc(r->user.len);
> - if (r->user.data == NULL)
> + (r->userdata = nftnl_udata_alloc(udata_size));
^ ^
No need for parens.
> + if (!r->userdata)
> return -1;
> + nftnl_udata_copy_data(r->userdata, udata, udata_size);
>
> - memcpy(r->user.data, udata, r->user.len);
> r->flags |= (1 << NFTNL_RULE_USERDATA);
> }
>
> @@ -481,6 +492,7 @@ int nftnl_jansson_parse_rule(struct nftnl_rule *r, json_t
> *tree,
> uint64_t uval64;
> uint32_t uval32;
> int i, family;
> + struct nftnl_udata_buf *buf;
>
> root = nftnl_jansson_get_node(tree, "rule", err);
> if (root == NULL)
> @@ -557,6 +569,27 @@ int nftnl_jansson_parse_rule(struct nftnl_rule *r,
> json_t *tree,
> nftnl_rule_add_expr(r, e);
> }
>
> + array = json_object_get(root, "userdata");
> + if (array != NULL) {
> + buf = nftnl_udata_alloc(NFT_USERDATA_MAXLEN);
> + if (!buf) {
> + perror("nftnl_jansson_parse_rule");
> + goto err;
> + }
> +
> + for (i = 0; i < json_array_size(array); ++i) {
> + if (nftnl_jansson_udata_parse(buf,
> + json_array_get(array, i),
> + err,
> + set_list) < 0)
> + goto err;
> + }
> +
> + nftnl_rule_set_data(r, NFTNL_RULE_USERDATA,
> + nftnl_udata_data(buf),
> + nftnl_udata_len(buf));
> + }
> +
> return 0;
> err:
> return -1;
> @@ -592,7 +625,7 @@ int nftnl_mxml_rule_parse(mxml_node_t *tree, struct
> nftnl_rule *r,
> struct nftnl_parse_err *err,
> struct nftnl_set_list
