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

On Sun, Jul 26, 2026 at 10:52 PM Thiébaud Weksteen <[email protected]> wrote:
>
> 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]>

Acked-by: Stephen Smalley <[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
>