Hi Jarkko,
On 5/18/2021 12:57 PM, Jarkko Sakkinen wrote:
On Tue, May 18, 2021 at 08:49:00PM +0300, Jarkko Sakkinen wrote:
On Mon, May 17, 2021 at 10:03:42AM -0700, Reinette Chatre wrote:
Hi Jarkko,
On 5/12/2021 2:53 PM, Jarkko Sakkinen wrote:
Migrate to kselftest harness. Use a fixture test with enclave initialized and de-initialized for each of the existing three tests, in other words:
- One FIXTURE() for managing the enclave life-cycle.
- Three TEST_F()'s, one for each test case.
This gives a leaps better reporting than before. Here's an example transcript:
TAP version 13 1..3
ok 1 enclave.unclobbered_vdso
ok 2 enclave.clobbered_vdso
ok 3 enclave.clobbered_vdso_and_user_function
Signed-off-by: Jarkko Sakkinen jarkko@kernel.org
v5:
- Use TH_LOG() for printing enclave address ranges instead of printf(), based on Reinette's remark.
Thank you for considering my feedback. The motivation for my comment was to consider how this test output will be parsed. If these tests will have their output parsed by automated systems then it needs to conform to the TAP13 format as supported by kselftest.
In your latest version the output printed during a successful test has been changed, using TH_LOG() as you noted. From what I can tell this is the only output addressed - failing tests continue to print error messages (perror, fprintf) without consideration of how they will be parsed. My apologies, I am not a kselftest expert to know what the best way for this integration is.
Reinette
It's a valid question, yes.
The problem is that only main.c can use kselftest macros because kselftest_harness.h pulls
static int test_harness_run(int __attribute__((unused)) argc, char __attribute__((unused)) **argv)
which will not end up having a call site (because there's no "TEST_HARNESS_MAIN").
The whole logging thing in kselftest harness is a bit ambiguous. Namely:
- There's a macro TH_LOG() defined in kselftest_harness.h, which "internally" uses fprintf().
- There's an inline function ksft_print_msg() in kselftest.h using vsprintf().
To add to that, kselftest_harness.h internally prints by using ksft_print_msg(), and provides TH_LOG(), which does not use ksft_print_msg().
I don't really get the logic in all this.
I tried to split TH_LOG() as separate entity but it's not possible, as the macros access a static variable called '_metadata'.
I'm not exactly sure how to proceed from this, if we want to make logging consistent.
I would personally suggest to leave the error messages intact in load.c, because there is no way to make them consistent, except by removing them.
It is not clear to me why ksft_print_msg() cannot be used but an alternative to it may be to just prefix all existing diagnostic messages with "# ".
Reinette