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

Heinrich Schuchardt xypron.glpk at gmx.de
Wed Oct 4 13:31:26 UTC 2017


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 at chromium.org>
Signed-off-by: Heinrich Schuchardt <xypron.glpk at 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 at 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) = {
-- 
2.14.1



More information about the U-Boot mailing list