Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752213AbbFBTUG (ORCPT ); Tue, 2 Jun 2015 15:20:06 -0400 Received: from www.linutronix.de ([62.245.132.108]:52565 "EHLO Galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751921AbbFBTTf (ORCPT ); Tue, 2 Jun 2015 15:19:35 -0400 Date: Tue, 2 Jun 2015 21:19:35 +0200 (CEST) From: Thomas Gleixner To: Baolin Wang cc: arnd@arndb.de, john.stultz@linaro.org, linux-kernel@vger.kernel.org, y2038@lists.linaro.org Subject: Re: [PATCH v4 01/25] linux/time64.h:Introduce the 'struct itimerspec64' for 64bit In-Reply-To: <1433159306-4589-1-git-send-email-baolin.wang@linaro.org> Message-ID: References: <1433159306-4589-1-git-send-email-baolin.wang@linaro.org> User-Agent: Alpine 2.11 (DEB 23 2013-08-11) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-Linutronix-Spam-Score: -1.0 X-Linutronix-Spam-Level: - X-Linutronix-Spam-Status: No , -1.0 points, 5.0 required, ALL_TRUSTED=-1,SHORTCIRCUIT=-0.0001 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1744 Lines: 54 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/