| Message ID | 20210531105003.44737-1-frankja@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [kvm-unit-tests] s390x: selftest: Fix report output | expand |
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)
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)
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) >
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) >> > >
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) >
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)
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(-)