From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755499AbdCTWlQ (ORCPT ); Mon, 20 Mar 2017 18:41:16 -0400 Received: from ns.lineo.co.jp ([203.141.200.203]:45801 "EHLO mail.lineo.co.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753629AbdCTWlO (ORCPT ); Mon, 20 Mar 2017 18:41:14 -0400 X-Greylist: delayed 2003 seconds by postgrey-1.27 at vger.kernel.org; Mon, 20 Mar 2017 18:41:14 EDT Date: Tue, 21 Mar 2017 06:40:06 +0900 From: Tomoyoshi ASANO To: Thomas Gleixner Subject: Re: [Y2038] [RESEND PATCH 2/7] time: Change posix clocks ops interfaces to use timespec64 Cc: Deepa Dinamani , y2038@lists.linaro.org, john.stultz@linaro.org, linux-kernel@vger.kernel.org, arnd@arndb.de In-Reply-To: References: <1489900666-32268-3-git-send-email-deepa.kernel@gmail.com> Message-Id: <20170321064005.D608.22C34B8C@lineo.co.jp> MIME-Version: 1.0 Content-Type: text/plain; charset="ISO-2022-JP" Content-Transfer-Encoding: 7bit X-Mailer: Becky! ver. 2.73 [ja] (Evaluation) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sun, 19 Mar 2017 11:51:34 +0100 (CET) Thomas Gleixner 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 ┗━┛