On Wed, Feb 17, 2016 at 11:14:36AM +1100, David Gibson wrote:
> 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?
Only because we haven't implemented a vprintf yet :-) Currently
vsnprintf is the only va_list taking printf we have. This pattern
has occurred enough times though that adding vprintf is a pretty
good idea.
>
> Apart from that,
>
> Reviewed-by: David Gibson <david@xxxxxxxxxxxxxxxxxxxxx>
Thanks,
drew
>
>
> > + 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
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
