From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id D0C011B209 for ; Mon, 2 Oct 2017 15:46:29 +0200 (CEST) Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 02 Oct 2017 06:46:28 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.42,469,1500966000"; d="scan'208";a="155674153" Received: from bricha3-mobl3.ger.corp.intel.com ([10.237.221.37]) by orsmga005.jf.intel.com with SMTP; 02 Oct 2017 06:46:25 -0700 Received: by (sSMTP sendmail emulation); Mon, 02 Oct 2017 14:46:24 +0100 Date: Mon, 2 Oct 2017 14:46:24 +0100 From: Bruce Richardson To: Adrien Mazarguil Cc: Stephen Hemminger , dev@dpdk.org Message-ID: <20171002134624.GA10500@bricha3-MOBL3.ger.corp.intel.com> References: <20170929153749.9806-1-stephen@networkplumber.org> <20171002115317.GJ3871@6wind.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20171002115317.GJ3871@6wind.com> Organization: Intel Research and Development Ireland Ltd. User-Agent: Mutt/1.8.3 (2017-05-23) Subject: Re: [dpdk-dev] [PATCH] checkpatch: re-enable warnings about split long strings X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 02 Oct 2017 13:46:30 -0000 On Mon, Oct 02, 2017 at 01:53:17PM +0200, Adrien Mazarguil wrote: > Hi Stephen, > > On Fri, Sep 29, 2017 at 08:37:49AM -0700, Stephen Hemminger wrote: > > The Linux kernel style policy about strings is that strings should > > be always put on one line. This makes sense since a typical use case > > is for a user to type the error message into a search engine or > > grep, and it won't be found if split across lines. This patch just > > re-enables that check. > > > > Yes, lots of DPDK code now splits strings, that doesn't make it > > right. > > > > Signed-off-by: Stephen Hemminger --- > > devtools/checkpatches.sh | 1 - 1 file changed, 1 deletion(-) > > > > diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh > > index a56c41a301c0..3e6081dd673e 100755 --- > > a/devtools/checkpatches.sh +++ b/devtools/checkpatches.sh @@ -44,7 > > +44,6 @@ options="$options --show-types" options="$options > > --ignore=LINUX_VERSION_CODE,FILE_PATH_CHANGES,\ > > VOLATILE,PREFER_PACKED,PREFER_ALIGNED,PREFER_PRINTF,\ > > PREFER_KERNEL_TYPES,BIT_MACRO,CONST_STRUCT,\ > > -SPLIT_STRING,LONG_LINE_STRING,\ > > LINE_SPACING,PARENTHESIS_ALIGNMENT,NETWORKING_BLOCK_COMMENT_STYLE,\ > > NEW_TYPEDEFS,COMPARISON_TO_NULL" > > I'm not sure, given that the main reason for splitting strings in the > first place is to avoid LONG_LINE_STRING warnings, I think we must > choose between the two options. If split strings are not allowed, then > long lines must be. > > Since checkpatches.sh is used by various automated scripts to complain > loudly about problems in submissions, the above change prevents > maintainers from writing long string at all (can't split and can't go > past 80 columns). > > As a result, they will be tempted to cripple their code with nasty > workarounds to shut up checkpatches.sh, we don't want that to happen. > > Also I think the reasons stated by original commit cf75514c8e2e are > still relevant. My vote would be to keep things as is. > In my experience, checkpatch is smart enough to recognise when a long line overflows the 80 character limit because of a single long string, so the two options are not mutually exclusive. In other words, long lines are not allowed except in the case where shortening the line involves splitting a string. There may be a small amount of work in getting checkpatch happy, i.e. by putting the string on a line on it's own, but we can indeed have our cake and eat it too in this case. /Bruce