On Wed, Oct 23, 2019 at 5:39 AM Jarkko Sakkinen
<jarkko.sakkinen@xxxxxxxxxxxxxxx> wrote:
>
> On Tue, Oct 22, 2019 at 03:41:58PM -0700, Sean Christopherson wrote:
> > On Fri, Oct 18, 2019 at 01:20:59PM +0300, Jarkko Sakkinen wrote:
> > > On Fri, Oct 18, 2019 at 01:12:52PM +0300, Jarkko Sakkinen wrote:
> > > > On Wed, Oct 16, 2019 at 08:03:26PM -0700, Sean Christopherson wrote:
> > > > > The bulk of this series is comprised of the selftest portion of the vDSO
> > > > > cleanup[*]. The big difference from the full vDSO series is to reuse as
> > > > > much of the existing selftest code as possible. There is still a bit of
> > > > > homebrew code in defining the low level assertion macro, but much less so
> > > > > than in the previous from-scratch version.
> > > > >
> > > > > Cc'd Andy, who also happens to be a reviewer for the harness code, on the
> > > > > off chance he has bandwidth to weigh in.
> > > > >
> > > > > Tagged for_v2? to make it clear that this doesn't need to be rushed into
> > > > > v23.
> > > >
> > > > No need for harness right now. Open coded tests are better for initial
> > > > upstreaming.
> > >
> > > Macros make code more productive to write but harder to read and
> > > debug. With only maybe three tests the cost of using them does
> > > not pay.
> >
> > Harder to read is debatable, personally I find this
> >
> > static void test_sgx_basic(struct sgx_secs *secs)
> > {
> > uint64_t result = 0;
> >
> > sgx_call_eenter((void *)&MAGIC, &result, (void *)secs->base);
> > EXPECT_EQ(result, MAGIC);
> > }
> >
> > to be more intuitive than
> >
> > static void test_sgx_basic(struct sgx_secs *secs)
> > {
> > uint64_t result = 0;
> >
> > printf("Input: 0x%lx\n", MAGIC);
> >
> > sgx_call_eenter((void *)&MAGIC, &result, (void *)secs->base);
> > if (result != MAGIC) {
> > fprintf(stderr, "0x%lx != 0x%lx\n", result, MAGIC);
> > exit(1);
> > }
> >
> > printf("Output: 0x%lx\n", result);
> > }
> >
> > but to each his own.
> >
> > The debug argument I just don't buy. How is this
> >
> > $ ./test_sgx
> > TAP version 13
> > 1..4
> > not ok 1 Expected 'result (0) == MAGIC (1234605616436508552)' at main.c:325
> > ok 2 test_sgx_vdso: Passed
> > ok 3 test_sgx_vdso_exit_handler: Passed
> > ok 4 test_sgx_vdso_exception_handler: Passed
> > # Pass 3 Fail 1 Xfail 0 Xpass 0 Skip 0 Error 0
> >
> >
> > harder to debug than this?
> >
> > $ ./test_sgx
> > Input: 0x1122334455667788
> > 0x0 != 0x1122334455667788
> >
> > Except for the error status in the shell there's not even an explicit
> > indicator that something went wrong, let alone any information about
> > what test was being run, what exact check failed, etc...
>
> Lets consider this post upstreaming. For me it comes in the end
> not to add new twists to the patch set unless there is life and
> death reason to do so.
>
I tend to agree.
For what it's worth, the main reason that I don't use any test harness
in most of the x86 selftests is that I wrote them so early in the
history of kselftests that there wasn't any infrastructure. I haven't
looked to see what the best practice is now.
