[kvm-unit-tests] x86: setjmp: check expected value of "i" to give better feedback
diff mbox series

Message ID 20190911023142.85970-1-morbo@google.com
State New
Headers show
Series
  • [kvm-unit-tests] x86: setjmp: check expected value of "i" to give better feedback
Related show

Commit Message

Bill Wendling Sept. 11, 2019, 2:31 a.m. UTC
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(-)

Comments

Jim Mattson Sept. 19, 2019, 8:10 p.m. UTC | #1
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>
Sean Christopherson Sept. 19, 2019, 8:39 p.m. UTC | #2
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
>
Bill Wendling Sept. 19, 2019, 10:16 p.m. UTC | #3
+ 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
>
Paolo Bonzini Sept. 24, 2019, 2:03 p.m. UTC | #4
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

Patch
diff mbox series

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;
 }