Skip to contentSkip to conversation

selftests: rtc: rtctest: Do not open-code TEST_HARNESS_MAIN

PATCH In review Kees Cook 2 participants 3 messages May 18, 2024

Subsystems (2)
lore
Share & tools
lore

Kees Cook

author
#0 lore raw

PATCH 1/1 selftests: rtc: rtctest: Do not open-code TEST_HARNESS_MAIN

Argument processing is specific to the test harness code. Any optional information needs to be passed via environment variables. Move alternate path to the RTC_DEV environment variable. Also do not open-code TEST_HARNESS_MAIN because its definition may change.

Additionally, setup checking can be done in the FIXTURE_SETUP(). With this adjustment, also improve the error reporting when the device cannot be opened.

---
Cc: Alexandre Belloni <[email protected]>
Cc: Shuah Khan <[email protected]>
Cc: Masahiro Yamada <[email protected]>
Cc: [email protected]
Cc: [email protected]

---
 tools/testing/selftests/rtc/Makefile  |  2 +-
 tools/testing/selftests/rtc/rtctest.c | 66 +++++----------------------
 2 files changed, 13 insertions(+), 55 deletions(-)
tools/testing/selftests/rtc/Makefile +1 -1
index 55198ecc04db..654f9d58da3c 100644
@@ -1,5 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0
-CFLAGS += -O3 -Wl,-no-as-needed -Wall
+CFLAGS += -O3 -Wl,-no-as-needed -Wall $(KHDR_INCLUDES)
 LDLIBS += -lrt -lpthread -lm
 
 TEST_GEN_PROGS = rtctest
tools/testing/selftests/rtc/rtctest.c +12 -54

124 lines not on this page · raw message

Signature · 1 line
2.34.1
#1 lore raw
Quoted 4 lines · Kees Cook

Argument processing is specific to the test harness code. Any optional information needs to be passed via environment variables. Move alternate path to the RTC_DEV environment variable. Also do not open-code TEST_HARNESS_MAIN because its definition may change.

Th main issue doing that is that this breaks the main use case of rtctest as /dev/rtc1 is usually the main target for those tests. Having the RTC_DEV environment variable only documented n this commit message is definitively not enough, I'm going to have to handle zillion of complaints that this is not working anymore.

Quoted 169 lines

Additionally, setup checking can be done in the FIXTURE_SETUP(). With this adjustment, also improve the error reporting when the device cannot be opened.

---
Cc: Alexandre Belloni <[email protected]>
Cc: Shuah Khan <[email protected]>
Cc: Masahiro Yamada <[email protected]>
Cc: [email protected]
Cc: [email protected]

---
 tools/testing/selftests/rtc/Makefile  |  2 +-
 tools/testing/selftests/rtc/rtctest.c | 66 +++++----------------------
 2 files changed, 13 insertions(+), 55 deletions(-)
tools/testing/selftests/rtc/Makefile +1 -1
index 55198ecc04db..654f9d58da3c 100644
@@ -1,5 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0
-CFLAGS += -O3 -Wl,-no-as-needed -Wall
+CFLAGS += -O3 -Wl,-no-as-needed -Wall $(KHDR_INCLUDES)
 LDLIBS += -lrt -lpthread -lm
 
 TEST_GEN_PROGS = rtctest
tools/testing/selftests/rtc/rtctest.c +12 -54
index 63ce02d1d5cc..41cfefcc20e1 100644
@@ -30,7 +30,18 @@ FIXTURE(rtc) {
 };
 
 FIXTURE_SETUP(rtc) {
+	char *alternate = getenv("RTC_DEV");
+
+	if (alternate)
+		rtc_file = alternate;
+
 	self->fd = open(rtc_file, O_RDONLY);
+
+	if (self->fd == -1 && errno == ENOENT)
+		SKIP(return, "Skipping test since %s does not exist", rtc_file);
+	EXPECT_NE(-1, self->fd) {
+		TH_LOG("%s: %s\n", rtc_file, strerror(errno));
+	}
 }
 
 FIXTURE_TEARDOWN(rtc) {
@@ -41,10 +52,6 @@ TEST_F(rtc, date_read) {
 	int rc;
 	struct rtc_time rtc_tm;
 
-	if (self->fd == -1 && errno == ENOENT)
-		SKIP(return, "Skipping test since %s does not exist", rtc_file);
-	ASSERT_NE(-1, self->fd);
-
 	/* Read the RTC time/date */
 	rc = ioctl(self->fd, RTC_RD_TIME, &rtc_tm);
 	ASSERT_NE(-1, rc);
@@ -88,10 +95,6 @@ TEST_F_TIMEOUT(rtc, date_read_loop, READ_LOOP_DURATION_SEC + 2) {
 	struct rtc_time rtc_tm;
 	time_t start_rtc_read, prev_rtc_read;
 
-	if (self->fd == -1 && errno == ENOENT)
-		SKIP(return, "Skipping test since %s does not exist", rtc_file);
-	ASSERT_NE(-1, self->fd);
-
 	TH_LOG("Continuously reading RTC time for %ds (with %dms breaks after every read).",
 	       READ_LOOP_DURATION_SEC, READ_LOOP_SLEEP_MS);
 
@@ -126,10 +129,6 @@ TEST_F_TIMEOUT(rtc, uie_read, NUM_UIE + 2) {
 	int i, rc, irq = 0;
 	unsigned long data;
 
-	if (self->fd == -1 && errno == ENOENT)
-		SKIP(return, "Skipping test since %s does not exist", rtc_file);
-	ASSERT_NE(-1, self->fd);
-
 	/* Turn on update interrupts */
 	rc = ioctl(self->fd, RTC_UIE_ON, 0);
 	if (rc == -1) {
@@ -155,10 +154,6 @@ TEST_F(rtc, uie_select) {
 	int i, rc, irq = 0;
 	unsigned long data;
 
-	if (self->fd == -1 && errno == ENOENT)
-		SKIP(return, "Skipping test since %s does not exist", rtc_file);
-	ASSERT_NE(-1, self->fd);
-
 	/* Turn on update interrupts */
 	rc = ioctl(self->fd, RTC_UIE_ON, 0);
 	if (rc == -1) {
@@ -198,10 +193,6 @@ TEST_F(rtc, alarm_alm_set) {
 	time_t secs, new;
 	int rc;
 
-	if (self->fd == -1 && errno == ENOENT)
-		SKIP(return, "Skipping test since %s does not exist", rtc_file);
-	ASSERT_NE(-1, self->fd);
-
 	rc = ioctl(self->fd, RTC_RD_TIME, &tm);
 	ASSERT_NE(-1, rc);
 
@@ -256,10 +247,6 @@ TEST_F(rtc, alarm_wkalm_set) {
 	time_t secs, new;
 	int rc;
 
-	if (self->fd == -1 && errno == ENOENT)
-		SKIP(return, "Skipping test since %s does not exist", rtc_file);
-	ASSERT_NE(-1, self->fd);
-
 	rc = ioctl(self->fd, RTC_RD_TIME, &alarm.time);
 	ASSERT_NE(-1, rc);
 
@@ -308,10 +295,6 @@ TEST_F_TIMEOUT(rtc, alarm_alm_set_minute, 65) {
 	time_t secs, new;
 	int rc;
 
-	if (self->fd == -1 && errno == ENOENT)
-		SKIP(return, "Skipping test since %s does not exist", rtc_file);
-	ASSERT_NE(-1, self->fd);
-
 	rc = ioctl(self->fd, RTC_RD_TIME, &tm);
 	ASSERT_NE(-1, rc);
 
@@ -366,10 +349,6 @@ TEST_F_TIMEOUT(rtc, alarm_wkalm_set_minute, 65) {
 	time_t secs, new;
 	int rc;
 
-	if (self->fd == -1 && errno == ENOENT)
-		SKIP(return, "Skipping test since %s does not exist", rtc_file);
-	ASSERT_NE(-1, self->fd);
-
 	rc = ioctl(self->fd, RTC_RD_TIME, &alarm.time);
 	ASSERT_NE(-1, rc);
 
@@ -410,25 +389,4 @@ TEST_F_TIMEOUT(rtc, alarm_wkalm_set_minute, 65) {
 	ASSERT_EQ(new, secs);
 }
 
-static void __attribute__((constructor))
-__constructor_order_last(void)
-{
-	if (!__constructor_order)
-		__constructor_order = _CONSTRUCTOR_ORDER_BACKWARD;
-}
-
-int main(int argc, char **argv)
-{
-	switch (argc) {
-	case 2:
-		rtc_file = argv[1];
-		/* FALLTHROUGH */
-	case 1:
-		break;
-	default:
-		fprintf(stderr, "usage: %s [rtcdev]\n", argv[0]);
-		return 1;
-	}
-
-	return test_harness_run(argc, argv);
-}
+TEST_HARNESS_MAIN
Signature · 1 line
2.34.1
Signature · 3 lines
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
author
#2 lore raw
Quoted 11 lines · Alexandre Belloni
Quoted 4 lines · Kees Cook

Argument processing is specific to the test harness code. Any optional information needs to be passed via environment variables. Move alternate path to the RTC_DEV environment variable. Also do not open-code TEST_HARNESS_MAIN because its definition may change.

Th main issue doing that is that this breaks the main use case of rtctest as /dev/rtc1 is usually the main target for those tests. Having the RTC_DEV environment variable only documented n this commit message is definitively not enough, I'm going to have to handle zillion of complaints that this is not working anymore.

Hm, maybe switch the default to /dev/rtc1? Maybe there's a better way to integrate arguments into a test runner. Right now the core harness code is doing the argument parsing...

Signature · 1 line
Kees Cook