* Re: [kvm-unit-tests PATCH] s390x: selftest: Fix report output
2021-05-31 10:50 [kvm-unit-tests PATCH] s390x: selftest: Fix report output Janosch Frank
@ 2021-05-31 10:56 ` Claudio Imbrenda
2021-05-31 11:11 ` Cornelia Huck
2021-05-31 11:15 ` David Hildenbrand
2 siblings, 0 replies; 6+ messages in thread
From: Claudio Imbrenda @ 2021-05-31 10:56 UTC (permalink / raw)
To: Janosch Frank; +Cc: kvm, linux-s390, david, thuth, cohuck
On Mon, 31 May 2021 10:50:03 +0000
Janosch Frank <frankja@linux.ibm.com> wrote:
> To make our TAP parser (and me) happy we don't want to have to reports
> with exactly the same wording.
>
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> ---
> s390x/selftest.c | 18 +++++++++++++-----
> 1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/s390x/selftest.c b/s390x/selftest.c
> index b2fe2e7b..c2ca9896 100644
> --- a/s390x/selftest.c
> +++ b/s390x/selftest.c
> @@ -47,12 +47,19 @@ static void test_malloc(void)
> *tmp2 = 123456789;
> mb();
>
> - report((uintptr_t)tmp & 0xf000000000000000ul, "malloc: got
> vaddr");
> - report(*tmp == 123456789, "malloc: access works");
> + report_prefix_push("malloc");
> + report_prefix_push("ptr_0");
> + report((uintptr_t)tmp & 0xf000000000000000ul, "allocated
> memory");
> + report(*tmp == 123456789, "wrote allocated memory");
> + report_prefix_pop();
> +
> + report_prefix_push("ptr_1");
> report((uintptr_t)tmp2 & 0xf000000000000000ul,
> - "malloc: got 2nd vaddr");
> - report((*tmp2 == 123456789), "malloc: access works");
> - report(tmp != tmp2, "malloc: addresses differ");
> + "allocated memory");
> + report((*tmp2 == 123456789), "wrote allocated memory");
> + report_prefix_pop();
> +
> + report(tmp != tmp2, "allocated memory addresses differ");
>
> expect_pgm_int();
> configure_dat(0);
> @@ -62,6 +69,7 @@ static void test_malloc(void)
>
> free(tmp);
> free(tmp2);
> + report_prefix_pop();
> }
>
> int main(int argc, char**argv)
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [kvm-unit-tests PATCH] s390x: selftest: Fix report output
2021-05-31 10:50 [kvm-unit-tests PATCH] s390x: selftest: Fix report output Janosch Frank
2021-05-31 10:56 ` Claudio Imbrenda
@ 2021-05-31 11:11 ` Cornelia Huck
2021-05-31 12:09 ` Janosch Frank
2021-05-31 11:15 ` David Hildenbrand
2 siblings, 1 reply; 6+ messages in thread
From: Cornelia Huck @ 2021-05-31 11:11 UTC (permalink / raw)
To: Janosch Frank; +Cc: kvm, linux-s390, imbrenda, david, thuth
On Mon, 31 May 2021 10:50:03 +0000
Janosch Frank <frankja@linux.ibm.com> wrote:
> To make our TAP parser (and me) happy we don't want to have to reports
"we want to have two reports" ?
If that's not what has been intended, I'm confused :)
> with exactly the same wording.
>
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
> s390x/selftest.c | 18 +++++++++++++-----
> 1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/s390x/selftest.c b/s390x/selftest.c
> index b2fe2e7b..c2ca9896 100644
> --- a/s390x/selftest.c
> +++ b/s390x/selftest.c
> @@ -47,12 +47,19 @@ static void test_malloc(void)
> *tmp2 = 123456789;
> mb();
>
> - report((uintptr_t)tmp & 0xf000000000000000ul, "malloc: got vaddr");
> - report(*tmp == 123456789, "malloc: access works");
> + report_prefix_push("malloc");
> + report_prefix_push("ptr_0");
> + report((uintptr_t)tmp & 0xf000000000000000ul, "allocated memory");
> + report(*tmp == 123456789, "wrote allocated memory");
> + report_prefix_pop();
> +
> + report_prefix_push("ptr_1");
> report((uintptr_t)tmp2 & 0xf000000000000000ul,
> - "malloc: got 2nd vaddr");
> - report((*tmp2 == 123456789), "malloc: access works");
> - report(tmp != tmp2, "malloc: addresses differ");
> + "allocated memory");
> + report((*tmp2 == 123456789), "wrote allocated memory");
> + report_prefix_pop();
> +
> + report(tmp != tmp2, "allocated memory addresses differ");
>
> expect_pgm_int();
> configure_dat(0);
> @@ -62,6 +69,7 @@ static void test_malloc(void)
>
> free(tmp);
> free(tmp2);
> + report_prefix_pop();
> }
>
> int main(int argc, char**argv)
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [kvm-unit-tests PATCH] s390x: selftest: Fix report output
2021-05-31 11:11 ` Cornelia Huck
@ 2021-05-31 12:09 ` Janosch Frank
0 siblings, 0 replies; 6+ messages in thread
From: Janosch Frank @ 2021-05-31 12:09 UTC (permalink / raw)
To: Cornelia Huck; +Cc: kvm, linux-s390, imbrenda, david, thuth
On 5/31/21 1:11 PM, Cornelia Huck wrote:
> On Mon, 31 May 2021 10:50:03 +0000
> Janosch Frank <frankja@linux.ibm.com> wrote:
>
>> To make our TAP parser (and me) happy we don't want to have to reports
>
> "we want to have two reports" ?
>
> If that's not what has been intended, I'm confused :)
Things that happen if the following sentence is heard:
"Can you fix this quickly, please?"
Will fix, thanks for the review!
>
>> with exactly the same wording.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>> s390x/selftest.c | 18 +++++++++++++-----
>> 1 file changed, 13 insertions(+), 5 deletions(-)
>>
>> diff --git a/s390x/selftest.c b/s390x/selftest.c
>> index b2fe2e7b..c2ca9896 100644
>> --- a/s390x/selftest.c
>> +++ b/s390x/selftest.c
>> @@ -47,12 +47,19 @@ static void test_malloc(void)
>> *tmp2 = 123456789;
>> mb();
>>
>> - report((uintptr_t)tmp & 0xf000000000000000ul, "malloc: got vaddr");
>> - report(*tmp == 123456789, "malloc: access works");
>> + report_prefix_push("malloc");
>> + report_prefix_push("ptr_0");
>> + report((uintptr_t)tmp & 0xf000000000000000ul, "allocated memory");
>> + report(*tmp == 123456789, "wrote allocated memory");
>> + report_prefix_pop();
>> +
>> + report_prefix_push("ptr_1");
>> report((uintptr_t)tmp2 & 0xf000000000000000ul,
>> - "malloc: got 2nd vaddr");
>> - report((*tmp2 == 123456789), "malloc: access works");
>> - report(tmp != tmp2, "malloc: addresses differ");
>> + "allocated memory");
>> + report((*tmp2 == 123456789), "wrote allocated memory");
>> + report_prefix_pop();
>> +
>> + report(tmp != tmp2, "allocated memory addresses differ");
>>
>> expect_pgm_int();
>> configure_dat(0);
>> @@ -62,6 +69,7 @@ static void test_malloc(void)
>>
>> free(tmp);
>> free(tmp2);
>> + report_prefix_pop();
>> }
>>
>> int main(int argc, char**argv)
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [kvm-unit-tests PATCH] s390x: selftest: Fix report output
2021-05-31 10:50 [kvm-unit-tests PATCH] s390x: selftest: Fix report output Janosch Frank
2021-05-31 10:56 ` Claudio Imbrenda
2021-05-31 11:11 ` Cornelia Huck
@ 2021-05-31 11:15 ` David Hildenbrand
2021-05-31 12:07 ` Janosch Frank
2 siblings, 1 reply; 6+ messages in thread
From: David Hildenbrand @ 2021-05-31 11:15 UTC (permalink / raw)
To: Janosch Frank, kvm; +Cc: linux-s390, imbrenda, thuth, cohuck
On 31.05.21 12:50, Janosch Frank wrote:
> To make our TAP parser (and me) happy we don't want to have to reports
> with exactly the same wording.
>
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
> s390x/selftest.c | 18 +++++++++++++-----
> 1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/s390x/selftest.c b/s390x/selftest.c
> index b2fe2e7b..c2ca9896 100644
> --- a/s390x/selftest.c
> +++ b/s390x/selftest.c
> @@ -47,12 +47,19 @@ static void test_malloc(void)
> *tmp2 = 123456789;
> mb();
>
> - report((uintptr_t)tmp & 0xf000000000000000ul, "malloc: got vaddr");
> - report(*tmp == 123456789, "malloc: access works");
> + report_prefix_push("malloc");
> + report_prefix_push("ptr_0");
instead of this "ptr_0" vs. "ptr_1" I'd just use
"allocated 1st page"
"wrote to 1st page"
"allocated 2nd page"
"wrote to 2nd page"
"1st and 2nd page differ"
Avoids one hierarchy of prefix_push ...
> + report((uintptr_t)tmp & 0xf000000000000000ul, "allocated memory");
> + report(*tmp == 123456789, "wrote allocated memory");
> + report_prefix_pop();
> +
> + report_prefix_push("ptr_1");
> report((uintptr_t)tmp2 & 0xf000000000000000ul,
> - "malloc: got 2nd vaddr");
> - report((*tmp2 == 123456789), "malloc: access works");
> - report(tmp != tmp2, "malloc: addresses differ");
> + "allocated memory");
> + report((*tmp2 == 123456789), "wrote allocated memory");
> + report_prefix_pop();
> +
> + report(tmp != tmp2, "allocated memory addresses differ");
>
> expect_pgm_int();
> configure_dat(0);
> @@ -62,6 +69,7 @@ static void test_malloc(void)
>
> free(tmp);
> free(tmp2);
> + report_prefix_pop();
> }
>
> int main(int argc, char**argv)
>
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [kvm-unit-tests PATCH] s390x: selftest: Fix report output
2021-05-31 11:15 ` David Hildenbrand
@ 2021-05-31 12:07 ` Janosch Frank
0 siblings, 0 replies; 6+ messages in thread
From: Janosch Frank @ 2021-05-31 12:07 UTC (permalink / raw)
To: David Hildenbrand, kvm; +Cc: linux-s390, imbrenda, thuth, cohuck
On 5/31/21 1:15 PM, David Hildenbrand wrote:
> On 31.05.21 12:50, Janosch Frank wrote:
>> To make our TAP parser (and me) happy we don't want to have to reports
>> with exactly the same wording.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>> s390x/selftest.c | 18 +++++++++++++-----
>> 1 file changed, 13 insertions(+), 5 deletions(-)
>>
>> diff --git a/s390x/selftest.c b/s390x/selftest.c
>> index b2fe2e7b..c2ca9896 100644
>> --- a/s390x/selftest.c
>> +++ b/s390x/selftest.c
>> @@ -47,12 +47,19 @@ static void test_malloc(void)
>> *tmp2 = 123456789;
>> mb();
>>
>> - report((uintptr_t)tmp & 0xf000000000000000ul, "malloc: got vaddr");
>> - report(*tmp == 123456789, "malloc: access works");
>> + report_prefix_push("malloc");
>> + report_prefix_push("ptr_0");
>
> instead of this "ptr_0" vs. "ptr_1" I'd just use
>
> "allocated 1st page"
> "wrote to 1st page"
> "allocated 2nd page"
> "wrote to 2nd page"
> "1st and 2nd page differ"
>
> Avoids one hierarchy of prefix_push ...
I'd like to keep them since I'll also move the allocation and writes
into a prefix section for the v2 which will provide me with better error
reports if we trigger asserts.
Also from the allocation alone these could be on the same page since we
allocate ints.
>
>> + report((uintptr_t)tmp & 0xf000000000000000ul, "allocated memory");
>> + report(*tmp == 123456789, "wrote allocated memory");
>> + report_prefix_pop();
>> +
>> + report_prefix_push("ptr_1");
>> report((uintptr_t)tmp2 & 0xf000000000000000ul,
>> - "malloc: got 2nd vaddr");
>> - report((*tmp2 == 123456789), "malloc: access works");
>> - report(tmp != tmp2, "malloc: addresses differ");
>> + "allocated memory");
>> + report((*tmp2 == 123456789), "wrote allocated memory");
>> + report_prefix_pop();
>> +
>> + report(tmp != tmp2, "allocated memory addresses differ");
>>
>> expect_pgm_int();
>> configure_dat(0);
>> @@ -62,6 +69,7 @@ static void test_malloc(void)
>>
>> free(tmp);
>> free(tmp2);
>> + report_prefix_pop();
>> }
>>
>> int main(int argc, char**argv)
>>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread