From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.warmcat.com (mail.warmcat.com [163.172.24.82]) by dpdk.org (Postfix) with ESMTP id 8D07D7F6D for ; Mon, 21 May 2018 09:26:20 +0200 (CEST) To: Stephen Hemminger , Shreyansh Jain Cc: "dev@dpdk.org" References: <152656480225.46638.3271983577765861155.stgit@localhost.localdomain> <152656493702.46638.10712692446180001555.stgit@localhost.localdomain> <70fc4570-7447-a9fa-dda6-a41f220c35ca@warmcat.com> <20180520235237.276dc37b@xeon-e3> From: Andy Green Message-ID: <8d4af186-09d9-14ff-19e6-a23936daba2a@warmcat.com> Date: Mon, 21 May 2018 15:25:48 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 In-Reply-To: <20180520235237.276dc37b@xeon-e3> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v5 01/21] lib/librte_ethdev: change eth-dev-ops API to return int 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, 21 May 2018 07:26:20 -0000 On 05/21/2018 02:52 PM, Stephen Hemminger wrote: > On Sun, 20 May 2018 02:43:58 +0000 > Shreyansh Jain wrote: > >>>> This doesn't feel correct. A counter, especially the number of >>> descriptors in a queue, doesn't have a negative value. So, 1) this is >>> an unnatural return and 2) we litter the code with unnecessary >>> typecast. >>>> >>>> In fact, even in the above change, the debug messages continue to >>> print unsigned values. So, another typecast would be required there. >>>> >>>> I don't agree with this change - at least not until some strong gcc 8 >>> warning reason is triggering this. Can you please point me to some >>> conversation on mailing list which enforces this? >>>> >>> >>> hmmmmm.... no, it's not my idea. >>> >>> If you don't like it, don't do it, I don't mind either way. I sent a >>> patch that just solved the compiler error only already, and was told on >>> the list it would be cooler if these things returned an int instead. >>> >>> There's no point challenging me about the wisdom of it, although it >>> seems reasonable to me. I sent a patch, list guy $1 says do X instead, >>> I do X and then list guy $2 says EXPLAIN YOURSELF. >> >> That is what a community is. Consensus has to be built, not expected automagically. If you touch a line, you are responsible for it (also, because in future git blame would point *you* out for a change). > > > My comment was a suggestion, not a "you must do it this way". OK. > The reason was it was cleaner change for Gcc fix > and it allowed for possibility that some driver might not detect an error > (for example if device was removed by hot plug). Yes, I also thought it was reasonable. Even if it was somehow unreasonable, it's selfcontained enough in terms of what it does that it shouldn't blow anything up. But it only took me five minutes. Binning that and just directly fixing the compiler warning is also 100% fine from my side and no worries. The main thing is 18.05 should be usable to build with things that want to bind to dpdk using contemporary tools like gcc8.1. If we can manage that, we can build on it for helping lagopus get ahead too. -Andy