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.
Signed-off-by: Kees Cook keescook@chromium.org --- Cc: Alexandre Belloni alexandre.belloni@bootlin.com Cc: Shuah Khan shuah@kernel.org Cc: Masahiro Yamada masahiroy@kernel.org Cc: linux-rtc@vger.kernel.org Cc: linux-kselftest@vger.kernel.org --- tools/testing/selftests/rtc/Makefile | 2 +- tools/testing/selftests/rtc/rtctest.c | 66 +++++---------------------- 2 files changed, 13 insertions(+), 55 deletions(-)
diff --git a/tools/testing/selftests/rtc/Makefile b/tools/testing/selftests/rtc/Makefile index 55198ecc04db..654f9d58da3c 100644 --- a/tools/testing/selftests/rtc/Makefile +++ b/tools/testing/selftests/rtc/Makefile @@ -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 diff --git a/tools/testing/selftests/rtc/rtctest.c b/tools/testing/selftests/rtc/rtctest.c index 63ce02d1d5cc..41cfefcc20e1 100644 --- a/tools/testing/selftests/rtc/rtctest.c +++ b/tools/testing/selftests/rtc/rtctest.c @@ -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
On 17/05/2024 17:16:58-0700, Kees Cook wrote:
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.
Additionally, setup checking can be done in the FIXTURE_SETUP(). With this adjustment, also improve the error reporting when the device cannot be opened.
Signed-off-by: Kees Cook keescook@chromium.org
Cc: Alexandre Belloni alexandre.belloni@bootlin.com Cc: Shuah Khan shuah@kernel.org Cc: Masahiro Yamada masahiroy@kernel.org Cc: linux-rtc@vger.kernel.org Cc: linux-kselftest@vger.kernel.org
tools/testing/selftests/rtc/Makefile | 2 +- tools/testing/selftests/rtc/rtctest.c | 66 +++++---------------------- 2 files changed, 13 insertions(+), 55 deletions(-)
diff --git a/tools/testing/selftests/rtc/Makefile b/tools/testing/selftests/rtc/Makefile index 55198ecc04db..654f9d58da3c 100644 --- a/tools/testing/selftests/rtc/Makefile +++ b/tools/testing/selftests/rtc/Makefile @@ -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 diff --git a/tools/testing/selftests/rtc/rtctest.c b/tools/testing/selftests/rtc/rtctest.c index 63ce02d1d5cc..41cfefcc20e1 100644 --- a/tools/testing/selftests/rtc/rtctest.c +++ b/tools/testing/selftests/rtc/rtctest.c @@ -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
2.34.1
On Sat, May 18, 2024 at 10:23:54PM +0200, Alexandre Belloni wrote:
On 17/05/2024 17:16:58-0700, Kees Cook wrote:
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...
linux-kselftest-mirror@lists.linaro.org