| Message ID | 20220508030524.2072035-1-liwang@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] rtc02: loosen the compare precision with few seconds | expand |
On Sun, May 8, 2022 at 11:06 AM Li Wang <liwang@redhat.com> wrote: > + if (!(set_tm->tm_sec == read_tm->tm_sec) > + || !(set_tm->tm_min == read_tm->tm_min) > + || !(set_tm->tm_hour == read_tm->tm_hour)) { > nit: I should use 'A != B' directly but not '!(A==B)' in the expression. That looks strange anyway. > + > + seconds1 = (set_tm->tm_hour * 3600) + (set_tm->tm_min * > 60) + set_tm->tm_sec; > + seconds2 = (read_tm->tm_hour * 3600) + (read_tm->tm_min * > 60) + read_tm->tm_sec; > + > + delta = seconds2 - seconds1; > + > + if (delta < 0 || delta > 3) > Better to print seconds value if fails.
Hi! > static int rtc_tm_cmp(struct rtc_time *set_tm, struct rtc_time *read_tm) > { > - return !((set_tm->tm_sec == read_tm->tm_sec) > - && (set_tm->tm_min == read_tm->tm_min) > - && (set_tm->tm_hour == read_tm->tm_hour) > - && (set_tm->tm_mday == read_tm->tm_mday) > + long delta, seconds1, seconds2; > + > + /* > + * Convert hour/min/sec into seconds to handle the normal > + * and special situations: > + * 1# > + * set_tm: 2022-04-28 13:00:50 > + * read_tm: 2022-04-28 13:00:50 > + * 2# > + * set_tm: 2022-04-28 13:00:50 > + * read_tm: 2022-04-28 13:00:51 > + * 3# > + * set_tm: 2022-04-28 13:00:59 > + * read_tm: 2022-04-28 13:01:00 > + * 4# > + * set_tm: 2022-04-28 13:59:59 > + * read_tm: 2022-04-28 14:00:00 > + * > + * Note: as we have avoided testing around the zero > + * clock, so it's impossible to hit situation 5# > + * set_tm: 2022-04-28 23:59:59 > + * read_tm: 2022-04-29 00:00:00 > + */ > + if (!(set_tm->tm_sec == read_tm->tm_sec) > + || !(set_tm->tm_min == read_tm->tm_min) > + || !(set_tm->tm_hour == read_tm->tm_hour)) { > + > + seconds1 = (set_tm->tm_hour * 3600) + (set_tm->tm_min * 60) + set_tm->tm_sec; > + seconds2 = (read_tm->tm_hour * 3600) + (read_tm->tm_min * 60) + read_tm->tm_sec; > + > + delta = seconds2 - seconds1; > + > + if (delta < 0 || delta > 3) > + return 1; > + } > + > + return !((set_tm->tm_mday == read_tm->tm_mday) > && (set_tm->tm_mon == read_tm->tm_mon) > && (set_tm->tm_year == read_tm->tm_year)); > } I would have done this a bit differently, first chek for day, mon, year then do the calculation as: if (set_tm->tm_year != read_tm->tm_year) return 1; if (set_tm->tm_mon != read_tm->tm_mon) return 1; if (set_tm->tm_mday != read_tm->tm_mday) return 1; seconds1 = .... seconds2 = .... delta = ... if (delta < 0 || delta > 3) return 1; return 0; I find this a bit clearer to read.
Cyril Hrubis <chrubis@suse.cz> wrote: > > I would have done this a bit differently, first chek for day, mon, year > then do the calculation as: > > if (set_tm->tm_year != read_tm->tm_year) > return 1; > > if (set_tm->tm_mon != read_tm->tm_mon) > return 1; > > if (set_tm->tm_mday != read_tm->tm_mday) > return 1; > > I slightly want to keep rtc_time comparison before converting into seconds here, because the time change rarely happens, we don't need to use seconds every time. + if ((set_tm->tm_sec != read_tm->tm_sec) + || (set_tm->tm_min != read_tm->tm_min) + || (set_tm->tm_hour != read_tm->tm_hour)) { > seconds1 = .... > seconds2 = .... > delta = ... > > if (delta < 0 || delta > 3) > return 1; > + } > > return 0; > > > I find this a bit clearer to read. > Fair enough!
diff --git a/testcases/kernel/device-drivers/rtc/rtc02.c b/testcases/kernel/device-drivers/rtc/rtc02.c index 0705357bb..8b03b0a90 100644 --- a/testcases/kernel/device-drivers/rtc/rtc02.c +++ b/testcases/kernel/device-drivers/rtc/rtc02.c @@ -40,10 +40,43 @@ static char *rtctime_to_str(struct rtc_time *tm) static int rtc_tm_cmp(struct rtc_time *set_tm, struct rtc_time *read_tm) { - return !((set_tm->tm_sec == read_tm->tm_sec) - && (set_tm->tm_min == read_tm->tm_min) - && (set_tm->tm_hour == read_tm->tm_hour) - && (set_tm->tm_mday == read_tm->tm_mday) + long delta, seconds1, seconds2; + + /* + * Convert hour/min/sec into seconds to handle the normal + * and special situations: + * 1# + * set_tm: 2022-04-28 13:00:50 + * read_tm: 2022-04-28 13:00:50 + * 2# + * set_tm: 2022-04-28 13:00:50 + * read_tm: 2022-04-28 13:00:51 + * 3# + * set_tm: 2022-04-28 13:00:59 + * read_tm: 2022-04-28 13:01:00 + * 4# + * set_tm: 2022-04-28 13:59:59 + * read_tm: 2022-04-28 14:00:00 + * + * Note: as we have avoided testing around the zero + * clock, so it's impossible to hit situation 5# + * set_tm: 2022-04-28 23:59:59 + * read_tm: 2022-04-29 00:00:00 + */ + if (!(set_tm->tm_sec == read_tm->tm_sec) + || !(set_tm->tm_min == read_tm->tm_min) + || !(set_tm->tm_hour == read_tm->tm_hour)) { + + seconds1 = (set_tm->tm_hour * 3600) + (set_tm->tm_min * 60) + set_tm->tm_sec; + seconds2 = (read_tm->tm_hour * 3600) + (read_tm->tm_min * 60) + read_tm->tm_sec; + + delta = seconds2 - seconds1; + + if (delta < 0 || delta > 3) + return 1; + } + + return !((set_tm->tm_mday == read_tm->tm_mday) && (set_tm->tm_mon == read_tm->tm_mon) && (set_tm->tm_year == read_tm->tm_year)); }
That possibly has time elapse between twice operations, especially on VM we can't guarantee the time precisely equal, let's lose a few seconds to make the test happy: tst_test.c:1433: TINFO: Timeout per run is 0h 10m 00s rtc02.c:66: TINFO: To set RTC date/time is: 2020-10-09 13:23:30 rtc02.c:80: TINFO: read RTC date/time is: 2020-10-09 13:23:31 rtc02.c:83: TFAIL: RTC SET TEST Signed-off-by: Li Wang <liwang@redhat.com> Cc: Eirik Fuller <efuller@redhat.com> Cc: Cyril Hrubis <chrubis@suse.cz> --- testcases/kernel/device-drivers/rtc/rtc02.c | 41 +++++++++++++++++++-- 1 file changed, 37 insertions(+), 4 deletions(-)