Skip to content

Conversation

@jeking3
Copy link
Collaborator

@jeking3 jeking3 commented Jul 6, 2025

Originally identified by Coverity Scan. Zero length periods like [3, 3) cannot intersect with anything. Invalid periods like [5, 4) also cannot intersect with anything. In both cases the unit tests are wrong, and fixing them reveals the implementation is not canonical for half-open sane intervals.

In testgeneric_period.cpp there is a bug on lines 128-129; again on 149-150:

  check("Intersects", zero_len.intersects(p1));
  check("Intersects", p1.intersects(zero_len));
...
  check("Intersects", null_per.intersects(p1));
  check("Intersects", p1.intersects(null_per));

Empty (null) periods cannot intersect with anything, so the test is wrong. Fixing the test reveals the implementation is wrong.

On simplifying the implementation of intersects to use the half-open canonical check (ensuring the half-open intervals are sane), it now passes.

  template<class point_rep, class duration_rep>
  inline BOOST_CXX14_CONSTEXPR
  bool period<point_rep,duration_rep>::intersects(const period<point_rep,duration_rep>& other) const
  { 
    return !is_null() && !other.is_null() && (begin_ < other.end()) && (other.begin_ < end());
  }
...
  check("Intersects", !zero_len.intersects(p1));
  check("Intersects", !p1.intersects(zero_len));
...
  check("Intersects", !null_per.intersects(p1));
  check("Intersects", !p1.intersects(null_per));
  ***passed***

This fixes #204

Originally identified by Coverity Scan.  Zero length periods like
[3, 3) cannot intersect with anything.  Invalid periods like
[5, 4) also cannot intersect with anything.  In both cases the
unit tests are wrong, and fixing them reveals the implementation
is not canonical for half-open sane intervals.
Copy link
Collaborator

@JeffGarland JeffGarland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks correct to me

@JeffGarland
Copy link
Collaborator

The CI issues seem spurious.

@JeffGarland JeffGarland merged commit 363e04d into boostorg:develop Jul 7, 2025
25 of 30 checks passed
@jeking3 jeking3 deleted the coverity-255065 branch July 7, 2025 01:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Coverity Scan noted possible issue in period() code

2 participants