[dpdk-dev] [PATCH v5 01/21] lib/librte_ethdev: change eth-dev-ops API to return int

Andy Green andy at warmcat.com
Mon May 21 09:25:48 CEST 2018



On 05/21/2018 02:52 PM, Stephen Hemminger wrote:
> On Sun, 20 May 2018 02:43:58 +0000
> Shreyansh Jain <shreyansh.jain at nxp.com> 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


More information about the dev mailing list