lib: adding tst_on_arch function in library

Testcases for specified arch should be limited on that only being supported platform to run, we now create a function tst_on_arch to achieve this new feature in LTP library. All you need to run a test on the expected arch is to set the '.arch' string in the 'struct tst_test' to choose the required arch list. e.g. '.arch = "x86_64, i386"'.

For more complicated usages such as customize your test code for a special arch, the tst_on_arch function could be invoked in the test directly.

Signed-off-by Li Wang <[email protected]>

---
 doc/test-writing-guidelines.txt | 54 +++++++++++++++++++++++++
 include/tst_arch.h              | 16 ++++++++
 include/tst_test.h              |  7 +++-
 lib/tst_arch.c                  | 71 +++++++++++++++++++++++++++++++++
 4 files changed, 147 insertions(+), 1 deletion(-)
 create mode 100644 include/tst_arch.h
 create mode 100644 lib/tst_arch.c

doc/test-writing-guidelines.txt +35 -0

index f1912dc12..10442d756 100644
@@ -1668,6 +1668,60 @@ sturct tst_test test = {
 };
 -------------------------------------------------------------------------------
 
+2.2.30 Testing on specified architecture
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Testcases for specified arch should be limited on that only being supported
+platform to run, we now create a function tst_on_arch to achieve this new
+feature in LTP library.  All you need to run a test on the expected arch is
+to set the '.arch' string in the 'struct tst_test' to choose the required
+arch list. e.g. '.arch = "x86_64, i386"'.
+
+[source,c]
+-------------------------------------------------------------------------------
+#include "tst_test.h"
+
+static void setup(void)
+{
+	...
+}
+
+static struct tst_test test = {
+	...
+	.setup = setup,
+	.arch = "x86_64, i386",
+	...
+}
+-------------------------------------------------------------------------------
+
+For more complicated usages such as customize your test code for a special
+arch, the tst_on_arch function could be invoked in the test directly.
+
+[source,c]
+-------------------------------------------------------------------------------
+#include "tst_test.h"
+
+static void do_test(void)
+{

doc/test-writing-guidelines.txt +9 -0

+}
+
+static struct tst_test test = {
+	...
+	.test_all = do_test,
+	...
+}
+-------------------------------------------------------------------------------
+
 
 2.3 Writing a testcase in shell
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

include/tst_arch.h new file +16 -0

new file mode 100644
index 000000000..7bf0493ce
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later
+ * Copyright (c) 2019 Li Wang <[email protected]>
+ */
+
+#ifndef TST_ARCH_H__
+#define TST_ARCH_H__
+
+/*
+ * Check if test platform is in the given arch list. If yes return 1,
+ * otherwise return 0.
+ *
+ * @arch, NULL or vliad arch list
+ */
+int tst_on_arch(const char *arch);
+
+#endif /* TST_ARCH_H__ */

include/tst_test.h +3 -1

index 8bdf38482..cafcb1a89 100644
@@ -28,6 +28,7 @@
 #include "tst_atomic.h"
 #include "tst_kvercmp.h"
 #include "tst_clone.h"
+#include "tst_arch.h"
 #include "tst_kernel.h"
 #include "tst_minmax.h"
 #include "tst_get_bad_addr.h"

@@ -114,6 +115,8 @@ struct tst_test {
 
 	const char *min_kver;
 
+	const char *arch;
+
 	/* If set the test is compiled out */
 	const char *tconf_msg;
 

@@ -253,7 +256,6 @@ const char *tst_strstatus(int status);
 unsigned int tst_timeout_remaining(void);
 void tst_set_timeout(int timeout);
 
-
 /*
  * Returns path to the test temporary directory in a newly allocated buffer.
  */

lib/tst_arch.c new file +58 -0

new file mode 100644
index 000000000..a9f2775b4
@@ -0,0 +1,71 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later
+ * Copyright (c) 2019 Li Wang <[email protected]>
+ */
+
+#include <string.h>
+
+#define TST_NO_DEFAULT_MAIN
+#include "tst_arch.h"
+#include "tst_test.h"
+
+static const char *const arch_type_list[] = {
+	"i386",
+	"x86",
+	"x86_64",
+	"ia64",
+	"ppc",
+	"ppc64",
+	"s390",
+	"s390x",
+	"arm",
+	"aarch64",
+	"sparc",
+	NULL
+};
+
+static int is_valid_arch(const char *arch)
+{
+	unsigned int i;
+
+	for (i = 0; arch_type_list[i]; i++) {
+		if (strstr(arch, arch_type_list[i]))
+			return 1;
+	}
+
+	return 0;
+}
+
+int tst_on_arch(const char *arch)
+{
+#if defined(__i386__)
+	char *tst_arch = "i386";
+#elif defined(__x86__)
+	char *tst_arch = "x86";
+#elif defined(__x86_64__)
+	char *tst_arch = "x86_64";
+#elif defined(__ia64__)
+	char *tst_arch = "ia64";
+#elif defined(__powerpc__)
+	char *tst_arch = "ppc";
+#elif defined(__powerpc64__)
+	char *tst_arch = "ppc64";
+#elif defined(__s390__)
+	char *tst_arch = "s390";
+#elif defined(__s390x__)
+	char *tst_arch = "s390x";
+#elif defined(__arm__)
+	char *tst_arch = "arm";
+#elif defined(__arch64__)

lib/tst_arch.c new file +2 -0

+	return 0;
+}

Signature · 5 lines

2.17.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Hi!

Quoted 1 line

Testcases for specified arch should be limited on that only being

supported

Quoted 2 lines

platform to run, we now create a function tst_on_arch to achieve this new feature in LTP library. All you need to run a test on the expected arch

is

Quoted 2 lines

to set the '.arch' string in the 'struct tst_test' to choose the required arch list. e.g. '.arch = "x86_64, i386"'.

There is no point in adding the coma between the architectures, i.e. this should be just .arch = "x86_64 i386".

Quoted 2 lines

For more complicated usages such as customize your test code for a

special

Quoted 12 lines

arch, the tst_on_arch function could be invoked in the test directly.

Signed-off-by Li Wang <[email protected]>

---
 doc/test-writing-guidelines.txt | 54 +++++++++++++++++++++++++
 include/tst_arch.h              | 16 ++++++++
 include/tst_test.h              |  7 +++-
 lib/tst_arch.c                  | 71 +++++++++++++++++++++++++++++++++
 4 files changed, 147 insertions(+), 1 deletion(-)
 create mode 100644 include/tst_arch.h
 create mode 100644 lib/tst_arch.c

Review

Quoted 1 line

(quoted diff) +0 -0

b/doc/test-writing-guidelines.txt

doc/test-writing-guidelines.txt sturct tst_test test = { · line 1669

Quoted 6 lines

index f1912dc12..10442d756 100644

doc/test-writing-guidelines.txt +0 -0

@@ -1668,6 +1668,60 @@ sturct tst_test test = {
 };
 

-------------------------------------------------------------------------------

doc/test-writing-guidelines.txt sturct tst_test test = {

Quoted 5 lines

doc/test-writing-guidelines.txt +4 -0

+2.2.30 Testing on specified architecture
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Testcases for specified arch should be limited on that only being

supported

doc/test-writing-guidelines.txt sturct tst_test test = { · line 1674

Quoted 1 line

doc/test-writing-guidelines.txt +1 -0

+platform to run, we now create a function tst_on_arch to achieve this

new

doc/test-writing-guidelines.txt sturct tst_test test = { · line 1675

Quoted 1 line

doc/test-writing-guidelines.txt +1 -0

+feature in LTP library.  All you need to run a test on the expected

arch is

doc/test-writing-guidelines.txt sturct tst_test test = { · line 1676

Quoted 1 line

doc/test-writing-guidelines.txt +1 -0

+to set the '.arch' string in the 'struct tst_test' to choose the

required

doc/test-writing-guidelines.txt sturct tst_test test = { · line 1725

Quoted 4 lines

doc/test-writing-guidelines.txt +3 -0

+arch list. e.g. '.arch = "x86_64, i386"'.
+
+[source,c]
 

+-------------------------------------------------------------------------------

Quoted 4 lines

doc/test-writing-guidelines.txt +4 -0

+#include "tst_test.h"
+
+static void setup(void)
+{

doc/test-writing-guidelines.txt sturct tst_test test = { · line 1725

Quoted 10 lines

doc/test-writing-guidelines.txt +9 -0

+     ...
+}
+
+static struct tst_test test = {
+     ...
+     .setup = setup,
+     .arch = "x86_64, i386",
+     ...
+}
 

+-------------------------------------------------------------------------------

doc/test-writing-guidelines.txt sturct tst_test test = { · line 1696

Quoted 2 lines

doc/test-writing-guidelines.txt +2 -0

+
+For more complicated usages such as customize your test code for a

special

doc/test-writing-guidelines.txt sturct tst_test test = { · line 1725

Quoted 4 lines

doc/test-writing-guidelines.txt +3 -0

+arch, the tst_on_arch function could be invoked in the test directly.
+
+[source,c]
 

+-------------------------------------------------------------------------------

Quoted 4 lines

doc/test-writing-guidelines.txt +4 -0

+#include "tst_test.h"
+
+static void do_test(void)
+{

doc/test-writing-guidelines.txt sturct tst_test test = { · line 1715

Quoted 10 lines

doc/test-writing-guidelines.txt +10 -0

+     if (tst_on_arch("x86_64, i386")) {
+             /* code for x86_64, i386 */
+             ...
+     } else if (tst_on_arch("ppc64")) {
+             /* code for ppc64 */
+             ...
+     } else if (tst_on_arch("s390, s390x")) {
+             /* code for s390x */
+             ...
+     }

What is the point of the runtime detection here?

It's not like we can run s390x binary on i386, i.e. we know for which architecture we are compiling for at the compile time.

doc/test-writing-guidelines.txt sturct tst_test test = { · line 1725

Quoted 9 lines

doc/test-writing-guidelines.txt +7 -0

+}
+
+static struct tst_test test = {
+     ...
+     .test_all = do_test,
+     ...
+}
 

+-------------------------------------------------------------------------------

Quoted 48 lines

doc/test-writing-guidelines.txt +1 -0

+
 
 2.3 Writing a testcase in shell
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

include/tst_arch.h new file +16 -0

new file mode 100644
index 000000000..7bf0493ce
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later
+ * Copyright (c) 2019 Li Wang <[email protected]>
+ */
+
+#ifndef TST_ARCH_H__
+#define TST_ARCH_H__
+
+/*
+ * Check if test platform is in the given arch list. If yes return 1,
+ * otherwise return 0.
+ *
+ * @arch, NULL or vliad arch list
+ */
+int tst_on_arch(const char *arch);
+
+#endif /* TST_ARCH_H__ */

include/tst_test.h +3 -0

index 8bdf38482..cafcb1a89 100644
@@ -28,6 +28,7 @@
 #include "tst_atomic.h"
 #include "tst_kvercmp.h"
 #include "tst_clone.h"
+#include "tst_arch.h"
 #include "tst_kernel.h"
 #include "tst_minmax.h"
 #include "tst_get_bad_addr.h"

@@ -114,6 +115,8 @@ struct tst_test {
 
      const char *min_kver;
 
+     const char *arch;
+
      /* If set the test is compiled out */
      const char *tconf_msg;
 

include/tst_test.h const char *tst_strstatus(int status); · line 260

Quoted 6 lines

include/tst_test.h +0 -1

@@ -253,7 +256,6 @@ const char *tst_strstatus(int status);
 unsigned int tst_timeout_remaining(void);
 void tst_set_timeout(int timeout);
 
-
 /*
  * Returns path to the test temporary directory in a newly allocated

buffer.

include/tst_test.h static struct tst_test test; · line 271

Quoted 7 lines

include/tst_test.h +2 -0

  */

@@ -265,6 +267,9 @@ static struct tst_test test;
 
 int main(int argc, char *argv[])
 {
+     if (!tst_on_arch(test.arch))
+             tst_brk(TCONF, "Test needs running on %s arch!",

test.arch);

include/tst_test.h static struct tst_test test; · line 274

Quoted 3 lines

include/tst_test.h +1 -0

+
      tst_run_tcases(argc, argv, &test);
 }

This may be a bit cleaner that compiling the test out, but will not save us from arch specific ifdefs completely so I'm not sure it's worth the trouble.

Quoted 65 lines

lib/tst_arch.c new file +58 -0

new file mode 100644
index 000000000..a9f2775b4
@@ -0,0 +1,71 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later
+ * Copyright (c) 2019 Li Wang <[email protected]>
+ */
+
+#include <string.h>
+
+#define TST_NO_DEFAULT_MAIN
+#include "tst_arch.h"
+#include "tst_test.h"
+
+static const char *const arch_type_list[] = {
+     "i386",
+     "x86",
+     "x86_64",
+     "ia64",
+     "ppc",
+     "ppc64",
+     "s390",
+     "s390x",
+     "arm",
+     "aarch64",
+     "sparc",
+     NULL
+};
+
+static int is_valid_arch(const char *arch)
+{
+     unsigned int i;
+
+     for (i = 0; arch_type_list[i]; i++) {
+             if (strstr(arch, arch_type_list[i]))
+                     return 1;
+     }
+
+     return 0;
+}
+
+int tst_on_arch(const char *arch)
+{
+#if defined(__i386__)
+     char *tst_arch = "i386";
+#elif defined(__x86__)
+     char *tst_arch = "x86";
+#elif defined(__x86_64__)
+     char *tst_arch = "x86_64";
+#elif defined(__ia64__)
+     char *tst_arch = "ia64";
+#elif defined(__powerpc__)
+     char *tst_arch = "ppc";
+#elif defined(__powerpc64__)
+     char *tst_arch = "ppc64";
+#elif defined(__s390__)
+     char *tst_arch = "s390";
+#elif defined(__s390x__)
+     char *tst_arch = "s390x";
+#elif defined(__arm__)
+     char *tst_arch = "arm";
+#elif defined(__arch64__)

lib/tst_arch.c line 68

Quoted 10 lines

lib/tst_arch.c new file +10 -0

+     char *tst_arch = "aarch64";
+#elif defined(__sparc__)
+     char *tst_arch = "sparc";
+#endif
+
+     if (arch != NULL && !is_valid_arch(arch))
+             tst_brk(TBROK, "please set valid arches!");
+
+     if (arch == NULL || strstr(arch, tst_arch))
+             return 1;

Isn't using strstr() completely broken here?

Couple of the architecture names are prefixes of the 64bit variant, also validating the architecture by strstr() is kind of pointless, it will match any garbage that contains one of the substrings.

If nothing else we should strdup() the string and then loop over strtok().

Quoted 9 lines

lib/tst_arch.c new file +2 -1

+     return 0;
+}
--

2.17.0

--
Mailing list info: https://lists.linux.it/listinfo/ltp

--
Cyril Hrubis
[email protected]

Quoted 5 lines

What is the point of the runtime detection here?

It's not like we can run s390x binary on i386, i.e. we know for which architecture we are compiling for at the compile time.

You are right. But we still have some chance to do analysis at runtime, if you take a look at patch 2/3, e.g. to parse '/proc/<pid>/maps' in max_map_count.c can be done at runtime detection. That's what I thought we can export the tst_on_arch() as a global function.

Quoted 72 lines

doc/test-writing-guidelines.txt sturct tst_test test = { · line 1722

Quoted 8 lines

doc/test-writing-guidelines.txt +7 -0

+}
+
+static struct tst_test test = {
+     ...
+     .test_all = do_test,
+     ...
+}

+-------------------------------------------------------------------------------

Quoted 48 lines

doc/test-writing-guidelines.txt +1 -0

+
 
 2.3 Writing a testcase in shell
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

include/tst_arch.h new file +16 -0

new file mode 100644
index 000000000..7bf0493ce
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later
+ * Copyright (c) 2019 Li Wang <[email protected]>
+ */
+
+#ifndef TST_ARCH_H__
+#define TST_ARCH_H__
+
+/*
+ * Check if test platform is in the given arch list. If yes return 1,
+ * otherwise return 0.
+ *
+ * @arch, NULL or vliad arch list
+ */
+int tst_on_arch(const char *arch);
+
+#endif /* TST_ARCH_H__ */

include/tst_test.h +3 -0

index 8bdf38482..cafcb1a89 100644
@@ -28,6 +28,7 @@
 #include "tst_atomic.h"
 #include "tst_kvercmp.h"
 #include "tst_clone.h"
+#include "tst_arch.h"
 #include "tst_kernel.h"
 #include "tst_minmax.h"
 #include "tst_get_bad_addr.h"

@@ -114,6 +115,8 @@ struct tst_test {
 
      const char *min_kver;
 
+     const char *arch;
+
      /* If set the test is compiled out */
      const char *tconf_msg;
 

include/tst_test.h const char *tst_strstatus(int status); · line 260

Quoted 6 lines

include/tst_test.h +0 -1

@@ -253,7 +256,6 @@ const char *tst_strstatus(int status);
 unsigned int tst_timeout_remaining(void);
 void tst_set_timeout(int timeout);
 
-
 /*
  * Returns path to the test temporary directory in a newly allocated

buffer.

include/tst_test.h static struct tst_test test; · line 271

Quoted 7 lines

include/tst_test.h +2 -0

  */

@@ -265,6 +267,9 @@ static struct tst_test test;
 
 int main(int argc, char *argv[])
 {
+     if (!tst_on_arch(test.arch))
+             tst_brk(TCONF, "Test needs running on %s arch!",

test.arch);

Quoted 8 lines

include/tst_test.h static struct tst_test test; · line 274

Quoted 3 lines

include/tst_test.h +1 -0

+
      tst_run_tcases(argc, argv, &test);
 }

This may be a bit cleaner that compiling the test out, but will not save us from arch specific ifdefs completely so I'm not sure it's worth the trouble.

Indeed, I also realized that after signing off this patch, we can't replace ifdefs completely via a simple function, since it occurring in the compiling early phase. But anyway I roll out this for comments in case we could find an improved way to do better.