[U-Boot] [PATCH v2 1/1] efi_selftest: make tests easier to read

Rename counter to more illustrative names. Update notification function description. Simplify notification function. Add comment for arbitrary non-zero value. Document @return. Use constants for return values of setup, execute, teardown.

Reported-by: Simon Glass sjg@chromium.org Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v2: Remove superfluous blank in a message text. --- include/efi_selftest.h | 3 + lib/efi_selftest/efi_selftest.c | 15 +++-- lib/efi_selftest/efi_selftest_events.c | 80 ++++++++++++---------- lib/efi_selftest/efi_selftest_exitbootservices.c | 46 +++++++------ lib/efi_selftest/efi_selftest_tpl.c | 85 +++++++++++++----------- 5 files changed, 129 insertions(+), 100 deletions(-)

diff --git a/include/efi_selftest.h b/include/efi_selftest.h index 76304a2b2a..beb662d4e1 100644 --- a/include/efi_selftest.h +++ b/include/efi_selftest.h @@ -14,6 +14,9 @@ #include <efi_api.h> #include <linker_lists.h>

+#define EFI_ST_SUCCESS 0 +#define EFI_ST_FAILURE 1 + /* * Prints an error message. * diff --git a/lib/efi_selftest/efi_selftest.c b/lib/efi_selftest/efi_selftest.c index ff00254c21..45d8d3d384 100644 --- a/lib/efi_selftest/efi_selftest.c +++ b/lib/efi_selftest/efi_selftest.c @@ -66,16 +66,17 @@ void efi_st_exit_boot_services(void) * * @test the test to be executed * @failures counter that will be incremented if a failure occurs + * @return EFI_ST_SUCCESS for success */ static int setup(struct efi_unit_test *test, unsigned int *failures) { int ret;

if (!test->setup) - return 0; + return EFI_ST_SUCCESS; efi_st_printf("\nSetting up '%s'\n", test->name); ret = test->setup(handle, systable); - if (ret) { + if (ret != EFI_ST_SUCCESS) { efi_st_error("Setting up '%s' failed\n", test->name); ++*failures; } else { @@ -89,16 +90,17 @@ static int setup(struct efi_unit_test *test, unsigned int *failures) * * @test the test to be executed * @failures counter that will be incremented if a failure occurs + * @return EFI_ST_SUCCESS for success */ static int execute(struct efi_unit_test *test, unsigned int *failures) { int ret;

if (!test->execute) - return 0; + return EFI_ST_SUCCESS; efi_st_printf("\nExecuting '%s'\n", test->name); ret = test->execute(); - if (ret) { + if (ret != EFI_ST_SUCCESS) { efi_st_error("Executing '%s' failed\n", test->name); ++*failures; } else { @@ -112,16 +114,17 @@ static int execute(struct efi_unit_test *test, unsigned int *failures) * * @test the test to be torn down * @failures counter that will be incremented if a failure occurs + * @return EFI_ST_SUCCESS for success */ static int teardown(struct efi_unit_test *test, unsigned int *failures) { int ret;

if (!test->teardown) - return 0; + return EFI_ST_SUCCESS; efi_st_printf("\nTearing down '%s'\n", test->name); ret = test->teardown(); - if (ret) { + if (ret != EFI_ST_SUCCESS) { efi_st_error("Tearing down '%s' failed\n", test->name); ++*failures; } else { diff --git a/lib/efi_selftest/efi_selftest_events.c b/lib/efi_selftest/efi_selftest_events.c index c4f66952b9..7766eedc38 100644 --- a/lib/efi_selftest/efi_selftest_events.c +++ b/lib/efi_selftest/efi_selftest_events.c @@ -14,20 +14,22 @@

static struct efi_event *event_notify; static struct efi_event *event_wait; -static unsigned int counter; +static unsigned int timer_ticks; static struct efi_boot_services *boottime;

/* - * Notification function, increments a counter. + * Notification function, increments the notfication count if parameter + * context is provided. * * @event notified event - * @context pointer to the counter + * @context pointer to the notification count */ static void EFIAPI notify(struct efi_event *event, void *context) { - if (!context) - return; - ++*(unsigned int *)context; + unsigned int *count = context; + + if (count) + ++*count; }

/* @@ -38,6 +40,7 @@ static void EFIAPI notify(struct efi_event *event, void *context) * * @handle: handle of the loaded image * @systable: system table + * @return: EFI_ST_SUCCESS for success */ static int setup(const efi_handle_t handle, const struct efi_system_table *systable) @@ -47,25 +50,27 @@ static int setup(const efi_handle_t handle, boottime = systable->boottime;

ret = boottime->create_event(EVT_TIMER | EVT_NOTIFY_SIGNAL, - TPL_CALLBACK, notify, (void *)&counter, + TPL_CALLBACK, notify, (void *)&timer_ticks, &event_notify); if (ret != EFI_SUCCESS) { efi_st_error("could not create event\n"); - return 1; + return EFI_ST_FAILURE; } ret = boottime->create_event(EVT_TIMER | EVT_NOTIFY_WAIT, TPL_CALLBACK, notify, NULL, &event_wait); if (ret != EFI_SUCCESS) { efi_st_error("could not create event\n"); - return 1; + return EFI_ST_FAILURE; } - return 0; + return EFI_ST_SUCCESS; }

/* * Tear down unit test. * * Close the events created in setup. + * + * @return: EFI_ST_SUCCESS for success */ static int teardown(void) { @@ -76,7 +81,7 @@ static int teardown(void) event_notify = NULL; if (ret != EFI_SUCCESS) { efi_st_error("could not close event\n"); - return 1; + return EFI_ST_FAILURE; } } if (event_wait) { @@ -84,10 +89,10 @@ static int teardown(void) event_wait = NULL; if (ret != EFI_SUCCESS) { efi_st_error("could not close event\n"); - return 1; + return EFI_ST_FAILURE; } } - return 0; + return EFI_ST_SUCCESS; }

/* @@ -98,6 +103,8 @@ static int teardown(void) * * Run a 100 ms single shot timer and check that it is called once * while waiting for 100 ms periodic timer for two periods. + * + * @return: EFI_ST_SUCCESS for success */ static int execute(void) { @@ -105,85 +112,86 @@ static int execute(void) efi_status_t ret;

/* Set 10 ms timer */ - counter = 0; + timer_ticks = 0; ret = boottime->set_timer(event_notify, EFI_TIMER_PERIODIC, 100000); if (ret != EFI_SUCCESS) { efi_st_error("Could not set timer\n"); - return 1; + return EFI_ST_FAILURE; } /* Set 100 ms timer */ ret = boottime->set_timer(event_wait, EFI_TIMER_RELATIVE, 1000000); if (ret != EFI_SUCCESS) { efi_st_error("Could not set timer\n"); - return 1; + return EFI_ST_FAILURE; }

+ /* Set some arbitrary non-zero value to make change detectable. */ index = 5; ret = boottime->wait_for_event(1, &event_wait, &index); if (ret != EFI_SUCCESS) { efi_st_error("Could not wait for event\n"); - return 1; + return EFI_ST_FAILURE; } ret = boottime->check_event(event_wait); if (ret != EFI_NOT_READY) { efi_st_error("Signaled state was not cleared.\n"); efi_st_printf("ret = %u\n", (unsigned int)ret); - return 1; + return EFI_ST_FAILURE; } if (index != 0) { efi_st_error("WaitForEvent returned wrong index\n"); - return 1; + return EFI_ST_FAILURE; } - efi_st_printf("Counter periodic: %u\n", counter); - if (counter < 8 || counter > 12) { + efi_st_printf("Notification count periodic: %u\n", timer_ticks); + if (timer_ticks < 8 || timer_ticks > 12) { efi_st_error("Incorrect timing of events\n"); - return 1; + return EFI_ST_FAILURE; } ret = boottime->set_timer(event_notify, EFI_TIMER_STOP, 0); if (index != 0) { efi_st_error("Could not cancel timer\n"); - return 1; + return EFI_ST_FAILURE; } /* Set 10 ms timer */ - counter = 0; + timer_ticks = 0; ret = boottime->set_timer(event_notify, EFI_TIMER_RELATIVE, 100000); if (index != 0) { efi_st_error("Could not set timer\n"); - return 1; + return EFI_ST_FAILURE; } /* Set 100 ms timer */ ret = boottime->set_timer(event_wait, EFI_TIMER_PERIODIC, 1000000); if (index != 0) { efi_st_error("Could not set timer\n"); - return 1; + return EFI_ST_FAILURE; } ret = boottime->wait_for_event(1, &event_wait, &index); if (ret != EFI_SUCCESS) { efi_st_error("Could not wait for event\n"); - return 1; + return EFI_ST_FAILURE; } - efi_st_printf("Counter single shot: %u\n", counter); - if (counter != 1) { + efi_st_printf("Notification count single shot: %u\n", timer_ticks); + if (timer_ticks != 1) { efi_st_error("Single shot timer failed\n"); - return 1; + return EFI_ST_FAILURE; } ret = boottime->wait_for_event(1, &event_wait, &index); if (ret != EFI_SUCCESS) { efi_st_error("Could not wait for event\n"); - return 1; + return EFI_ST_FAILURE; } - efi_st_printf("Stopped counter: %u\n", counter); - if (counter != 1) { + efi_st_printf("Notification count stopped timer: %u\n", timer_ticks); + if (timer_ticks != 1) { efi_st_error("Stopped timer fired\n"); - return 1; + return EFI_ST_FAILURE; } ret = boottime->set_timer(event_wait, EFI_TIMER_STOP, 0); if (index != 0) { efi_st_error("Could not cancel timer\n"); - return 1; + return EFI_ST_FAILURE; }

- return 0; + return EFI_ST_SUCCESS; }

EFI_UNIT_TEST(events) = { diff --git a/lib/efi_selftest/efi_selftest_exitbootservices.c b/lib/efi_selftest/efi_selftest_exitbootservices.c index 60271e6180..cddd11d52b 100644 --- a/lib/efi_selftest/efi_selftest_exitbootservices.c +++ b/lib/efi_selftest/efi_selftest_exitbootservices.c @@ -1,5 +1,5 @@ /* - * efi_selftest_events + * efi_selftest_exitbootservices * * Copyright (c) 2017 Heinrich Schuchardt xypron.glpk@gmx.de * @@ -13,19 +13,19 @@

static struct efi_boot_services *boottime; static struct efi_event *event_notify; -static unsigned int counter; +static unsigned int notification_count;

/* - * Notification function, increments a counter. + * Notification function, increments the notification count. * * @event notified event - * @context pointer to the counter + * @context pointer to the notification count */ static void EFIAPI notify(struct efi_event *event, void *context) { - if (!context) - return; - ++*(unsigned int *)context; + unsigned int *count = context; + + ++*count; }

/* @@ -35,6 +35,7 @@ static void EFIAPI notify(struct efi_event *event, void *context) * * @handle: handle of the loaded image * @systable: system table + * @return: EFI_ST_SUCCESS for success */ static int setup(const efi_handle_t handle, const struct efi_system_table *systable) @@ -43,21 +44,24 @@ static int setup(const efi_handle_t handle,

boottime = systable->boottime;

- counter = 0; + notification_count = 0; ret = boottime->create_event(EVT_SIGNAL_EXIT_BOOT_SERVICES, - TPL_CALLBACK, notify, (void *)&counter, + TPL_CALLBACK, notify, + (void *)&notification_count, &event_notify); if (ret != EFI_SUCCESS) { efi_st_error("could not create event\n"); - return 1; + return EFI_ST_FAILURE; } - return 0; + return EFI_ST_SUCCESS; }

/* * Tear down unit test. * * Close the event created in setup. + * + * @return: EFI_ST_SUCCESS for success */ static int teardown(void) { @@ -68,10 +72,10 @@ static int teardown(void) event_notify = NULL; if (ret != EFI_SUCCESS) { efi_st_error("could not close event\n"); - return 1; + return EFI_ST_FAILURE; } } - return 0; + return EFI_ST_SUCCESS; }

/* @@ -82,19 +86,21 @@ static int teardown(void) * * Call ExitBootServices again and check that the notification function is * not called again. + * + * @return: EFI_ST_SUCCESS for success */ static int execute(void) { - if (counter != 1) { - efi_st_error("ExitBootServices was not notified"); - return 1; + if (notification_count != 1) { + efi_st_error("ExitBootServices was not notified\n"); + return EFI_ST_FAILURE; } efi_st_exit_boot_services(); - if (counter != 1) { - efi_st_error("ExitBootServices was notified twice"); - return 1; + if (notification_count != 1) { + efi_st_error("ExitBootServices was notified twice\n"); + return EFI_ST_FAILURE; } - return 0; + return EFI_ST_SUCCESS; }

EFI_UNIT_TEST(exitbootservices) = { diff --git a/lib/efi_selftest/efi_selftest_tpl.c b/lib/efi_selftest/efi_selftest_tpl.c index 90ace0f51e..5d13f3b52d 100644 --- a/lib/efi_selftest/efi_selftest_tpl.c +++ b/lib/efi_selftest/efi_selftest_tpl.c @@ -13,20 +13,20 @@

static struct efi_event *event_notify; static struct efi_event *event_wait; -static unsigned int counter; +static unsigned int notification_count; static struct efi_boot_services *boottime;

/* - * Notification function, increments a counter. + * Notification function, increments the notification count. * * @event notified event - * @context pointer to the counter + * @context pointer to the notification count */ static void EFIAPI notify(struct efi_event *event, void *context) { - if (!context) - return; - ++*(unsigned int *)context; + unsigned int *count = context; + + ++*count; }

/* @@ -37,6 +37,7 @@ static void EFIAPI notify(struct efi_event *event, void *context) * * @handle: handle of the loaded image * @systable: system table + * @return: EFI_ST_SUCCESS for success */ static int setup(const efi_handle_t handle, const struct efi_system_table *systable) @@ -46,25 +47,28 @@ static int setup(const efi_handle_t handle, boottime = systable->boottime;

ret = boottime->create_event(EVT_TIMER | EVT_NOTIFY_SIGNAL, - TPL_CALLBACK, notify, (void *)&counter, + TPL_CALLBACK, notify, + (void *)&notification_count, &event_notify); if (ret != EFI_SUCCESS) { efi_st_error("could not create event\n"); - return 1; + return EFI_ST_FAILURE; } ret = boottime->create_event(EVT_TIMER | EVT_NOTIFY_WAIT, TPL_HIGH_LEVEL, notify, NULL, &event_wait); if (ret != EFI_SUCCESS) { efi_st_error("could not create event\n"); - return 1; + return EFI_ST_FAILURE; } - return 0; + return EFI_ST_SUCCESS; }

/* * Tear down unit test. * * Close the events created in setup. + * + * @return: EFI_ST_SUCCESS for success */ static int teardown(void) { @@ -75,7 +79,7 @@ static int teardown(void) event_notify = NULL; if (ret != EFI_SUCCESS) { efi_st_error("could not close event\n"); - return 1; + return EFI_ST_FAILURE; } } if (event_wait) { @@ -83,11 +87,11 @@ static int teardown(void) event_wait = NULL; if (ret != EFI_SUCCESS) { efi_st_error("could not close event\n"); - return 1; + return EFI_ST_FAILURE; } } boottime->restore_tpl(TPL_APPLICATION); - return 0; + return EFI_ST_SUCCESS; }

/* @@ -101,6 +105,8 @@ static int teardown(void) * * Lower the TPL level and check that the queued notification * function is called. + * + * @return: EFI_ST_SUCCESS for success */ static int execute(void) { @@ -109,100 +115,103 @@ static int execute(void) UINTN old_tpl;

/* Set 10 ms timer */ - counter = 0; + notification_count = 0; ret = boottime->set_timer(event_notify, EFI_TIMER_PERIODIC, 100000); if (ret != EFI_SUCCESS) { efi_st_error("Could not set timer\n"); - return 1; + return EFI_ST_FAILURE; } /* Set 100 ms timer */ ret = boottime->set_timer(event_wait, EFI_TIMER_RELATIVE, 1000000); if (ret != EFI_SUCCESS) { efi_st_error("Could not set timer\n"); - return 1; + return EFI_ST_FAILURE; } index = 5; ret = boottime->wait_for_event(1, &event_wait, &index); if (ret != EFI_SUCCESS) { efi_st_error("Could not wait for event\n"); - return 1; + return EFI_ST_FAILURE; } ret = boottime->check_event(event_wait); if (ret != EFI_NOT_READY) { efi_st_error("Signaled state was not cleared.\n"); efi_st_printf("ret = %u\n", (unsigned int)ret); - return 1; + return EFI_ST_FAILURE; } if (index != 0) { efi_st_error("WaitForEvent returned wrong index\n"); - return 1; + return EFI_ST_FAILURE; } - efi_st_printf("Counter with TPL level TPL_APPLICATION: %u\n", counter); - if (counter < 8 || counter > 12) { + efi_st_printf("Notification count with TPL level TPL_APPLICATION: %u\n", + notification_count); + if (notification_count < 8 || notification_count > 12) { efi_st_error("Incorrect timing of events\n"); - return 1; + return EFI_ST_FAILURE; } ret = boottime->set_timer(event_notify, EFI_TIMER_STOP, 0); if (index != 0) { efi_st_error("Could not cancel timer\n"); - return 1; + return EFI_ST_FAILURE; } /* Raise TPL level */ old_tpl = boottime->raise_tpl(TPL_CALLBACK); if (old_tpl != TPL_APPLICATION) { efi_st_error("Initial TPL level was not TPL_APPLICATION"); - return 1; + return EFI_ST_FAILURE; } /* Set 10 ms timer */ - counter = 0; + notification_count = 0; ret = boottime->set_timer(event_notify, EFI_TIMER_PERIODIC, 100000); if (index != 0) { efi_st_error("Could not set timer\n"); - return 1; + return EFI_ST_FAILURE; } /* Set 100 ms timer */ ret = boottime->set_timer(event_wait, EFI_TIMER_RELATIVE, 1000000); if (ret != EFI_SUCCESS) { efi_st_error("Could not set timer\n"); - return 1; + return EFI_ST_FAILURE; } do { ret = boottime->check_event(event_wait); } while (ret == EFI_NOT_READY); if (ret != EFI_SUCCESS) { efi_st_error("Could not check event\n"); - return 1; + return EFI_ST_FAILURE; } - efi_st_printf("Counter with TPL level TPL_CALLBACK: %u\n", counter); - if (counter != 0) { + efi_st_printf("Notification count with TPL level TPL_CALLBACK: %u\n", + notification_count); + if (notification_count != 0) { efi_st_error("Suppressed timer fired\n"); - return 1; + return EFI_ST_FAILURE; } /* Set 1 ms timer */ ret = boottime->set_timer(event_wait, EFI_TIMER_RELATIVE, 1000); if (ret != EFI_SUCCESS) { efi_st_error("Could not set timer\n"); - return 1; + return EFI_ST_FAILURE; } /* Restore the old TPL level */ boottime->restore_tpl(TPL_APPLICATION); ret = boottime->wait_for_event(1, &event_wait, &index); if (ret != EFI_SUCCESS) { efi_st_error("Could not wait for event\n"); - return 1; + return EFI_ST_FAILURE; } - efi_st_printf("Counter with TPL level TPL_APPLICATION: %u\n", counter); - if (counter < 1) { + efi_st_printf("Notification count with TPL level TPL_APPLICATION: %u\n", + notification_count); + if (notification_count < 1) { efi_st_error("Queued timer event did not fire\n"); - return 1; + return EFI_ST_FAILURE; } ret = boottime->set_timer(event_wait, EFI_TIMER_STOP, 0); if (index != 0) { efi_st_error("Could not cancel timer\n"); - return 1; + return EFI_ST_FAILURE; }

- return 0; + return EFI_ST_SUCCESS; }

EFI_UNIT_TEST(tpl) = {