[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