| Message ID | 20220405075225.15903-2-frankja@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390x: Cleanup and maintenance 4 | expand |
On 05/04/2022 09.52, Janosch Frank wrote: > There's no guarantee that we even find a device at the address we're > testing for if we're not running under QEMU. > > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > --- > s390x/css.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/s390x/css.c b/s390x/css.c > index a333e55a..52d35f49 100644 > --- a/s390x/css.c > +++ b/s390x/css.c > @@ -15,6 +15,7 @@ > #include <interrupt.h> > #include <asm/arch_def.h> > #include <alloc_page.h> > +#include <hardware.h> > > #include <malloc_io.h> > #include <css.h> > @@ -641,6 +642,12 @@ int main(int argc, char *argv[]) > { > int i; > > + /* There's no guarantee where our devices are without qemu */ > + if (detect_host() != HOST_IS_KVM && detect_host() != HOST_IS_TCG) { > + report_skip("Not running under QEMU"); > + goto done; > + } > + > report_prefix_push("Channel Subsystem"); Prefix gets pushed after the if-statement (where you jump to "done" via goto) ... > enable_io_isc(0x80 >> IO_SCH_ISC); > for (i = 0; tests[i].name; i++) { > @@ -648,7 +655,8 @@ int main(int argc, char *argv[]) > tests[i].func(); > report_prefix_pop(); > } > - report_prefix_pop(); > > +done: > + report_prefix_pop(); ... but here it gets popped in the case of "goto done", too. ==> Please either move the if-statement or the "done" label. Thomas > return report_summary(); > }
On Tue, 5 Apr 2022 07:52:18 +0000 Janosch Frank <frankja@linux.ibm.com> wrote: > There's no guarantee that we even find a device at the address we're > testing for if we're not running under QEMU. > > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > --- > s390x/css.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/s390x/css.c b/s390x/css.c > index a333e55a..52d35f49 100644 > --- a/s390x/css.c > +++ b/s390x/css.c > @@ -15,6 +15,7 @@ > #include <interrupt.h> > #include <asm/arch_def.h> > #include <alloc_page.h> > +#include <hardware.h> > > #include <malloc_io.h> > #include <css.h> > @@ -641,6 +642,12 @@ int main(int argc, char *argv[]) > { > int i; > > + /* There's no guarantee where our devices are without qemu */ > + if (detect_host() != HOST_IS_KVM && detect_host() != HOST_IS_TCG) { you could also do !host_is_kvm() && !host_is_tcg() , I think it's more readable, but I do not have strong opinions regarding that > + report_skip("Not running under QEMU"); > + goto done; > + } > + > report_prefix_push("Channel Subsystem"); the prefix push should probably go before the if > enable_io_isc(0x80 >> IO_SCH_ISC); > for (i = 0; tests[i].name; i++) { > @@ -648,7 +655,8 @@ int main(int argc, char *argv[]) > tests[i].func(); > report_prefix_pop(); > } > - report_prefix_pop(); > > +done: > + report_prefix_pop(); > return report_summary(); > }
diff --git a/s390x/css.c b/s390x/css.c index a333e55a..52d35f49 100644 --- a/s390x/css.c +++ b/s390x/css.c @@ -15,6 +15,7 @@ #include <interrupt.h> #include <asm/arch_def.h> #include <alloc_page.h> +#include <hardware.h> #include <malloc_io.h> #include <css.h> @@ -641,6 +642,12 @@ int main(int argc, char *argv[]) { int i; + /* There's no guarantee where our devices are without qemu */ + if (detect_host() != HOST_IS_KVM && detect_host() != HOST_IS_TCG) { + report_skip("Not running under QEMU"); + goto done; + } + report_prefix_push("Channel Subsystem"); enable_io_isc(0x80 >> IO_SCH_ISC); for (i = 0; tests[i].name; i++) { @@ -648,7 +655,8 @@ int main(int argc, char *argv[]) tests[i].func(); report_prefix_pop(); } - report_prefix_pop(); +done: + report_prefix_pop(); return report_summary(); }
There's no guarantee that we even find a device at the address we're testing for if we're not running under QEMU. Signed-off-by: Janosch Frank <frankja@linux.ibm.com> --- s390x/css.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)