On 3 June 2015 at 03:19, Thomas Gleixner <[email protected]> wrote: > On Mon, 1 Jun 2015, Baolin Wang wrote: > > > Subject: linux/time64.h:Introduce the 'struct itimerspec64' for 64bit > > This subject line is wrong in several aspects: > > - linux/time64.h is a file name and does not describe the subsystem > you are changing. 'time:' is the proper choice here > > - Missing space between colon and first word of the sentence > > - What means struct itimerspec64 for 64bit? Is this a 64bit only > variant or what? > > > "Subject: time: Introduce struct itimerspec64" > > is the proper subject line as it names the subsystem (time) and tells > clearly what the patch does. > > > This patch introduces the 'struct itimerspec64' for 64bit to replace > itimerspec, > > Again: 'for 64bit' is really wrong here. > > > and also introduces the conversion methods: itimerspec64_to_itimerspec() > and > > itimerspec_to_itimerspec64(), that makes itimerspec ready for 2038 year. > > You explain in great length WHAT the patch is doing, which is > pointless because one can see that from the patch itself. You should > explain WHY you are doing this first. > > "itimerspec is not year 2038 safe on 32bit systems due to the > limitation of the struct timespec members. Introduce itimerspec64 > which uses struct timespec64 instead and provide conversion > functions" > > Hmm? > > > > +struct itimerspec64 { > > + struct timespec64 it_interval; /* timer period */ > > + struct timespec64 it_value; /* timer expiration */ > > I really hate tail comments. If you want to document your structure > use proper KernelDoc for it. > > Thanks, > > tglx >
Hi Thomas, Thanks for your reviewing in detail, i'll fix these comments in next patch. -- Baolin.wang Best Regards _______________________________________________ Y2038 mailing list [email protected] https://lists.linaro.org/mailman/listinfo/y2038
