[LTP] [PATCH v3 1/3] lib: adding .supported_archs field in tst_test structure
Cyril Hrubis
chrubis@suse.cz
Mon Nov 8 13:34:38 CET 2021
Hi!
> Testcases for specific arch should be limited on that only being supported
> platform to run, we now involve a .supported_archs to achieve this feature
> in LTP library. All you need to run a test on the expected arch is to set
> the '.supported_archs' array in the 'struct tst_test' to choose the required
> arch list. e.g.
>
> .supported_archs = (const char *const []){"x86_64", "ppc64", NULL}
>
> This helps move the TCONF info from code to tst_test metadata as well.
>
> And, we also export a struct tst_arch to save the system architecture
> for using in the whole test cases.
>
> extern struct arch {
> char name[16];
> enum tst_arch_type type;
> } tst_arch;
>
> Signed-off-by: Li Wang <liwang@redhat.com>
> ---
> doc/c-test-api.txt | 36 +++++++++++++++
> include/tst_arch.h | 39 ++++++++++++++++
> include/tst_test.h | 10 +++++
> lib/tst_arch.c | 108 +++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 193 insertions(+)
> create mode 100644 include/tst_arch.h
> create mode 100644 lib/tst_arch.c
>
> diff --git a/doc/c-test-api.txt b/doc/c-test-api.txt
> index 3127018a4..5e32b7075 100644
> --- a/doc/c-test-api.txt
> +++ b/doc/c-test-api.txt
> @@ -2261,6 +2261,42 @@ struct tst_test test = {
> };
> -------------------------------------------------------------------------------
>
> +1.39 Testing on the specific architecture
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +Testcases for specific arch should be limited on that only being supported
> +platform to run, we now involve a .supported_archs to achieve this feature
> +in LTP library. All you need to run a test on the expected arch is to set
> +the '.supported_archs' array in the 'struct tst_test' to choose the required
> +arch list. e.g.
> +
> + .supported_archs = (const char *const []){"x86_64", "ppc64", NULL}
> +
> +This helps move the TCONF info from code to tst_test metadata as well.
> +
> +And, we also export a struct tst_arch to save the system architecture for
> +using in the whole test cases.
> +
> + extern struct arch {
> + char name[16];
> + enum tst_arch_type type;
> + } tst_arch;
> +
> +[source,c]
> +-------------------------------------------------------------------------------
> +#include "tst_test.h"
> +
> +static struct tst_test test = {
> + ...
> + .setup = setup,
> + .supported_archs = (const char *const []) {
> + "x86_64",
> + "ppc64",
> + "s390x",
> + NULL
> + },
> +};
> +-------------------------------------------------------------------------------
> +
> 2. Common problems
> ------------------
>
> diff --git a/include/tst_arch.h b/include/tst_arch.h
> new file mode 100644
> index 000000000..784c3093b
> --- /dev/null
> +++ b/include/tst_arch.h
> @@ -0,0 +1,39 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later
> + * Copyright (c) 2021 Li Wang <liwang@redhat.com>
> + */
> +
> +#ifndef TST_ARCH_H__
> +#define TST_ARCH_H__
> +
> +enum tst_arch_type {
> + TST_I386,
> + TST_X86,
Why do we have both i386 and X86 here? Isn't __i386__ synonymous for
__x86__ does gcc even define __x86__?
I doubt that we care about the differencies between i386, i586 and i686
at all. I would just keep TST_X86 in the list for any 32bit intel
compatible hardware.
> + TST_X86_64,
> + TST_IA64,
> + TST_PPC,
> + TST_PPC64,
> + TST_S390,
> + TST_S390X,
> + TST_ARM,
> + TST_AARCH64,
> + TST_SPARC,
> +};
> +
> +/*
> + * This tst_arch is to save the system architecture for
> + * using in the whole testcase.
> + */
> +extern struct arch {
> + char name[16];
> + enum tst_arch_type type;
> +} tst_arch;
the struct should be prefixed with tst_ as well. i.e.
extern struct tst_arch {
...
> +/*
> + * Check if test platform is in the given arch list. If yes return 1,
> + * otherwise return 0.
> + *
> + * @archlist A NULL terminated array of architectures to support.
> + */
> +int tst_is_on_arch(const char *const *archlist);
> +
> +#endif /* TST_ARCH_H__ */
> diff --git a/include/tst_test.h b/include/tst_test.h
> index 3dcb45de0..7611520ee 100644
> --- a/include/tst_test.h
> +++ b/include/tst_test.h
> @@ -43,6 +43,7 @@
> #include "tst_fips.h"
> #include "tst_taint.h"
> #include "tst_memutils.h"
> +#include "tst_arch.h"
>
> /*
> * Reports testcase result.
> @@ -139,6 +140,12 @@ struct tst_test {
>
> const char *min_kver;
>
> + /*
> + * The supported_archs is a NULL terminated list of archs the test
> + * does support.
> + */
> + const char *const *supported_archs;
> +
> /* If set the test is compiled out */
> const char *tconf_msg;
>
> @@ -316,6 +323,9 @@ static struct tst_test test;
>
> int main(int argc, char *argv[])
> {
> + if (!tst_is_on_arch(test.supported_archs))
> + tst_brk(TCONF, "This arch '%s' is not supported for test!", tst_arch.name);
Why are you putting this check into the header? All the other checks are
in the do_setup() in lib/tst_test.c
> tst_run_tcases(argc, argv, &test);
> }
>
> diff --git a/lib/tst_arch.c b/lib/tst_arch.c
> new file mode 100644
> index 000000000..ae5817075
> --- /dev/null
> +++ b/lib/tst_arch.c
> @@ -0,0 +1,108 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later
> + * Copyright (c) 2021 Li Wang <liwang@redhat.com>
> + */
> +
> +#include <string.h>
> +#include <stdlib.h>
> +
> +#define TST_NO_DEFAULT_MAIN
> +#include "tst_arch.h"
> +#include "tst_test.h"
> +
> +struct arch tst_arch;
> +
> +static const char *const arch_type_list[] = {
> + "i386",
> + "x86",
> + "x86_64",
> + "ia64",
> + "ppc",
> + "ppc64",
> + "s390",
> + "s390x",
> + "arm",
> + "aarch64",
> + "sparc",
> + NULL
> +};
> +
> +static int is_matched_arch(const char *arch, const char *name)
> +{
> + if (strcmp(arch, name) == 0)
> + return 1;
> +
> + return 0;
> +}
> +
> +static int is_valid_arch_name(const char *name)
> +{
> + unsigned int i;
> +
> + for (i = 0; arch_type_list[i]; i++) {
> + if (is_matched_arch(arch_type_list[i], name))
> + return 1;
> + }
> +
> + return 0;
> +}
> +
> +static void get_system_arch(void)
> +{
> +#if defined(__i386__)
> + strcpy(tst_arch.name, "i386");
> + tst_arch.type = TST_I386;
> +#elif defined(__x86__)
> + strcpy(tst_arch.name, "x86");
> + tst_arch.type = TST_X86;
> +#elif defined(__x86_64__)
> + strcpy(tst_arch.name, "x86_64");
> + tst_arch.type = TST_X86_64;
> +#elif defined(__ia64__)
> + strcpy(tst_arch.name, "ia64");
> + tst_arch.type = TST_IA64;
> +#elif defined(__powerpc__)
> + strcpy(tst_arch.name, "ppc");
> + tst_arch.type = TST_PPC;
> +#elif defined(__powerpc64__)
> + strcpy(tst_arch.name, "ppc64");
> + tst_arch.type = TST_PPC64;
> +#elif defined(__s390x__)
> + strcpy(tst_arch.name, "s390x");
> + tst_arch.type = TST_S390X;
> +#elif defined(__s390__)
> + strcpy(tst_arch.name, "s390");
> + tst_arch.type = TST_S390;
> +#elif defined(__arm__)
> + strcpy(tst_arch.name, "s390x");
> + tst_arch.type = TST_ARM;
> +#elif defined(__aarch64__)
> + strcpy(tst_arch.name, "aarch64");
> + tst_arch.type = TST_AARCH64;
> +#elif defined(__sparc__)
> + strcpy(tst_arch.name, "sparc");
> + tst_arch.type = TST_SPARC;
> +#endif
> +}
Actually we can initialize this staically as:
const struct arch tst_arch = {
#if defined(__x86_64__)
.name = "x86_64",
.type = TST_X86_64,
#elif
...
#else
.name = "unknown",
.type = TST_ARCH_UNKNOWN,
#endif
};
> +int tst_is_on_arch(const char *const *archlist)
> +{
> + unsigned int i;
> +
> + get_system_arch();
> +
> + if (archlist == NULL)
> + return 1;
Just if (!archlist)
> + for (i = 0; archlist[i]; i++) {
> + if (!is_valid_arch_name(archlist[i]))
> + tst_brk(TBROK, "%s is invalid arch, please reset!",
^
fix?
> + archlist[i]);
> + }
> +
> + for (i = 0; archlist[i]; i++) {
> + if (is_matched_arch(tst_arch.name, archlist[i]))
I would just do !strcmp() here, there is no point in havin a function
that just calls strcmp() and returns the result.
> + return 1;
> + }
> +
> + return 0;
> +}
> --
> 2.31.1
>
>
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp
--
Cyril Hrubis
chrubis@suse.cz
More information about the ltp
mailing list