Messages in this thread | | | Date | Tue, 21 Mar 2017 06:40:06 +0900 | From | Tomoyoshi ASANO <> | Subject | Re: [Y2038] [RESEND PATCH 2/7] time: Change posix clocks ops interfaces to use timespec64 |
| |
On Sun, 19 Mar 2017 11:51:34 +0100 (CET) Thomas Gleixner <tglx@linutronix.de> wrote:
> On Sat, 18 Mar 2017, Deepa Dinamani wrote: > > > struct timespec is not y2038 safe. > > Replace the posix_clock ops interfaces to use > > struct timespec64. > > The patch also changes struct itimerspec interfaces to > > struct itimerspec64 as itimerspec internally uses timespec > > and itimerspec64 uses timespec64. > > PTP clocks is the only module that sets up these interfaces. > > All individual drivers rely on PTP class driver for exposure > > to userspace. Hence, the change also deals with fixing up these > > PTP interfaces. > > The patch also changes dynamic posix clock implementation to > > reflect the changes in the functional clock interface. > > Please do not explain WHAT the patch is doing. We can see that from the > diff itself. What's important is the WHY. A good changelog is structured in > paragraphs, which explain the context, the problem and the solution. Please > read Documentation/process/submitting-patches.rst. Let me give you an > example. > > struct timespec is not Y2038 safe on 32 bit machines and needs to be > replaced with struct timespec64. > > The posix clock functions use struct timespec directly and through struct > itimerspec. > > Change all function prototypes to use timespec64 and itimerspec64 and fix > up all implementations. > > That gives all the information a reviewer or someone who is looking at the > commit later needs: context, problem scope and solution. > > Hmm? > > > /* posix clock implementation */ > > > > -static int ptp_clock_getres(struct posix_clock *pc, struct timespec *tp) > > +static int ptp_clock_getres(struct posix_clock *pc, struct timespec64 *tp) > > That's a pretty pointless exercise. getres() returns the resolution of the > clock which obviously can never be affected by Y2038. > > > static int pc_clock_settime(clockid_t id, const struct timespec *ts) > > { > > struct posix_clock_desc cd; > > + struct timespec64 ts64 = timespec_to_timespec64(*ts); > > int err; > > Please order the variables as a reverse fir tree sorted by length. > > struct timespec64 ts64 = timespec_to_timespec64(*ts); > struct posix_clock_desc cd; > int err; > > That's way simpler to parse than the above random length odering. > > > @@ -418,14 +427,19 @@ static int pc_timer_settime(struct k_itimer *kit, int flags, > > { > > clockid_t id = kit->it_clock; > > struct posix_clock_desc cd; > > + struct itimerspec64 old64; > > + struct itimerspec64 ts64 = itimerspec_to_itimerspec64(ts); > > See above. > > Thanks, > > tglx > _______________________________________________ > Y2038 mailing list > Y2038@lists.linaro.org > https://lists.linaro.org/mailman/listinfo/y2038
-- ┏━━━━━━━━━━━━━━━━━━━━┓ ┏┫ リネオは お客様の これりな を後押し ┣┓ ┃┗┳━━━━━━━━━━━━━━━━━━┳┛┃ ┗━┛ http://www.lineo.co.jp ┗━┛
| |