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 &r->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(&r->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));
+       }
+
        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 *set_list)
  {
-       mxml_node_t *node;
+       mxml_node_t *node, *node_ud;
        struct nftnl_expr *e;
        const char *table, *chain;
        int family;
@@ -649,6 +682,35 @@ int nftnl_mxml_rule_parse(mxml_node_t *tree, struct 
nftnl_rule *r,
                nftnl_rule_add_expr(r, e);
        }

+       node_ud = mxmlFindElement(tree, tree, "userdata", NULL, NULL,
+                                 MXML_DESCEND);

You better wrap this code into a function in the mxml.c file as you
will need this later on for sets too. Same thing for the json code.


Done.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to