[Git][NTPsec/ntpsec][master] Coverity -- another try.
Hal Murray
halmurray at sonic.net
Fri Jan 24 20:30:15 UTC 2025
Gary said:
> Uh, I merely suggested the direction to a valid fix. My code was inteded
> to show off where the bug is, not to be an elegant fix. Since you
> claimed it was a bug in Coverity, I showed it is UB in ntpsec code.
Thanks. I think the root problem with this discussion is that I don't see
the bug yet.
Yes, if j and I get screwed up there will be an underflow on the subtract.
Buf they never get screwed up. Here is how it works:
The idea is to toss out the outliers. The array is sorted with n slots.
i is an index to the bottom, init to 0. j is one past the top, init to
n. So j-i is the number of active slots left in the array. m is the
goal, init to 60% of n.
The loop test is
while ((j - i) > m) {
Each time around the loop, either i gets bigger by 1 or j gets smaller
by 1
That's tossing out the bottom or top active slot left in the array
So j-i gets smaller by 1 each time around
j starts out bigger than i, so the subtract if safe.
The test has a >
No matter what m is, j-i won't underflow
if m is big, it bails early
if m is 0, it bails when j-i gets to 0
but that case won't happen, there is a test for n=0 several lines up
if m is something reasonable it will work as expected.
In the Coverity case with n=2, m is 2 so it bails before it goes around the loop. I and j never change.
I don't know how Coverity is supposed to work. Either there is a bug it its emulation of our code, or it isn't trying to do the emulation and is just flipping a coin or such and following the loop a few times. That just flip approach seems like it would make zillions of flase positives so a bug seems more likely.
-----
I tried James' fix: m=n*6/10
Coverity caught a real bug. That rounds down. We need to round up.
--
These are my opinions. I hate spam.
More information about the devel
mailing list