On Mon, Feb 15, 2016 at 02:49:15PM +0100, 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@xxxxxxxxxx>
> Reviewed-by: Thomas Huth <thuth@xxxxxxxxxx>
> Tested-by: Laurent Vivier <lvivier@xxxxxxxxxx>
> ---
> 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..75dc91faab69a 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)
> + 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);
Is there a reason for using vsnprintf() rather than just vprintf()
here?
Apart from that,
Reviewed-by: David Gibson <david@xxxxxxxxxxxxxxxxxxxxx>
> + 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@xxxxxxxxxx>
> + *
> + * 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..4c4b441322770
> --- /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@xxxxxxxxxx>
> + *
> + * 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 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
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
Attachment:
signature.asc
Description: PGP signature
