Thread (3 messages) 3 messages, 3 authors, 2015-12-17

Re: [PATCH] powerpc: Remove broken GregorianDay()

From: Alexandre Belloni <hidden>
Date: 2015-12-15 09:01:58
Also in: linux-rtc

On 15/12/2015 at 18:09:14 +1100, Daniel Axtens wrote :
GregorianDay() is supposed to calculate the day of the week
(tm->tm_wday) for a given day/month/year. In that calcuation it
indexed into an array called MonthOffset using tm->tm_mon-1. However
tm_mon is zero-based, not one-based, so this is off-by-one. It also
means that every January, GregoiranDay() will access element -1 of
the MonthOffset array.

It also doesn't appear to be a correct algorithm either: see in
contrast kernel/time/timeconv.c's time_to_tm function.

It's been broken forever, which suggests no-one in userland uses
this. It looks like no-one in the kernel uses tm->tm_wday either
(see e.g. drivers/rtc/rtc-ds1305.c:319).

tm->tm_wday is conventionally set to -1 when not available in
hardware so we can simply set it to -1 and drop the function.
(There are over a dozen other drivers in drivers/rtc that do
this.)

Found using UBSAN.

Cc: Andrey Ryabinin <redacted>
Cc: Andrew Morton <akpm@linux-foundation.org> # as an example of what UBSan finds.
Cc: Alessandro Zummo <redacted>
Cc: Alexandre Belloni <redacted>
Cc: rtc-linux@googlegroups.com
Signed-off-by: Daniel Axtens <redacted>
Acked-by: Alexandre Belloni <redacted>
---
This is almost entirely a powerpc patch, but it also touches
the OPAL rtc driver. Alessandro, Alexandre, RTC folk, would
you be OK with mpe taking this through the powerpc tree?
Sure.

I have a plan to actually correct tm_wday before sending it to user
space when it is -1.
I may as well calculate it every time but I'm not sure the extra
calculation is actually worth it because as you mention it, it is not
used by anybody.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help