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.
/Jarkko
