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@xxxxxxxxxxxx> > --- > Cc: Alexandre Belloni <alexandre.belloni@xxxxxxxxxxx> > Cc: Shuah Khan <shuah@xxxxxxxxxx> > Cc: Masahiro Yamada <masahiroy@xxxxxxxxxx> > Cc: linux-rtc@xxxxxxxxxxxxxxx > Cc: linux-kselftest@xxxxxxxxxxxxxxx > --- > 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 > -- Alexandre Belloni, co-owner and COO, Bootlin Embedded Linux and Kernel engineering https://bootlin.com