* Re: [kvm-unit-tests PATCH] x86: setjmp: check expected value of "i" to give better feedback
2019-09-11 2:31 [kvm-unit-tests PATCH] x86: setjmp: check expected value of "i" to give better feedback Bill Wendling
@ 2019-09-19 20:10 ` Jim Mattson
2019-09-19 20:39 ` Sean Christopherson
2019-09-24 14:03 ` [kvm-unit-tests PATCH] " Paolo Bonzini
2 siblings, 0 replies; 6+ messages in thread
From: Jim Mattson @ 2019-09-19 20:10 UTC (permalink / raw)
To: Bill Wendling; +Cc: kvm list, Sean Christopherson
On Tue, Sep 10, 2019 at 7:32 PM Bill Wendling <morbo@google.com> wrote:
>
> Use a list of expected values instead of printing out numbers, which
> aren't very meaningful. This prints only if the expected and actual
> values differ.
>
> Signed-off-by: Bill Wendling <morbo@google.com>
> ---
> x86/setjmp.c | 19 +++++++++++++++----
> 1 file changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/x86/setjmp.c b/x86/setjmp.c
> index 976a632..c0b25ec 100644
> --- a/x86/setjmp.c
> +++ b/x86/setjmp.c
> @@ -1,19 +1,30 @@
> #include "libcflat.h"
> #include "setjmp.h"
>
> +int expected[] = {
> + 0, 1, 2, 3, 4, 5, 6, 7, 8, 9
> +};
> +
> +#define NUM_EXPECTED (sizeof(expected) / sizeof(int))
> +
> int main(void)
> {
> - volatile int i;
> + volatile int i = -1, index = 0;
> + volatile bool had_errors = false;
> jmp_buf j;
>
> if (setjmp(j) == 0) {
> i = 0;
> }
> - printf("%d\n", i);
> - if (++i < 10) {
> + if (expected[index++] != i) {
> + printf("FAIL: actual %d / expected %d\n", i, expected[index]);
> + had_errors = true;
> + }
> + if (index < NUM_EXPECTED) {
> + i++;
> longjmp(j, 1);
> }
>
> - printf("done\n");
> + printf("Test %s\n", had_errors ? "FAILED" : "PASSED");
> return 0;
> }
> --
> 2.23.0.162.g0b9fbb3734-goog
Acked-by: Jim Mattson <jmattson@google.com>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [kvm-unit-tests PATCH] x86: setjmp: check expected value of "i" to give better feedback
2019-09-11 2:31 [kvm-unit-tests PATCH] x86: setjmp: check expected value of "i" to give better feedback Bill Wendling
2019-09-19 20:10 ` Jim Mattson
@ 2019-09-19 20:39 ` Sean Christopherson
2019-09-19 22:14 ` [kvm-unit-tests PATCH 1/1] " Bill Wendling
2019-09-24 14:03 ` [kvm-unit-tests PATCH] " Paolo Bonzini
2 siblings, 1 reply; 6+ messages in thread
From: Sean Christopherson @ 2019-09-19 20:39 UTC (permalink / raw)
To: Bill Wendling; +Cc: kvm, jmattson
On Tue, Sep 10, 2019 at 07:31:42PM -0700, Bill Wendling wrote:
> Use a list of expected values instead of printing out numbers, which
> aren't very meaningful. This prints only if the expected and actual
> values differ.
>
> Signed-off-by: Bill Wendling <morbo@google.com>
> ---
> x86/setjmp.c | 19 +++++++++++++++----
> 1 file changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/x86/setjmp.c b/x86/setjmp.c
> index 976a632..c0b25ec 100644
> --- a/x86/setjmp.c
> +++ b/x86/setjmp.c
> @@ -1,19 +1,30 @@
> #include "libcflat.h"
> #include "setjmp.h"
>
> +int expected[] = {
This should be 'static const'.
> + 0, 1, 2, 3, 4, 5, 6, 7, 8, 9
> +};
> +
> +#define NUM_EXPECTED (sizeof(expected) / sizeof(int))
How about NUM_LONGJMPS? And you can use ARRAY_SIZE.
> +
> int main(void)
> {
> - volatile int i;
> + volatile int i = -1, index = 0;
> + volatile bool had_errors = false;
> jmp_buf j;
>
> if (setjmp(j) == 0) {
> i = 0;
> }
> - printf("%d\n", i);
> - if (++i < 10) {
> + if (expected[index++] != i) {
> + printf("FAIL: actual %d / expected %d\n", i, expected[index]);
This will print the wrong expected value on failure since index was
incremented above.
> + had_errors = true;
> + }
had_errors seems like overkill. If we fail once, what's the pointing of
continuing on?
> + if (index < NUM_EXPECTED) {
> + i++;
> longjmp(j, 1);
> }
And we should pass i to longjmp(), otherwise there isn't really any point
to having separate index and i variables, e.g.:
volatile int index = 0;
jmp_buf j;
int i;
i = setjmp(j);
if (expected[index] != i) {
printf("FAIL: actual %d / expected %d\n", i, expected[index]);
return -1;
}
index++;
if (i < NUM_LONGJMPS)
longjmp(jmp_buf, i + 1);
return 0;
>
> - printf("done\n");
> + printf("Test %s\n", had_errors ? "FAILED" : "PASSED");
> return 0;
> }
> --
> 2.23.0.162.g0b9fbb3734-goog
>
^ permalink raw reply [flat|nested] 6+ messages in thread* [kvm-unit-tests PATCH 1/1] x86: setjmp: check expected value of "i" to give better feedback
2019-09-19 20:39 ` Sean Christopherson
@ 2019-09-19 22:14 ` Bill Wendling
2019-09-19 22:16 ` Bill Wendling
0 siblings, 1 reply; 6+ messages in thread
From: Bill Wendling @ 2019-09-19 22:14 UTC (permalink / raw)
To: kvm; +Cc: sean.j.christopherson, jmattson, Bill Wendling
Use a list of expected values instead of printing out numbers, which
aren't very meaningful. This prints only if the expected and actual
values differ.
Signed-off-by: Bill Wendling <morbo@google.com>
---
x86/setjmp.c | 23 +++++++++++++++--------
1 file changed, 15 insertions(+), 8 deletions(-)
diff --git a/x86/setjmp.c b/x86/setjmp.c
index 976a632..1874944 100644
--- a/x86/setjmp.c
+++ b/x86/setjmp.c
@@ -1,19 +1,26 @@
#include "libcflat.h"
#include "setjmp.h"
+static const int expected[] = {
+ 0, 1, 2, 3, 4, 5, 6, 7, 8, 9
+};
+
+#define NUM_LONGJMPS ARRAY_SIZE(expected)
+
int main(void)
{
- volatile int i;
+ volatile int index = 0;
jmp_buf j;
+ int i;
- if (setjmp(j) == 0) {
- i = 0;
- }
- printf("%d\n", i);
- if (++i < 10) {
- longjmp(j, 1);
+ i = setjmp(j);
+ if (expected[index] != i) {
+ printf("FAIL: actual %d / expected %d\n", i, expected[index]);
+ return -1;
}
+ index++;
+ if (i + 1 < NUM_LONGJMPS)
+ longjmp(j, i + 1);
- printf("done\n");
return 0;
}
--
2.23.0.351.gc4317032e6-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [kvm-unit-tests PATCH 1/1] x86: setjmp: check expected value of "i" to give better feedback
2019-09-19 22:14 ` [kvm-unit-tests PATCH 1/1] " Bill Wendling
@ 2019-09-19 22:16 ` Bill Wendling
0 siblings, 0 replies; 6+ messages in thread
From: Bill Wendling @ 2019-09-19 22:16 UTC (permalink / raw)
To: kvm list, Paolo Bonzini, Radim Krčmář, Marc Orr
Cc: Sean Christopherson, Jim Mattson
+ Paolo Bonzini, Radim Krčmář, Marc Orr
On Thu, Sep 19, 2019 at 3:15 PM Bill Wendling <morbo@google.com> wrote:
>
> Use a list of expected values instead of printing out numbers, which
> aren't very meaningful. This prints only if the expected and actual
> values differ.
>
> Signed-off-by: Bill Wendling <morbo@google.com>
> ---
> x86/setjmp.c | 23 +++++++++++++++--------
> 1 file changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/x86/setjmp.c b/x86/setjmp.c
> index 976a632..1874944 100644
> --- a/x86/setjmp.c
> +++ b/x86/setjmp.c
> @@ -1,19 +1,26 @@
> #include "libcflat.h"
> #include "setjmp.h"
>
> +static const int expected[] = {
> + 0, 1, 2, 3, 4, 5, 6, 7, 8, 9
> +};
> +
> +#define NUM_LONGJMPS ARRAY_SIZE(expected)
> +
> int main(void)
> {
> - volatile int i;
> + volatile int index = 0;
> jmp_buf j;
> + int i;
>
> - if (setjmp(j) == 0) {
> - i = 0;
> - }
> - printf("%d\n", i);
> - if (++i < 10) {
> - longjmp(j, 1);
> + i = setjmp(j);
> + if (expected[index] != i) {
> + printf("FAIL: actual %d / expected %d\n", i, expected[index]);
> + return -1;
> }
> + index++;
> + if (i + 1 < NUM_LONGJMPS)
> + longjmp(j, i + 1);
>
> - printf("done\n");
> return 0;
> }
> --
> 2.23.0.351.gc4317032e6-goog
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [kvm-unit-tests PATCH] x86: setjmp: check expected value of "i" to give better feedback
2019-09-11 2:31 [kvm-unit-tests PATCH] x86: setjmp: check expected value of "i" to give better feedback Bill Wendling
2019-09-19 20:10 ` Jim Mattson
2019-09-19 20:39 ` Sean Christopherson
@ 2019-09-24 14:03 ` Paolo Bonzini
2 siblings, 0 replies; 6+ messages in thread
From: Paolo Bonzini @ 2019-09-24 14:03 UTC (permalink / raw)
To: Bill Wendling, kvm; +Cc: jmattson, sean.j.christopherson
On 11/09/19 04:31, Bill Wendling wrote:
> Use a list of expected values instead of printing out numbers, which
> aren't very meaningful. This prints only if the expected and actual
> values differ.
>
> Signed-off-by: Bill Wendling <morbo@google.com>
> ---
> x86/setjmp.c | 19 +++++++++++++++----
> 1 file changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/x86/setjmp.c b/x86/setjmp.c
> index 976a632..c0b25ec 100644
> --- a/x86/setjmp.c
> +++ b/x86/setjmp.c
> @@ -1,19 +1,30 @@
> #include "libcflat.h"
> #include "setjmp.h"
>
> +int expected[] = {
> + 0, 1, 2, 3, 4, 5, 6, 7, 8, 9
> +};
> +
> +#define NUM_EXPECTED (sizeof(expected) / sizeof(int))
> +
> int main(void)
> {
> - volatile int i;
> + volatile int i = -1, index = 0;
> + volatile bool had_errors = false;
> jmp_buf j;
>
> if (setjmp(j) == 0) {
> i = 0;
> }
> - printf("%d\n", i);
> - if (++i < 10) {
> + if (expected[index++] != i) {
> + printf("FAIL: actual %d / expected %d\n", i, expected[index]);
> + had_errors = true;
> + }
> + if (index < NUM_EXPECTED) {
> + i++;
> longjmp(j, 1);
> }
>
> - printf("done\n");
> + printf("Test %s\n", had_errors ? "FAILED" : "PASSED");
> return 0;
> }
>
Queued, thanks.
Paolo
^ permalink raw reply [flat|nested] 6+ messages in thread