On Sun, 19 Mar 2017 11:51:34 +0100 (CET)
Thomas Gleixner <t...@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
> y2...@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/y2038
--
┏━━━━━━━━━━━━━━━━━━━━┓
┏┫ リネオは お客様の これりな を後押し ┣┓
┃┗┳━━━━━━━━━━━━━━━━━━┳┛┃
┗━┛ http://www.lineo.co.jp ┗━┛
