From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f53.google.com (mail-wm0-f53.google.com [74.125.82.53]) by dpdk.org (Postfix) with ESMTP id 79B431B262 for ; Mon, 2 Oct 2017 18:21:18 +0200 (CEST) Received: by mail-wm0-f53.google.com with SMTP id i82so9489133wmd.3 for ; Mon, 02 Oct 2017 09:21:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=cA0+7E25u6/dooeZRDtTAJ6D5QDIIgRByQTJGA3CCL0=; b=aCPK08Nb3fXqCxLUnQy0P3CJEKelx9OtBNeV+wL/v2/7d7dce/nzTQl5y8dq8G2h+y QJlpTPATOji6sJXPKc/FrXyDSyviYOPQD3RN3iq2TqqUlM58tCW2lyTVwdPyKC3Nvs9e yGN5JId6YK7R7aORQ1Y6N/v9rMfFH9pdumiUpeIPMA2oGuqZ2m9yjKMIku2e1j37z2+L 7zohPER3seuIqXg4gD/jNd+qipsBo3FTo4vKoDOnemLKb0p0rsILPKdYlRd6yIBxEgp1 5hgxrwHiBnX242AGFUXagTN/mGI3o1XnWQTeRR+r6HCluCnCBUB0IcF3IRjpLh5g71DD HGzw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=cA0+7E25u6/dooeZRDtTAJ6D5QDIIgRByQTJGA3CCL0=; b=qktQIOUE0HvU/ngVeTkn5Mz7lh2uMubaKJ8FeO49eWq5jxYVTdbvLTdbfrACfidumv eJ0ImIjJvjq579oVPQU7UQ2/yg7dPZCeId1WA8el/l19fWuKX0dRvyZ4hpa0APBqhago /iqfjLDTRH34bLI5GQdp5ZVMdZimVIuYEZ0Jhw8qXsbqRM0x4EdhsB2wRnheP3RkspUG YF5LQwoxZmWXdL1nvpY3rLoLimC0G1iMwvTQgCCfCRHvFfzpHy7o43p3zZLtPpG/haQO VJbQKBdtJPKYQEAKoTbttDL6AN2Jhf+lSNjbB+by052z2v7ZooD4oPZB3aejDPW0ZCzh T5dw== X-Gm-Message-State: AMCzsaUgDTYa9rlsI0bnlrJDVbST17bPA1xVYc/4i/9uZGwBKYorMpSS OHd11W075BpRNi+AytNjhgFfnWFl X-Google-Smtp-Source: AOwi7QBOU33hpkf3QBPBEgcmk1jwcifvQ5I8HcT9vyTUBHwLfJ3bThfutVyrWWarLR1xmo6V2SL5qA== X-Received: by 10.28.217.136 with SMTP id q130mr3296153wmg.89.1506961278024; Mon, 02 Oct 2017 09:21:18 -0700 (PDT) Received: from 6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id f89sm11825059wmh.18.2017.10.02.09.21.17 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 02 Oct 2017 09:21:17 -0700 (PDT) Date: Mon, 2 Oct 2017 18:21:06 +0200 From: Adrien Mazarguil To: Bruce Richardson Cc: Stephen Hemminger , dev@dpdk.org Message-ID: <20171002162106.GQ3871@6wind.com> References: <20170929153749.9806-1-stephen@networkplumber.org> <20171002115317.GJ3871@6wind.com> <20171002134624.GA10500@bricha3-MOBL3.ger.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20171002134624.GA10500@bricha3-MOBL3.ger.corp.intel.com> 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 16:21:18 -0000 On Mon, Oct 02, 2017 at 02:46:24PM +0100, Bruce Richardson wrote: > 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. I can't seem to get around warnings without ignoring either SPLIT_STRING or LONG_LINE_STRING as of Linux v4.14-rc3's checkpatch.pl. I think you can only get around them by fooling it somehow. You really need to ignore at least LONG_LINE_STRING to meet the requirements of the commit log. However SPLIT_STRING still looks necessary to address part of cf75514c8e2e ("devtools: ignore warning on long log string"): "...lines that make use of PRIx64 with string concatenation will still be flagged if the beginning of the last string fragment begins after the 80 character threshold." It's not all that uncommon in my opinion. -- Adrien Mazarguil 6WIND