From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Hemminger Subject: Re: [PATCH v5 01/21] lib/librte_ethdev: change eth-dev-ops API to return int Date: Sun, 20 May 2018 23:52:37 -0700 Message-ID: <20180520235237.276dc37b@xeon-e3> References: <152656480225.46638.3271983577765861155.stgit@localhost.localdomain> <152656493702.46638.10712692446180001555.stgit@localhost.localdomain> <70fc4570-7447-a9fa-dda6-a41f220c35ca@warmcat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: Andy Green , "dev@dpdk.org" To: Shreyansh Jain Return-path: Received: from mail-pf0-f196.google.com (mail-pf0-f196.google.com [209.85.192.196]) by dpdk.org (Postfix) with ESMTP id 75B9AA492 for ; Mon, 21 May 2018 08:52:40 +0200 (CEST) Received: by mail-pf0-f196.google.com with SMTP id p12-v6so6594954pff.13 for ; Sun, 20 May 2018 23:52:40 -0700 (PDT) In-Reply-To: List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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". 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).