On 11 November 2013 18:32, Ana Rey <anarey@xxxxxxxxx> wrote:
> Makefile and tests files for libnftables.
>
> In this patch, I create a initial object 'a' and I set a testing values. Then,
> I convert this object in a Netlink message.
>
> From this Netlink message, I parse that (through nft_table_nlmsg_parse
> function) in 'b' objetc.
>
> Thus, we make sure that object transformations are correct.
> ---
> v2: fix writing erorres in messages that I unintentionally added in v1 that
> were spotted by Phil Oester.
>
> Fix a multiline comment error. The nft-ruleset-test is not included
> in this patch, also, I delete it in the tests/test-script.sh. Those errors were
> spotted by Arturo Borrero.
>
Hi Ana, thanks for the update.
I've been testing your patch. Some comments below.
> diff --git a/tests/nft-expr_bitwise-test.c b/tests/nft-expr_bitwise-test.c
> new file mode 100644
> index 0000000..46823d8
> --- /dev/null
> +++ b/tests/nft-expr_bitwise-test.c
> @@ -0,0 +1,104 @@
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +
> +#include <netinet/in.h>
> +#include <netinet/ip.h>
> +#include <linux/netfilter/nf_tables.h>
> +#include <libmnl/libmnl.h>
> +#include <libnftables/rule.h>
> +#include <libnftables/expr.h>
> +
> +static void die(const char *msg)
> +{
> + printf("\033[31mERROR:\e[0m %s\n", msg);
> + exit(EXIT_FAILURE);
> +}
> +
> +static void cmp_nft_rule_expr(struct nft_rule_expr *rule_a,
> + struct nft_rule_expr *rule_b)
> +{
> + uint32_t maska = NULL;
> + uint32_t maskb = NULL;
> + uint32_t xora = NULL;
> + uint32_t xorb = NULL;
I think those initialization are unnecessary. Also, my compiler says:
nft-expr_bitwise-test.c: In function ‘cmp_nft_rule_expr’:
nft-expr_bitwise-test.c:21:19: warning: initialization makes integer
from pointer without a cast [enabled by default]
nft-expr_bitwise-test.c:22:19: warning: initialization makes integer
from pointer without a cast [enabled by default]
nft-expr_bitwise-test.c:23:18: warning: initialization makes integer
from pointer without a cast [enabled by default]
nft-expr_bitwise-test.c:24:18: warning: initialization makes integer
from pointer without a cast [enabled by default]
Same in nft-expr_cmp-test.c
> diff --git a/tests/nft-expr_match-test.c b/tests/nft-expr_match-test.c
> new file mode 100644
> index 0000000..9a8b634
> --- /dev/null
> +++ b/tests/nft-expr_match-test.c
> @@ -0,0 +1,105 @@
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +
> +#include <arpa/inet.h>
> +#include <netinet/in.h>
> +#include <netinet/ip.h>
> +#include <linux/netfilter/nf_tables.h>
> +#include <linux/netfilter/xt_iprange.h>
> +#include <linux/netfilter_ipv4/ipt_LOG.h>
In file included from nft-expr_match-test.c:10:0:
/usr/include/linux/netfilter_ipv4/ipt_LOG.h:4:2: warning: #warning
"Please update iptables, this file will be removed soon!" [-Wcpp]
Should we ignore this warning?
Also found in nft-expr_target-test.c
> diff --git a/tests/nft-expr_target-test.c b/tests/nft-expr_target-test.c
> new file mode 100644
> index 0000000..756ab2d
> --- /dev/null
> +++ b/tests/nft-expr_target-test.c
> @@ -0,0 +1,112 @@
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +
> +#include <netinet/in.h>
> +#include <netinet/ip.h>
> +#include <linux/netfilter/nf_tables.h>
> +#include <linux/netfilter/xt_iprange.h>
> +#include <linux/netfilter_ipv4/ipt_LOG.h>
> +#include <libmnl/libmnl.h>
> +#include <libnftables/rule.h>
> +#include <libnftables/expr.h>
> +
> +static void die(const char *msg)
> +{
> + printf("\033[31mERROR:\e[0m %s\n", msg);
> + exit(EXIT_FAILURE);
> +}
> +
> +static void die2(const char *msg, uint32_t a, uint32_t b)
> +{
> + printf("\033[31mERROR:\e[0m %s size a: %d b: %d \n",msg, a, b);
> + exit(EXIT_FAILURE);
> +}
> +
> +static void cmp_nft_rule_expr(struct nft_rule_expr *rule_a,
> + struct nft_rule_expr *rule_b)
> +{
> + uint32_t lena, lenb;
> +
> + if (strcmp(nft_rule_expr_get_str(rule_a, NFT_EXPR_TG_NAME),
> + nft_rule_expr_get_str(rule_b, NFT_EXPR_TG_NAME)) != 0)
> + die("Expr TG_NAME mismatches");
> +
> + if (nft_rule_expr_get_u32(rule_a, NFT_EXPR_TG_REV) !=
> + nft_rule_expr_get_u32(rule_b, NFT_EXPR_TG_REV))
> + die("Expr TG_REV mismatches");
> +
> + nft_rule_expr_get(rule_a, NFT_EXPR_TG_INFO, &lena);
> + nft_rule_expr_get(rule_b, NFT_EXPR_TG_INFO, &lenb);
> +
> + /* TODO ¿BUG? */
> + if (lena != lenb)
> + die2("Expr TG_DATA size mismatches", lena, lenb);
> +}
> +
When I run the test:
root@debian:/home/aborrero/git/libnftables/tests# ./test-script.sh
/home/aborrero/git/libnftables/tests/.libs/lt-nft-chain-test: OK
/home/aborrero/git/libnftables/tests/.libs/lt-nft-expr_bitwise-test: OK
/home/aborrero/git/libnftables/tests/.libs/lt-nft-expr_byteorder-test: OK
/home/aborrero/git/libnftables/tests/.libs/lt-nft-expr_cmp-test: OK
/home/aborrero/git/libnftables/tests/.libs/lt-nft-expr_counter-test: OK
/home/aborrero/git/libnftables/tests/.libs/lt-nft-expr_ct-test: OK
/home/aborrero/git/libnftables/tests/.libs/lt-nft-expr_exthdr-test: OK
/home/aborrero/git/libnftables/tests/.libs/lt-nft-expr_immediate-test: OK
/home/aborrero/git/libnftables/tests/.libs/lt-nft-expr_limit-test: OK
/home/aborrero/git/libnftables/tests/.libs/lt-nft-expr_log-test: OK
/home/aborrero/git/libnftables/tests/.libs/lt-nft-expr_lookup-test: OK
/home/aborrero/git/libnftables/tests/.libs/lt-nft-expr_match-test: OK
/home/aborrero/git/libnftables/tests/.libs/lt-nft-expr_meta-test: OK
/home/aborrero/git/libnftables/tests/.libs/lt-nft-expr_nat-test: OK
/home/aborrero/git/libnftables/tests/.libs/lt-nft-expr_payload-test: OK
ERROR: Expr TG_DATA size mismatches size a: 32 b: 36
/home/aborrero/git/libnftables/tests/.libs/lt-nft-rule-test: OK
/home/aborrero/git/libnftables/tests/.libs/lt-nft-set-test: OK
/home/aborrero/git/libnftables/tests/.libs/lt-nft-table-test: OK
I guess the error above is the bug you spotted.
> +int main(int argc, char *argv[])
> +{
> + struct nft_rule *a, *b;
> + struct nft_rule_expr *ex;
> + struct nlmsghdr *nlh;
> + struct ipt_log_info *info;
> + char buf[4096];
> + struct nft_rule_expr_iter *iter_a, *iter_b;
> + struct nft_rule_expr *rule_a, *rule_b;
> +
> + a = nft_rule_alloc();
> + b = nft_rule_alloc();
> + if (a == NULL || b == NULL)
> + die("OOM");
> +
> + ex = nft_rule_expr_alloc("target");
> + if (ex == NULL)
> + die("OOM");
> + nft_rule_expr_set(ex, NFT_EXPR_TG_NAME, "LOG", strlen("LOG"));
> + nft_rule_expr_set_u32(ex, NFT_EXPR_TG_REV, 0x12345678);
> +
> + info = calloc(1, sizeof(struct ipt_log_info));
> + if (info == NULL)
> + die("OOM");
> + sprintf(info->prefix, "test: ");
> + info->prefix[sizeof(info->prefix)-1] = '\0';
> + info->logflags = 0x0f;
> + info->level = 5;
> + nft_rule_expr_set(ex, NFT_EXPR_TG_INFO, info, sizeof(*info));
> +
> + nft_rule_add_expr(a, ex);
> +
> + nlh = nft_rule_nlmsg_build_hdr(buf, NFT_MSG_NEWRULE, AF_INET, 0, 1234);
> + nft_rule_nlmsg_build_payload(nlh, a);
> +
> + if (nft_rule_nlmsg_parse(nlh, b) < 0)
> + die("parsing problems");
> +
> + iter_a = nft_rule_expr_iter_create(a);
> + iter_b = nft_rule_expr_iter_create(b);
> + if (iter_a == NULL || iter_b == NULL)
> + die("OOM");
> +
> + rule_a = nft_rule_expr_iter_next(iter_a);
> + rule_b = nft_rule_expr_iter_next(iter_b);
> + if (rule_a == NULL || rule_b == NULL)
> + die("OOM");
> +
> + cmp_nft_rule_expr(rule_a, rule_b);
> +
> + if (nft_rule_expr_iter_next(iter_a) != NULL ||
> + nft_rule_expr_iter_next(iter_b) != NULL)
> + die("More 1 expr.");
> +
> + nft_rule_expr_iter_destroy(iter_a);
> + nft_rule_expr_iter_destroy(iter_b);
> + nft_rule_free(a);
> + nft_rule_free(b);
> + /* BUG: leak memory
> + ==28351== HEAP SUMMARY:
> + * ==28351== in use at exit: 436 bytes in 8 blocks
> + * ==28351== total heap usage: 8 allocs, 0 frees, 436 bytes allocated
> + */
Do you think the leak is in nft_rule_expr_iter_destroy()?
==1860== 16 bytes in 1 blocks are definitely lost in loss record 2 of 8
==1860== at 0x4C272B8: calloc (vg_replace_malloc.c:566)
==1860== by 0x4E388D2: nft_rule_expr_iter_create (rule.c:860)
==1860== by 0x400D6F: main (nft-expr_target-test.c:85)
Also, I think the comment itself is not in good format (sorry for
being repetitive).
Regards
--
Arturo Borrero González
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
