diff mbox

[kvm-unit-tests,v2,02/14] lib: share arm-selftest utility functions

Message ID 1454957594-30601-3-git-send-email-drjones@redhat.com
State Superseded
Headers show

Commit Message

Andrew Jones Feb. 8, 2016, 6:53 p.m. UTC 
arm-selftest has a couple utility functions that could be useful
to other unit tests, even other architectures. So move them out.
split_var moves to lib/util, where we can add other random
utilities over time. assert_args inspires report_abort, which
allows us to report a message, using the current prefix, that
we're aborting (outputs ABORT vs. PASS/FAIL). This is useful
for cases when unit tests can't complete due to missing
dependencies of some sort, such as missing/invalid inputs from
the user.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 arm/selftest.c               | 45 ++++++++++++++------------------------------
 config/config-arm-common.mak |  1 +
 lib/libcflat.h               | 11 ++++++-----
 lib/report.c                 | 16 ++++++++++++++++
 lib/util.c                   | 18 ++++++++++++++++++
 lib/util.h                   | 23 ++++++++++++++++++++++
 6 files changed, 78 insertions(+), 36 deletions(-)
 create mode 100644 lib/util.c
 create mode 100644 lib/util.h

Comments

Thomas Huth Feb. 12, 2016, 12:40 p.m. UTC | #1 
On 08.02.2016 19:53, Andrew Jones wrote:
> arm-selftest has a couple utility functions that could be useful
> to other unit tests, even other architectures. So move them out.
> split_var moves to lib/util, where we can add other random
> utilities over time. assert_args inspires report_abort, which
> allows us to report a message, using the current prefix, that
> we're aborting (outputs ABORT vs. PASS/FAIL). This is useful
> for cases when unit tests can't complete due to missing
> dependencies of some sort, such as missing/invalid inputs from
> the user.
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  arm/selftest.c               | 45 ++++++++++++++------------------------------
>  config/config-arm-common.mak |  1 +
>  lib/libcflat.h               | 11 ++++++-----
>  lib/report.c                 | 16 ++++++++++++++++
>  lib/util.c                   | 18 ++++++++++++++++++
>  lib/util.h                   | 23 ++++++++++++++++++++++
>  6 files changed, 78 insertions(+), 36 deletions(-)
>  create mode 100644 lib/util.c
>  create mode 100644 lib/util.h
> 
> diff --git a/arm/selftest.c b/arm/selftest.c
> index aad7eecd529ad..8a68f8bf25167 100644
> --- a/arm/selftest.c
> +++ b/arm/selftest.c
> @@ -6,6 +6,7 @@
>   * This work is licensed under the terms of the GNU LGPL, version 2.
>   */
>  #include <libcflat.h>
> +#include <util.h>
>  #include <alloc.h>
>  #include <devicetree.h>
>  #include <asm/setup.h>
> @@ -18,43 +19,21 @@
>  #include <asm/cpumask.h>
>  #include <asm/barrier.h>
>  
> -static void assert_args(int num_args, int needed_args)
> -{
> -	if (num_args < needed_args) {
> -		printf("selftest: not enough arguments\n");
> -		abort();
> -	}
> -}
> -
> -static char *split_var(char *s, long *val)
> -{
> -	char *p;
> -
> -	p = strchr(s, '=');
> -	if (!p)
> -		return NULL;
> -
> -	*val = atol(p+1);
> -	*p = '\0';
> -
> -	return s;
> -}
> -
>  static void check_setup(int argc, char **argv)
>  {
> -	int nr_tests = 0, i;
> -	char *var;
> +	int nr_tests = 0, len, i;
>  	long val;
>  
>  	for (i = 0; i < argc; ++i) {
>  
> -		var = split_var(argv[i], &val);
> -		if (!var)
> +		len = parse_keyval(argv[i], &val);
> +		if (len == -1)
>  			continue;
>  
> -		report_prefix_push(var);
> +		argv[i][len] = '\0';
> +		report_prefix_push(argv[i]);
>  
> -		if (strcmp(var, "mem") == 0) {
> +		if (strcmp(argv[i], "mem") == 0) {
>  
>  			phys_addr_t memsize = PHYS_END - PHYS_OFFSET;
>  			phys_addr_t expected = ((phys_addr_t)val)*1024*1024;
> @@ -63,7 +42,7 @@ static void check_setup(int argc, char **argv)
>  							memsize/1024/1024);
>  			++nr_tests;
>  
> -		} else if (strcmp(var, "smp") == 0) {
> +		} else if (strcmp(argv[i], "smp") == 0) {
>  
>  			report("nr_cpus = %d", nr_cpus == (int)val, nr_cpus);
>  			++nr_tests;
> @@ -72,7 +51,8 @@ static void check_setup(int argc, char **argv)
>  		report_prefix_pop();
>  	}
>  
> -	assert_args(nr_tests, 2);
> +	if (nr_tests < 2)
> +		report_abort("missing input");
>  }
>  
>  static struct pt_regs expected_regs;
> @@ -343,7 +323,10 @@ static void cpu_report(void)
>  int main(int argc, char **argv)
>  {
>  	report_prefix_push("selftest");
> -	assert_args(argc, 1);
> +
> +	if (argc < 1)
> +		report_abort("no test specified");
> +
>  	report_prefix_push(argv[0]);
>  
>  	if (strcmp(argv[0], "setup") == 0) {
> diff --git a/config/config-arm-common.mak b/config/config-arm-common.mak
> index 698555d6a676f..bd153cf6ea5ba 100644
> --- a/config/config-arm-common.mak
> +++ b/config/config-arm-common.mak
> @@ -27,6 +27,7 @@ CFLAGS += -I lib -I lib/libfdt
>  asm-offsets = lib/$(ARCH)/asm-offsets.h
>  include config/asm-offsets.mak
>  
> +cflatobjs += lib/util.o
>  cflatobjs += lib/alloc.o
>  cflatobjs += lib/devicetree.o
>  cflatobjs += lib/virtio.o
> diff --git a/lib/libcflat.h b/lib/libcflat.h
> index 9747ccdbc9f1d..8411f6c5d92e3 100644
> --- a/lib/libcflat.h
> +++ b/lib/libcflat.h
> @@ -57,11 +57,12 @@ extern int snprintf(char *buf, int size, const char *fmt, ...);
>  extern int vsnprintf(char *buf, int size, const char *fmt, va_list va);
>  extern long atol(const char *ptr);
>  
> -void report_prefix_push(const char *prefix);
> -void report_prefix_pop(void);
> -void report(const char *msg_fmt, bool pass, ...);
> -void report_xfail(const char *msg_fmt, bool xfail, bool pass, ...);
> -int report_summary(void);
> +extern void report_prefix_push(const char *prefix);
> +extern void report_prefix_pop(void);
> +extern void report(const char *msg_fmt, bool pass, ...);
> +extern void report_xfail(const char *msg_fmt, bool xfail, bool pass, ...);
> +extern void report_abort(const char *msg_fmt, ...);
> +extern int report_summary(void);
>  
>  #define ARRAY_SIZE(_a) (sizeof(_a)/sizeof((_a)[0]))
>  
> diff --git a/lib/report.c b/lib/report.c
> index 35e664108a921..62916c4ac3c8a 100644
> --- a/lib/report.c
> +++ b/lib/report.c
> @@ -96,3 +96,19 @@ int report_summary(void)
>  
>  	spin_unlock(&lock);
>  }
> +
> +void report_abort(const char *msg_fmt, ...)
> +{
> +	va_list va;
> +	char buf[2000];
> +
> +	puts("ABORT: ");
> +	puts(prefixes);
> +	va_start(va, msg_fmt);
> +	vsnprintf(buf, sizeof(buf), msg_fmt, va);
> +	va_end(va);
> +	puts(buf);
> +	puts("\n");
> +	report_summary();
> +	abort();
> +}
> diff --git a/lib/util.c b/lib/util.c
> new file mode 100644
> index 0000000000000..69b18100c9722
> --- /dev/null
> +++ b/lib/util.c
> @@ -0,0 +1,18 @@
> +/*
> + * Copyright (C) 2016, Red Hat Inc, Andrew Jones <drjones@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.
> + */
> +#include <libcflat.h>
> +
> +int parse_keyval(char *s, long *val)
> +{
> +	char *p;
> +
> +	p = strchr(s, '=');
> +	if (!p)
> +		return -1;
> +
> +	*val = atol(p+1);
> +	return p - s;
> +}
> diff --git a/lib/util.h b/lib/util.h
> new file mode 100644
> index 0000000000000..5b07626175bab
> --- /dev/null
> +++ b/lib/util.h
> @@ -0,0 +1,23 @@
> +#ifndef _UTIL_H_
> +#define _UTIL_H_
> +/*
> + * Collection of utility functions to share between unit tests.
> + *
> + * Copyright (C) 2016, Red Hat Inc, Andrew Jones <drjones@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.
> + */
> +
> +/*
> + * parse_keyval extracts the integer from a string formatted as
> + * string=integer. This is useful for passing expected values in
> + * to the unit test on the command line, i.e. it helps parse QEMU
> + * command lines that include something like -append var1=1 var2=2
> + * @s is the input string, likely a command line parameter, and
> + * @val is a pointer to where the integer will be stored.
> + *
> + * Returns the offset of the '=', or -1 if no keyval pair is found.
> + */
> +extern int parse_keyval(char *s, long *val);
> +
> +#endif

Looks fine to me.

Reviewed-by: Thomas Huth <thuth@redhat.com>

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arm/selftest.c b/arm/selftest.c
index aad7eecd529ad..8a68f8bf25167 100644
--- a/arm/selftest.c
+++ b/arm/selftest.c
@@ -6,6 +6,7 @@ 
  * This work is licensed under the terms of the GNU LGPL, version 2.
  */
 #include <libcflat.h>
+#include <util.h>
 #include <alloc.h>
 #include <devicetree.h>
 #include <asm/setup.h>
@@ -18,43 +19,21 @@ 
 #include <asm/cpumask.h>
 #include <asm/barrier.h>
 
-static void assert_args(int num_args, int needed_args)
-{
-	if (num_args < needed_args) {
-		printf("selftest: not enough arguments\n");
-		abort();
-	}
-}
-
-static char *split_var(char *s, long *val)
-{
-	char *p;
-
-	p = strchr(s, '=');
-	if (!p)
-		return NULL;
-
-	*val = atol(p+1);
-	*p = '\0';
-
-	return s;
-}
-
 static void check_setup(int argc, char **argv)
 {
-	int nr_tests = 0, i;
-	char *var;
+	int nr_tests = 0, len, i;
 	long val;
 
 	for (i = 0; i < argc; ++i) {
 
-		var = split_var(argv[i], &val);
-		if (!var)
+		len = parse_keyval(argv[i], &val);
+		if (len == -1)
 			continue;
 
-		report_prefix_push(var);
+		argv[i][len] = '\0';
+		report_prefix_push(argv[i]);
 
-		if (strcmp(var, "mem") == 0) {
+		if (strcmp(argv[i], "mem") == 0) {
 
 			phys_addr_t memsize = PHYS_END - PHYS_OFFSET;
 			phys_addr_t expected = ((phys_addr_t)val)*1024*1024;
@@ -63,7 +42,7 @@  static void check_setup(int argc, char **argv)
 							memsize/1024/1024);
 			++nr_tests;
 
-		} else if (strcmp(var, "smp") == 0) {
+		} else if (strcmp(argv[i], "smp") == 0) {
 
 			report("nr_cpus = %d", nr_cpus == (int)val, nr_cpus);
 			++nr_tests;
@@ -72,7 +51,8 @@  static void check_setup(int argc, char **argv)
 		report_prefix_pop();
 	}
 
-	assert_args(nr_tests, 2);
+	if (nr_tests < 2)
+		report_abort("missing input");
 }
 
 static struct pt_regs expected_regs;
@@ -343,7 +323,10 @@  static void cpu_report(void)
 int main(int argc, char **argv)
 {
 	report_prefix_push("selftest");
-	assert_args(argc, 1);
+
+	if (argc < 1)
+		report_abort("no test specified");
+
 	report_prefix_push(argv[0]);
 
 	if (strcmp(argv[0], "setup") == 0) {
diff --git a/config/config-arm-common.mak b/config/config-arm-common.mak
index 698555d6a676f..bd153cf6ea5ba 100644
--- a/config/config-arm-common.mak
+++ b/config/config-arm-common.mak
@@ -27,6 +27,7 @@  CFLAGS += -I lib -I lib/libfdt
 asm-offsets = lib/$(ARCH)/asm-offsets.h
 include config/asm-offsets.mak
 
+cflatobjs += lib/util.o
 cflatobjs += lib/alloc.o
 cflatobjs += lib/devicetree.o
 cflatobjs += lib/virtio.o
diff --git a/lib/libcflat.h b/lib/libcflat.h
index 9747ccdbc9f1d..8411f6c5d92e3 100644
--- a/lib/libcflat.h
+++ b/lib/libcflat.h
@@ -57,11 +57,12 @@  extern int snprintf(char *buf, int size, const char *fmt, ...);
 extern int vsnprintf(char *buf, int size, const char *fmt, va_list va);
 extern long atol(const char *ptr);
 
-void report_prefix_push(const char *prefix);
-void report_prefix_pop(void);
-void report(const char *msg_fmt, bool pass, ...);
-void report_xfail(const char *msg_fmt, bool xfail, bool pass, ...);
-int report_summary(void);
+extern void report_prefix_push(const char *prefix);
+extern void report_prefix_pop(void);
+extern void report(const char *msg_fmt, bool pass, ...);
+extern void report_xfail(const char *msg_fmt, bool xfail, bool pass, ...);
+extern void report_abort(const char *msg_fmt, ...);
+extern int report_summary(void);
 
 #define ARRAY_SIZE(_a) (sizeof(_a)/sizeof((_a)[0]))
 
diff --git a/lib/report.c b/lib/report.c
index 35e664108a921..62916c4ac3c8a 100644
--- a/lib/report.c
+++ b/lib/report.c
@@ -96,3 +96,19 @@  int report_summary(void)
 
 	spin_unlock(&lock);
 }
+
+void report_abort(const char *msg_fmt, ...)
+{
+	va_list va;
+	char buf[2000];
+
+	puts("ABORT: ");
+	puts(prefixes);
+	va_start(va, msg_fmt);
+	vsnprintf(buf, sizeof(buf), msg_fmt, va);
+	va_end(va);
+	puts(buf);
+	puts("\n");
+	report_summary();
+	abort();
+}
diff --git a/lib/util.c b/lib/util.c
new file mode 100644
index 0000000000000..69b18100c9722
--- /dev/null
+++ b/lib/util.c
@@ -0,0 +1,18 @@ 
+/*
+ * Copyright (C) 2016, Red Hat Inc, Andrew Jones <drjones@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.
+ */
+#include <libcflat.h>
+
+int parse_keyval(char *s, long *val)
+{
+	char *p;
+
+	p = strchr(s, '=');
+	if (!p)
+		return -1;
+
+	*val = atol(p+1);
+	return p - s;
+}
diff --git a/lib/util.h b/lib/util.h
new file mode 100644
index 0000000000000..5b07626175bab
--- /dev/null
+++ b/lib/util.h
@@ -0,0 +1,23 @@ 
+#ifndef _UTIL_H_
+#define _UTIL_H_
+/*
+ * Collection of utility functions to share between unit tests.
+ *
+ * Copyright (C) 2016, Red Hat Inc, Andrew Jones <drjones@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.
+ */
+
+/*
+ * parse_keyval extracts the integer from a string formatted as
+ * string=integer. This is useful for passing expected values in
+ * to the unit test on the command line, i.e. it helps parse QEMU
+ * command lines that include something like -append var1=1 var2=2
+ * @s is the input string, likely a command line parameter, and
+ * @val is a pointer to where the integer will be stored.
+ *
+ * Returns the offset of the '=', or -1 if no keyval pair is found.
+ */
+extern int parse_keyval(char *s, long *val);
+
+#endif