[libnftables PATCH 1/6] test: return EXIT_FAILURE if some error was found

[libnftables PATCH 1/6] test: return EXIT_FAILURE if some error was found

[Arturo Borrero Gonzalez]

Before this patch, 0 was returned unconditionally.

Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
---
 tests/nft-parsing-test.c |   15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/tests/nft-parsing-test.c b/tests/nft-parsing-test.c
index 866c985..70028a0 100644
--- a/tests/nft-parsing-test.c
+++ b/tests/nft-parsing-test.c
@@ -234,7 +234,7 @@ static int test_json(const char *filename)
 			if (nft_ruleset_parse(rs, NFT_RULESET_PARSE_JSON, json) == 0)
 				ret = compare_test(TEST_JSON_RULESET, rs, filename);
 			else
-				ret = -1;
+				goto failparsing;
 
 			nft_ruleset_free(rs);
 			}
@@ -329,7 +329,7 @@ static int test_xml(const char *filename)
 				ret = compare_test(TEST_XML_RULESET, rs,
 						   filename);
 			else
-				ret = -1;
+				goto failparsing;
 
 			nft_ruleset_free(rs);
 		}
@@ -352,6 +352,7 @@ int main(int argc, char *argv[])
 	DIR *d;
 	struct dirent *dent;
 	char path[PATH_MAX];
+	int ret = 0, exit_code = 0;
 
 	if (argc != 2) {
 		fprintf(stderr, "Usage: %s <directory>\n", argv[0]);
@@ -374,19 +375,25 @@ int main(int argc, char *argv[])
 		snprintf(path, sizeof(path), "%s/%s", argv[1], dent->d_name);
 
 		if (strcmp(&dent->d_name[len-4], ".xml") == 0) {
-			if (test_xml(path) == 0) {
+			if ((ret = test_xml(path)) == 0) {
 				printf("parsing and validating %s: ", path);
 				printf("\033[32mOK\e[0m\n");
 			}
+			exit_code += ret;
 		}
 		if (strcmp(&dent->d_name[len-5], ".json") == 0) {
-			if (test_json(path) == 0) {
+			if ((ret = test_json(path)) == 0) {
 				printf("parsing and validating %s: ", path);
 				printf("\033[32mOK\e[0m\n");
 			}
+			exit_code += ret;
 		}
 	}
 
 	closedir(d);
+
+	if (exit_code != 0)
+		exit(EXIT_FAILURE);
+
 	return 0;
 }

[libnftables PATCH 2/6] test: fix memleak in XML testing

[Arturo Borrero Gonzalez]

Ensure the tree is freed when done.

Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
---
 tests/nft-parsing-test.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/nft-parsing-test.c b/tests/nft-parsing-test.c
index 70028a0..5eaaa1e 100644
--- a/tests/nft-parsing-test.c
+++ b/tests/nft-parsing-test.c
@@ -335,9 +335,11 @@ static int test_xml(const char *filename)
 		}
 	}
 
+	mxmlDelete(tree);
 	return ret;
 
 failparsing:
+	mxmlDelete(tree);
 	printf("parsing %s: ", filename);
 	printf("\033[31mFAILED\e[0m (%s)\n", strerror(errno));
 	return -1;

[libnftables PATCH 3/6] test: use constants for coloured strings

[Arturo Borrero Gonzalez]

Use constants for coloured strings, which have a ugly format.

Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
---
 tests/nft-parsing-test.c |   17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/tests/nft-parsing-test.c b/tests/nft-parsing-test.c
index 5eaaa1e..5333dbd 100644
--- a/tests/nft-parsing-test.c
+++ b/tests/nft-parsing-test.c
@@ -33,6 +33,9 @@ enum {
 	TEST_JSON_RULESET,
 };
 
+#define FAILED_STR "\033[31mFAILED\e[0m"
+#define OK_STR "\033[32mOK\e[0m"
+
 #if defined(XML_PARSING) || defined(JSON_PARSING)
 static void print_detail_error(char *a, char *b)
 {
@@ -160,8 +163,7 @@ static int compare_test(uint32_t type, void *input, const char *filename)
 	if (strncmp(orig, out, strlen(out)) == 0)
 		return 0;
 
-	printf("validating %s: ", filename);
-	printf("\033[31mFAILED\e[0m\n");
+	printf("parsing and validating %s: %s\n", filename, FAILED_STR);
 	print_detail_error(orig, out);
 	return -1;
 }
@@ -245,8 +247,7 @@ static int test_json(const char *filename)
 	return ret;
 
 failparsing:
-	printf("parsing %s: ", filename);
-	printf("\033[31mFAILED\e[0m (%s)\n", strerror(errno));
+	printf("parsing %s: %s (%s)\n", filename, FAILED_STR, strerror(errno));
 	free(json);
 	json_decref(root);
 	return -1;
@@ -340,8 +341,8 @@ static int test_xml(const char *filename)
 
 failparsing:
 	mxmlDelete(tree);
-	printf("parsing %s: ", filename);
-	printf("\033[31mFAILED\e[0m (%s)\n", strerror(errno));
+	printf("parsing and validating %s: %s (%s)\n", filename, FAILED_STR,
+	       strerror(errno));
 	return -1;
 #else
 	errno = EOPNOTSUPP;
@@ -379,14 +380,14 @@ int main(int argc, char *argv[])
 		if (strcmp(&dent->d_name[len-4], ".xml") == 0) {
 			if ((ret = test_xml(path)) == 0) {
 				printf("parsing and validating %s: ", path);
-				printf("\033[32mOK\e[0m\n");
+				printf("%s\n", OK_STR);
 			}
 			exit_code += ret;
 		}
 		if (strcmp(&dent->d_name[len-5], ".json") == 0) {
 			if ((ret = test_json(path)) == 0) {
 				printf("parsing and validating %s: ", path);
-				printf("\033[32mOK\e[0m\n");
+				printf("%s\n", OK_STR);
 			}
 			exit_code += ret;
 		}

[libnftables PATCH 4/6] test: report errors building XML tree

[Arturo Borrero Gonzalez]

Report error when building XML tree.

Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
---
 tests/nft-parsing-test.c |   10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/tests/nft-parsing-test.c b/tests/nft-parsing-test.c
index 5333dbd..3d26c3a 100644
--- a/tests/nft-parsing-test.c
+++ b/tests/nft-parsing-test.c
@@ -274,12 +274,18 @@ static int test_xml(const char *filename)
 	tree = mxmlLoadFile(NULL, fp, MXML_NO_CALLBACK);
 	fclose(fp);
 
-	if (tree == NULL)
+	if (tree == NULL) {
+		printf("unable to build XML tree from file %s %s\n", filename,
+		       FAILED_STR);
 		return -1;
+	}
 
 	xml = mxmlSaveAllocString(tree, MXML_NO_CALLBACK);
-	if (xml == NULL)
+	if (xml == NULL) {
+		printf("unable to alloc string from XML tree from %s %s\n",
+		       filename, FAILED_STR);
 		return -1;
+	}
 
 	/* Check what parsing should be done */
 	if (strcmp(tree->value.opaque, "table") == 0) {

[libnftables PATCH 5/6] test: style cleanup

[Arturo Borrero Gonzalez]

Some style cleanups.

Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
---
 tests/nft-parsing-test.c |   34 ++++++++++++++++++++++------------
 1 file changed, 22 insertions(+), 12 deletions(-)

diff --git a/tests/nft-parsing-test.c b/tests/nft-parsing-test.c
index 3d26c3a..e147e79 100644
--- a/tests/nft-parsing-test.c
+++ b/tests/nft-parsing-test.c
@@ -193,8 +193,10 @@ static int test_json(const char *filename)
 	if (json_object_get(root, "table") != NULL) {
 		t = nft_table_alloc();
 		if (t != NULL) {
-			if (nft_table_parse(t, NFT_TABLE_PARSE_JSON, json) == 0)
-				ret = compare_test(TEST_JSON_TABLE, t, filename);
+			if (nft_table_parse(t,
+					    NFT_TABLE_PARSE_JSON, json) == 0)
+				ret = compare_test(TEST_JSON_TABLE,
+						   t, filename);
 			else
 				goto failparsing;
 
@@ -203,8 +205,10 @@ static int test_json(const char *filename)
 	} else if (json_object_get(root, "chain") != NULL) {
 		c = nft_chain_alloc();
 		if (c != NULL) {
-			if (nft_chain_parse(c, NFT_CHAIN_PARSE_JSON, json) == 0)
-				ret = compare_test(TEST_JSON_CHAIN, c, filename);
+			if (nft_chain_parse(c,
+					    NFT_CHAIN_PARSE_JSON, json) == 0)
+				ret = compare_test(TEST_JSON_CHAIN,
+						   c, filename);
 			else
 				goto failparsing;
 
@@ -213,8 +217,10 @@ static int test_json(const char *filename)
 	} else if (json_object_get(root, "rule") != NULL) {
 		r = nft_rule_alloc();
 		if (r != NULL) {
-			if (nft_rule_parse(r, NFT_RULE_PARSE_JSON, json) == 0)
-				ret = compare_test(TEST_JSON_RULE, r, filename);
+			if (nft_rule_parse(r,
+					   NFT_RULE_PARSE_JSON, json) == 0)
+				ret = compare_test(TEST_JSON_RULE,
+						   r, filename);
 			else
 				goto failparsing;
 
@@ -229,17 +235,19 @@ static int test_json(const char *filename)
 				goto failparsing;
 
 			nft_set_free(s);
-			}
+		}
 	} else if (json_object_get(root, "nftables") != NULL) {
 		rs = nft_ruleset_alloc();
 		if (rs != NULL) {
-			if (nft_ruleset_parse(rs, NFT_RULESET_PARSE_JSON, json) == 0)
-				ret = compare_test(TEST_JSON_RULESET, rs, filename);
+			if (nft_ruleset_parse(rs,
+					      NFT_RULESET_PARSE_JSON, json) == 0)
+				ret = compare_test(TEST_JSON_RULESET,
+						   rs, filename);
 			else
 				goto failparsing;
 
 			nft_ruleset_free(rs);
-			}
+		}
 	}
 
 	free(json);
@@ -292,7 +300,8 @@ static int test_xml(const char *filename)
 		t = nft_table_alloc();
 		if (t != NULL) {
 			if (nft_table_parse(t, NFT_TABLE_PARSE_XML, xml) == 0)
-				ret = compare_test(TEST_XML_TABLE, t, filename);
+				ret = compare_test(TEST_XML_TABLE,
+						   t, filename);
 			else
 				goto failparsing;
 
@@ -302,7 +311,8 @@ static int test_xml(const char *filename)
 		c = nft_chain_alloc();
 		if (c != NULL) {
 			if (nft_chain_parse(c, NFT_CHAIN_PARSE_XML, xml) == 0)
-				ret = compare_test(TEST_XML_CHAIN, c, filename);
+				ret = compare_test(TEST_XML_CHAIN,
+						   c, filename);
 			else
 				goto failparsing;
 

[libnftables PATCH 6/6] test: report compilation without support

[Arturo Borrero Gonzalez]

Print a message when there is no support for some parser.

Remove those EOPNOTSUPPs because they are unused.

Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
---
 tests/nft-parsing-test.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/nft-parsing-test.c b/tests/nft-parsing-test.c
index e147e79..32c8837 100644
--- a/tests/nft-parsing-test.c
+++ b/tests/nft-parsing-test.c
@@ -260,7 +260,7 @@ failparsing:
 	json_decref(root);
 	return -1;
 #else
-	errno = EOPNOTSUPP;
+	printf("Compiled without support for JSON.\n");
 	return -1;
 #endif
 }
@@ -361,7 +361,7 @@ failparsing:
 	       strerror(errno));
 	return -1;
 #else
-	errno = EOPNOTSUPP;
+	printf("Compiled without support for XML.\n");
 	return -1;
 #endif
 }

Re: [libnftables PATCH 1/6] test: return EXIT_FAILURE if some error was found

[Pablo Neira Ayuso]

On Thu, Oct 31, 2013 at 01:36:18PM +0100, Arturo Borrero Gonzalez wrote:
> Before this patch, 0 was returned unconditionally.

Applied, thanks.

Re: [libnftables PATCH 2/6] test: fix memleak in XML testing

[Pablo Neira Ayuso]

On Thu, Oct 31, 2013 at 01:36:23PM +0100, Arturo Borrero Gonzalez wrote:
> Ensure the tree is freed when done.

Applied, thanks.

Re: [libnftables PATCH 3/6] test: use constants for coloured strings

[Pablo Neira Ayuso]

On Thu, Oct 31, 2013 at 01:36:29PM +0100, Arturo Borrero Gonzalez wrote:
> Use constants for coloured strings, which have a ugly format.
> 
> Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
> ---
>  tests/nft-parsing-test.c |   17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/tests/nft-parsing-test.c b/tests/nft-parsing-test.c
> index 5eaaa1e..5333dbd 100644
> --- a/tests/nft-parsing-test.c
> +++ b/tests/nft-parsing-test.c
> @@ -33,6 +33,9 @@ enum {
>  	TEST_JSON_RULESET,
>  };
>  
> +#define FAILED_STR "\033[31mFAILED\e[0m"
> +#define OK_STR "\033[32mOK\e[0m"

I don't like this macro thing, sorry.

Re: [libnftables PATCH 4/6] test: report errors building XML tree

[Pablo Neira Ayuso]

On Thu, Oct 31, 2013 at 01:36:34PM +0100, Arturo Borrero Gonzalez wrote:
> Report error when building XML tree.

Applied, thanks.

Re: [libnftables PATCH 5/6] test: style cleanup

[Pablo Neira Ayuso]

On Thu, Oct 31, 2013 at 01:36:39PM +0100, Arturo Borrero Gonzalez wrote:
> Some style cleanups.
> 
> Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
> ---
>  tests/nft-parsing-test.c |   34 ++++++++++++++++++++++------------
>  1 file changed, 22 insertions(+), 12 deletions(-)
> 
> diff --git a/tests/nft-parsing-test.c b/tests/nft-parsing-test.c
> index 3d26c3a..e147e79 100644
> --- a/tests/nft-parsing-test.c
> +++ b/tests/nft-parsing-test.c
> @@ -193,8 +193,10 @@ static int test_json(const char *filename)
>  	if (json_object_get(root, "table") != NULL) {
>  		t = nft_table_alloc();
>  		if (t != NULL) {
> -			if (nft_table_parse(t, NFT_TABLE_PARSE_JSON, json) == 0)
> -				ret = compare_test(TEST_JSON_TABLE, t, filename);
> +			if (nft_table_parse(t,
> +					    NFT_TABLE_PARSE_JSON, json) == 0)
> +				ret = compare_test(TEST_JSON_TABLE,
> +						   t, filename);
>  			else
>  				goto failparsing;

If you need to break lines like this, it usually means that this code
needs to be split in smaller functions. Please, send me a clean up by
encapsulating the tests in new smaller functions, eg. test_json_table().

Re: [libnftables PATCH 6/6] test: report compilation without support

[Pablo Neira Ayuso]

On Thu, Oct 31, 2013 at 01:36:44PM +0100, Arturo Borrero Gonzalez wrote:
> Print a message when there is no support for some parser.
> 
> Remove those EOPNOTSUPPs because they are unused.

Applied, thanks.