Wireshark-dev: Re: [Wireshark-dev] tvb_length_remaining()and tvb_reported_length_remaining()
Jaap Keuter <jaap.keuter@...> writes:
> Basically it comes down to: check your functions nul return value, whatever it
> is. That's a reviewers job, with Coverity's help.
Well, the problem here is that not everyone is writing their code to properly
check the return value in all cases, and reviewers don't always catch the
problem. And I don't think very many people are actually using Coverity, so
even if the tools detect it, it doesn't really help if nobody is reviewing the
problems it reports.
So, any thoughts on just changing the return value of these functions?
> > While it's easy enough to fix this bug by declaring len2 as a gint instead of
> > as a guint32, I was wondering if it might possibly be better to change the
behavior
> > of tvb_length_remaining() and tvb_reported_length_remaining() to return 0
> > instead of -1? This would obviously lead to other problems though, such as
> > code that might only test for "tvb_length_remaining()< 0" for example.
I haven't really seen cases where the return values are only checked for being
less than 0, but even if there were some, the code would have to deal with the
possibility of a 0 return value anyway. My feeling is that whatever problems
might arise from returning 0 instead of -1 would be far less problematic than
what we have today.
Well, maybe there is a good use case for having them return -1, in which case
we're stuck with what we've got I guess unless someone can come up with a better
alternative.
