From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f44.google.com (mail-wm0-f44.google.com [74.125.82.44]) by dpdk.org (Postfix) with ESMTP id 8D6592A6C for ; Tue, 23 Feb 2016 18:15:53 +0100 (CET) Received: by mail-wm0-f44.google.com with SMTP id g62so3959022wme.0 for ; Tue, 23 Feb 2016 09:15:53 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:date:message-id:organization:user-agent :in-reply-to:references:mime-version:content-transfer-encoding :content-type; bh=AI6uKkR2wlZ3aIf8o+C7o7iIuFMJb344OZEX8+zk0OM=; b=RMDTx41pEHQtKemd5lhGOYQ8X+53iHy6u8OT5VYbgSnTZqmN4h4g2G5V/xr/uiW3Hd 88JOr9zvDfVK19ppKtTV0ZsDCUMzvRkGIlpoRAAuYfKRZoAYGLUaYTkYLCPalswrYJT3 YnZN6UP2ZJ8c3dm0wqard85BmCbuWUKcNScznKWzy/F5nNV84b0WgouJzdJHuGabZBlR FOtkSCaE5UNSqTjw+VCnEXEHyGsROnxlpLtJXrhMlsYpGj1gx3KV9sOMwHzs7s8rRzaG 8bIEU6kqX1mpatGYesryb4yskpyPTei+BsEayK6Y8NcHu0Bmq9ByH/F05VBHaYc8Jo7S B8lg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:organization :user-agent:in-reply-to:references:mime-version :content-transfer-encoding:content-type; bh=AI6uKkR2wlZ3aIf8o+C7o7iIuFMJb344OZEX8+zk0OM=; b=dUEVaXqK4hD90/xgU+q9k7PoPAaCxp4xQR4YyKyO5cWxUw1iRf9Ng+HK/xX5F7GpgC 7HvTmpcVTlsCDGEEzLIggyNsVZ9xHG6QQlbZ4pQ/XR/uGVv/w/6qvgRGbTzn9gD5Gnlh gCJwPC+lP6NO3IXZCyHqnyM2rJvTWhMXeA/mvhARQcoMEk2VYKqm/pOtCuON4PB1/AwF Dh4PD5eM/oCWhDXNjbeWRCSM2rq7NezgfxewoPyYXyJz+aKVaOUsD1RmwNF2t+7bzRuI t2kHg5RTgxEyiFXbhyt33MiOU6uKkfZZpfvTYY4hC7jW1jtxBCFKwY232K1oSNRlqNuL Bvfg== X-Gm-Message-State: AG10YOTwqrA8o+omIgiN6qkeMUP6RJ1eagzRLsyO/oh46eo7Lqlr2CYdyc3BCPpHABJ6vTpq X-Received: by 10.28.221.136 with SMTP id u130mr19037319wmg.40.1456247753404; Tue, 23 Feb 2016 09:15:53 -0800 (PST) Received: from xps13.localnet (171.36.101.84.rev.sfr.net. [84.101.36.171]) by smtp.gmail.com with ESMTPSA id e14sm27234168wmi.21.2016.02.23.09.15.52 (version=TLSv1/SSLv3 cipher=OTHER); Tue, 23 Feb 2016 09:15:52 -0800 (PST) From: Thomas Monjalon To: Bruce Richardson Date: Tue, 23 Feb 2016 18:14:19 +0100 Message-ID: <2522248.H1QVqHKcgq@xps13> Organization: 6WIND User-Agent: KMail/4.14.10 (Linux/4.1.6-1-ARCH; KDE/4.14.11; x86_64; ; ) In-Reply-To: <20160223151304.GA17644@bricha3-MOBL3> References: <1454063522-1948-1-git-send-email-adrien.mazarguil@6wind.com> <20160218161016.GQ27079@6wind.com> <20160223151304.GA17644@bricha3-MOBL3> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH 4/5] mlx5: add support for flow director X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 23 Feb 2016 17:15:53 -0000 2016-02-23 15:13, Bruce Richardson: > On Thu, Feb 18, 2016 at 05:10:16PM +0100, Adrien Mazarguil wrote: > > Hi Bruce, > > > > On Wed, Feb 17, 2016 at 05:13:44PM +0000, Bruce Richardson wrote: > > > Hi Adrien, Yaacov, > > > > > > this patch raises a lot of warnings (17) with checkpatch. Can you perhaps look > > > to see if this number can be reduced. > > > > We actually did it before submitting that patch, there is indeed a bunch of > > remaining warnings that have been left on purpose. Not sure if we have the > > same configuration for checkpatch, but they should fall into the following > > categories: > > > > - "WARNING: return of an errno should typically be negative" - All return > > values are documented in the code. Since this PMD uses syscalls in its > > control path, it uses positive errno values internally for > > consistency. Public callback functions however return negative error > > values. > > > > - "WARNING: line over 80 characters" - Well, although I'm a big fan of the > > 80 characters limit, breaking those would have made the code harder to > > read. > > > > - "WARNING: Missing a blank line after declarations" - It's actually a > > declaration through a macro, there is no missing blank line. > > > > - "WARNING: networking block comments don't use an empty /* line" - Not sure > > if we really care? I don't particularly mind. > > > > - "CHECK: Comparison to NULL could be written "!" - I do not mind either, > > writing the full check seems clearer to me. > > > > - "CHECK: Unnecessary parentheses around fdir_info->mask" - Looks like a > > valid, although minor error. > > > > Please tell me which of these still need to be fixed. > > > Hi Adrien, > > sorry for the delayed reply, I was out for a couple of days. > > As none of the above are errors, I'm not going to mandate that they be fixed > before I merge in the patch, so long as you as maintainer are happy with them. > > My request mainly came about because of the sheer number of warnings that were > being flagged. To keep the codebase clean requires constant discipline, so I > don't like seeing patches where 17 warnings are flagged. I was hoping since > a new rev of the set was likely anyway that some steps could be taken to reduce > that number. > > Thomas, any thoughts here, since I'm still "learning the ropes" as committer. > Do you have any rules-of-thumb or guidelines as regards checkpatch warnings? The > contributor guide only seems to cover running checkpatch, not anything about > what to do with the output. I totally agree with you, Bruce. Everybody must make some effort to keep consistency and avoid coding style exceptions. Some code areas are not yet fully compliant with the rules, depending of their history and... their maintainers ;) I think we can tolerate some exceptions like for the 80 char limit. Some checks may be disabled after discussion (networking block comments?). Other checks deserve to be followed more strictly (e.g. negative errno).