[PATCH testsuite v3 1/2] tests/file_contexts: refactor existing tests

Change the logging format when reporting an error to better capture the
location and context of the error.

Split tests into functions to better isolate the tested behaviour.

Signed-off-by: Thiébaud Weksteen <[email protected]>
---
Changes since v2:
- Call test functions in test_validate.c

 tests/file_contexts/internal.c       | 16 ++---
 tests/file_contexts/internal.h       | 20 +++++-
 tests/file_contexts/test_lookup.c    | 60 ++++++++++++------
 tests/file_contexts/test_open.c      | 94 +++++++++++++++++-----------
 tests/file_contexts/test_open_base.c | 46 ++++++++++----
 tests/file_contexts/test_validate.c  | 40 +++++++-----
 6 files changed, 181 insertions(+), 95 deletions(-)

diff --git a/tests/file_contexts/internal.c b/tests/file_contexts/internal.c
index 22b5790..ba45dd6 100644
--- a/tests/file_contexts/internal.c
+++ b/tests/file_contexts/internal.c
@@ -10,8 +10,8 @@
 
 #include "internal.h"
 
-void assertContextsMatch(struct selabel_handle *hnd, struct test_t *tests,
-			 size_t n)
+void assertContextsMatch(struct selabel_handle *hnd, const char *log_prefix,
+			 struct test_t *tests, size_t n)
 {
 	for (int i = 0; i < n; i++) {
 		char *context = NULL;
@@ -19,20 +19,22 @@ void assertContextsMatch(struct selabel_handle *hnd, struct test_t *tests,
 
 		if (selabel_lookup(hnd, &context, test.path, S_IFREG)) {
 			if (test.context) {
-				perror("file_contexts:selabel_lookup");
-				fprintf(stderr, "Lookup for %s failed\n", test.path);
+				log_errno("Lookup for %s from %s failed",
+					  test.path, log_prefix);
 				exit(2);
 			}
 			// Expected failure. Continue to the next test.
 			continue;
 		} else if (!test.context) {
-			fprintf(stderr, "Lookup for %s was supposed to failed\n", test.path);
+			log_err("Lookup for %s from %s was supposed to failed but got %s",
+				test.path, log_prefix, context);
 			exit(2);
 		}
 
 		if (strcmp(context, tests[i].context)) {
-			fprintf(stderr, "Lookup for %s returned %s, expected %s\n",
-				tests[i].path, context, tests[i].context);
+			log_err("Lookup for %s from %s returned %s, expected %s",
+				tests[i].path, log_prefix, context,
+				tests[i].context);
 			exit(2);
 		}
 
diff --git a/tests/file_contexts/internal.h b/tests/file_contexts/internal.h
index a07548f..c51bbc6 100644
--- a/tests/file_contexts/internal.h
+++ b/tests/file_contexts/internal.h
@@ -1,8 +1,21 @@
+#include <errno.h>
+#include <string.h>
+
 #include <selinux/label.h>
 #include <selinux/selinux.h>
 
 #define ARRAY_SIZE(a) (sizeof(a) / sizeof((a)[0]))
 
+/* Log an error message with the file name, function name and line */
+#define log_err(fmt, ...)                                                    \
+	fprintf(stderr, "%s:%s:%d: " fmt "\n", __FILE__, __func__, __LINE__, \
+		##__VA_ARGS__)
+
+/* Log an error message as well as errno */
+#define log_errno(fmt, ...)                                            \
+	fprintf(stderr, "%s:%s:%d " fmt " (%s)\n", __FILE__, __func__, \
+		__LINE__, ##__VA_ARGS__, strerror(errno))
+
 /* Structure to capture a test. The |path| will be resolved and expected to
  * match the |context|. If |context| is NULL, the lookup is expected to fail. */
 struct test_t {
@@ -11,6 +24,7 @@ struct test_t {
 };
 
 /* Assert that all paths described in |tests| resolve appropriately. |hnd| must
- * be opened. |n| is the number of tests in |tests|. */
-void assertContextsMatch(struct selabel_handle *hnd, struct test_t *tests,
-			 size_t n);
+ * be opened. |n| is the number of tests in |tests|. |log_prefix| is added to
+ * any log message */
+void assertContextsMatch(struct selabel_handle *hnd, const char *log_prefix,
+			 struct test_t *tests, size_t n);
diff --git a/tests/file_contexts/test_lookup.c b/tests/file_contexts/test_lookup.c
index 9a54d1d..5957db4 100644
--- a/tests/file_contexts/test_lookup.c
+++ b/tests/file_contexts/test_lookup.c
@@ -9,38 +9,58 @@
 
 #include "internal.h"
 
-int main(int argc, char **argv)
+void test_lookup(const char *basedir)
 {
-	struct selabel_handle *hnd;
-
-	if (argc != 2) {
-		fprintf(stderr, "basedir not provided\n");
-		exit(1);
-	}
-
 	char *path;
-	asprintf(&path, "%s/f2.fc", argv[1]);
-	struct selinux_opt f2_opts[] = {
-		{ .type = SELABEL_OPT_PATH, .value = path }
+	asprintf(&path, "%s/f2.fc", basedir);
+	struct selinux_opt f2_opts[] = { {
+			.type = SELABEL_OPT_PATH,
+			.value = path
+		}
 	};
 
-	hnd = selabel_open(SELABEL_CTX_FILE, f2_opts, ARRAY_SIZE(f2_opts));
-
+	struct selabel_handle *hnd =
+		selabel_open(SELABEL_CTX_FILE, f2_opts, ARRAY_SIZE(f2_opts));
 	free(path);
 
 	if (!hnd) {
-		perror("file_context:f2_options");
+		log_errno("Unable to open file backend");
 		exit(2);
 	}
 
 	struct test_t tests[] = {
-		{ .path = "/base", .context = "system_u:object_r:test_base_t:s0" },
-		{ .path = "/base/unkown", .context = "system_u:object_r:test_base_wildcard_t:s0" },
-		{ .path = "/base/sub", .context = "system_u:object_r:test_base_sub_t:s0" },
-		{ .path = "/base/file.list", .context = "system_u:object_r:test_file_list_t:s0" },
-		{ .path = "/base/file_list", .context = "system_u:object_r:test_base_wildcard_t:s0" },
+		{
+			.path = "/base",
+			.context = "system_u:object_r:test_base_t:s0"
+		},
+		{
+			.path = "/base/unkown",
+			.context = "system_u:object_r:test_base_wildcard_t:s0"
+		},
+		{
+			.path = "/base/sub",
+			.context = "system_u:object_r:test_base_sub_t:s0"
+		},
+		{
+			.path = "/base/file.list",
+			.context = "system_u:object_r:test_file_list_t:s0"
+		},
+		{
+			.path = "/base/file_list",
+			.context = "system_u:object_r:test_base_wildcard_t:s0"
+		},
 	};
-	assertContextsMatch(hnd, tests, ARRAY_SIZE(tests));
+	assertContextsMatch(hnd, __func__, tests, ARRAY_SIZE(tests));
+}
+
+int main(int argc, char **argv)
+{
+	if (argc != 2) {
+		log_err("basedir not provided");
+		exit(1);
+	}
+
+	test_lookup(argv[1]);
 
 	return 0;
 }
diff --git a/tests/file_contexts/test_open.c b/tests/file_contexts/test_open.c
index eae5691..4dd3300 100644
--- a/tests/file_contexts/test_open.c
+++ b/tests/file_contexts/test_open.c
@@ -1,6 +1,5 @@
 #include <stdio.h>
 #include <stdlib.h>
-#include <string.h>
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <unistd.h>
@@ -8,75 +7,96 @@
 #include <selinux/label.h>
 #include <selinux/selinux.h>
 
-int main(int argc, char **argv)
+#include "internal.h"
+
+void test_no_options(void)
 {
-	bool has_default_path = false;
-	struct selabel_handle *hnd;
 	struct stat default_stat;
+	if (stat(selinux_file_context_path(), &default_stat))
+		return;
 
-	if (argc != 2) {
-		fprintf(stderr, "basedir not provided\n");
-		exit(1);
-	}
+	/* Empty options */
+	struct selabel_handle *hnd = selabel_open(
+					     SELABEL_CTX_FILE, /* options= */ NULL, /* nopt= */ 0);
 
-	const char *default_path = selinux_file_context_path();
-	if (!stat(default_path, &default_stat)) {
-		has_default_path = true;
+	if (!hnd) {
+		log_errno("Unable to open default content file");
+		exit(2);
 	}
+	selabel_close(hnd);
+}
 
-	if (has_default_path) {
-		/* Empty options */
-		hnd = selabel_open(SELABEL_CTX_FILE, /* options= */ NULL, /* nopt= */ 0);
+void test_null_path(void)
+{
+	struct stat default_stat;
+	if (stat(selinux_file_context_path(), &default_stat))
+		return;
 
-		if (!hnd) {
-			perror("file_context:no_options");
-			exit(2);
+	/* Default options */
+	struct selinux_opt null_opts[] = { {
+			.type = SELABEL_OPT_PATH,
+			.value = NULL
 		}
-		selabel_close(hnd);
-
-		/* Default options */
-		struct selinux_opt null_opts[] = {
-			{ .type = SELABEL_OPT_PATH, .value = NULL }
-		};
+	};
 
-		hnd = selabel_open(SELABEL_CTX_FILE, /* options= */ null_opts, /* nopt= */ 1);
+	struct selabel_handle *hnd = selabel_open(SELABEL_CTX_FILE, null_opts,
+						  ARRAY_SIZE(null_opts));
 
-		if (!hnd) {
-			perror("file_context:default_options");
-			exit(2);
-		}
-		selabel_close(hnd);
+	if (!hnd) {
+		log_errno("Unable to open default content file");
+		exit(2);
 	}
+	selabel_close(hnd);
+}
 
+void test_valid_path(const char *basedir)
+{
 	/* f1.fc file */
 	char *path;
-	asprintf(&path, "%s/f1.fc", argv[1]);
-	struct selinux_opt f1_opts[] = {
-		{ .type = SELABEL_OPT_PATH, .value = path }
+	asprintf(&path, "%s/f1.fc", basedir);
+	struct selinux_opt f1_opts[] = { {
+			.type = SELABEL_OPT_PATH,
+			.value = path
+		}
 	};
 
-	hnd = selabel_open(SELABEL_CTX_FILE, /* options= */ f1_opts, /* nopt= */ 1);
+	struct selabel_handle *hnd =
+		selabel_open(SELABEL_CTX_FILE, f1_opts, ARRAY_SIZE(f1_opts));
 	free(path);
 
 	if (!hnd) {
-		perror("file_context:f1_options");
+		log_errno("Unable to open file backend");
 		exit(2);
 	}
 
 	char *context = NULL;
 	if (selabel_lookup(hnd, &context, "/", S_IFREG)) {
-		perror("file_contexts:f1_lookup");
+		log_errno("Unable to lookup \"/\"");
 		exit(2);
 	}
 
-	if (strcmp(context, "system_u:object_r:rootfs:s0")) {
-		perror("file_contexts:f1_strcmp");
+	const char *expected = "system_u:object_r:rootfs:s0";
+	if (strcmp(context, expected)) {
+		log_err("Incorrect context returned, expected %s got %s",
+			expected, context);
 		exit(2);
 	}
 
 	free(context);
 
 	selabel_close(hnd);
+}
+
+int main(int argc, char **argv)
+{
+	if (argc != 2) {
+		log_err("basedir not provided");
+		exit(1);
+	}
+
+	test_no_options();
+	test_null_path();
+	test_valid_path(argv[1]);
 
 	return 0;
 }
diff --git a/tests/file_contexts/test_open_base.c b/tests/file_contexts/test_open_base.c
index 33b0cac..3e4b4c6 100644
--- a/tests/file_contexts/test_open_base.c
+++ b/tests/file_contexts/test_open_base.c
@@ -22,26 +22,40 @@ void test_default_options(const char *basedir)
 
 	char *path;
 	asprintf(&path, "%s/f3.fc", basedir);
-	struct selinux_opt f3_opts[] = {
-		{ .type = SELABEL_OPT_PATH, .value = path }
+	struct selinux_opt f3_opts[] = { {
+			.type = SELABEL_OPT_PATH,
+			.value = path
+		}
 	};
 
 	hnd = selabel_open(SELABEL_CTX_FILE, f3_opts, ARRAY_SIZE(f3_opts));
 	free(path);
 
 	if (!hnd) {
-		perror("file_context:f3_default_options");
+		log_errno("Unable to open file backend");
 		exit(2);
 	}
 
 	struct test_t tests[] = {
 		{ .path = "/", .context = "system_u:object_r:rootfs:s0" },
-		{ .path = "/local", .context = "system_u:object_r:test_local:s0" },
-		{ .path = "/homedirs", .context = "system_u:object_r:test_homedirs:s0" },
-		{ .path = "/sub", .context = "system_u:object_r:test_subbed:s0" },
-		{ .path = "/sub_dist", .context = "system_u:object_r:test_subbed:s0" },
+		{
+			.path = "/local",
+			.context = "system_u:object_r:test_local:s0"
+		},
+		{
+			.path = "/homedirs",
+			.context = "system_u:object_r:test_homedirs:s0"
+		},
+		{
+			.path = "/sub",
+			.context = "system_u:object_r:test_subbed:s0"
+		},
+		{
+			.path = "/sub_dist",
+			.context = "system_u:object_r:test_subbed:s0"
+		},
 	};
-	assertContextsMatch(hnd, tests, ARRAY_SIZE(tests));
+	assertContextsMatch(hnd, __func__, tests, ARRAY_SIZE(tests));
 
 	selabel_close(hnd);
 }
@@ -62,7 +76,7 @@ void test_base_only_option(const char *basedir)
 	free(path);
 
 	if (!hnd) {
-		perror("file_context:f3_base_only_options");
+		log_errno("Unable to open file backend");
 		exit(2);
 	}
 
@@ -70,10 +84,16 @@ void test_base_only_option(const char *basedir)
 		{ .path = "/", .context = "system_u:object_r:rootfs:s0" },
 		{ .path = "/local", .context = NULL },
 		{ .path = "/homedirs", .context = NULL },
-		{ .path = "/sub", .context = "system_u:object_r:test_subbed:s0" },
-		{ .path = "/sub_dist", .context = "system_u:object_r:test_subbed:s0" },
+		{
+			.path = "/sub",
+			.context = "system_u:object_r:test_subbed:s0"
+		},
+		{
+			.path = "/sub_dist",
+			.context = "system_u:object_r:test_subbed:s0"
+		},
 	};
-	assertContextsMatch(hnd, tests, ARRAY_SIZE(tests));
+	assertContextsMatch(hnd, __func__, tests, ARRAY_SIZE(tests));
 
 	selabel_close(hnd);
 }
@@ -81,7 +101,7 @@ void test_base_only_option(const char *basedir)
 int main(int argc, char **argv)
 {
 	if (argc != 2) {
-		fprintf(stderr, "basedir not provided\n");
+		log_err("basedir not provided");
 		exit(1);
 	}
 
diff --git a/tests/file_contexts/test_validate.c b/tests/file_contexts/test_validate.c
index 8abefed..4d9e920 100644
--- a/tests/file_contexts/test_validate.c
+++ b/tests/file_contexts/test_validate.c
@@ -1,6 +1,5 @@
 #include <stdio.h>
 #include <stdlib.h>
-#include <string.h>
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <unistd.h>
@@ -10,47 +9,58 @@
 
 #include "internal.h"
 
-int main(int argc, char **argv)
+void test_validate_unknown_fails(const char *basedir)
 {
 	char *path;
-	struct selabel_handle *hnd;
-
-	if (argc != 2) {
-		fprintf(stderr, "basedir not provided\n");
-		exit(1);
-	}
-
-	asprintf(&path, "%s/f1.fc", argv[1]);
+	asprintf(&path, "%s/f1.fc", basedir);
 	struct selinux_opt f1_opts[] = {
 		{ .type = SELABEL_OPT_PATH, .value = path },
 		{ .type = SELABEL_OPT_VALIDATE, .value = "1" }
 	};
 
 	/* f1 types are not defined in the test policy. */
-	hnd = selabel_open(SELABEL_CTX_FILE, f1_opts, ARRAY_SIZE(f1_opts));
+	struct selabel_handle *hnd =
+		selabel_open(SELABEL_CTX_FILE, f1_opts, ARRAY_SIZE(f1_opts));
 	free(path);
 
 	if (hnd) {
-		fprintf(stderr, "The validation of f1 should have failed.\n");
+		log_err("The validation of f1 should have failed");
+		selabel_close(hnd);
 		exit(2);
 	}
+}
 
-	asprintf(&path, "%s/f2.fc", argv[1]);
+void test_validate_known_succeeds(const char *basedir)
+{
+	char *path;
+	asprintf(&path, "%s/f2.fc", basedir);
 	struct selinux_opt f2_opts[] = {
 		{ .type = SELABEL_OPT_PATH, .value = path },
 		{ .type = SELABEL_OPT_VALIDATE, .value = "1" }
 	};
 
 	/* f2 types are defined in the test policy. */
-	hnd = selabel_open(SELABEL_CTX_FILE, f2_opts, ARRAY_SIZE(f2_opts));
+	struct selabel_handle *hnd =
+		selabel_open(SELABEL_CTX_FILE, f2_opts, ARRAY_SIZE(f2_opts));
 	free(path);
 
 	if (!hnd) {
-		perror("Unable to read valid file_contexts");
+		log_errno("Unable to read valid file_contexts");
 		exit(2);
 	}
 
 	selabel_close(hnd);
+}
+
+int main(int argc, char **argv)
+{
+	if (argc != 2) {
+		log_err("basedir not provided");
+		exit(1);
+	}
+
+	test_validate_unknown_fails(argv[1]);
+	test_validate_known_succeeds(argv[1]);
 
 	return 0;
 }
-- 
2.55.0.229.g6434b31f56-goog