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