Skip to content

Commit fe1abfa

Browse files
authored
Fix TRange issues with clip, contains and random (#2987)
**Avoid truncation of parameter to TRange `clip()` and `contains()` methods** For instance, if `TRange<int8_t>::clip()` is passed an `int` value which exceeds the natural range of `int8_t` then it is truncated and the clipping returns an unexpected value. For example: ```c++ TRange<int8_t> range(0, 100); /* Compiler catches this with an overflow error, attempting to convert `int` to `int8_t` */ range.clip(0x1005); /* But here the compiler says nothing, and 0x1005 is silently truncated to 0x05, thus 5 is returned where 100 is expected */ int v = 0x1005; range.clip(v); ``` Both `clip` and `contain` methods are now declared `constexpr` so they can be evaluated at compile time if required. **Fix random() for 64-bit values.** May need second 32-bit random value to cover range properly Add tests.
1 parent e1daf4d commit fe1abfa

File tree

3 files changed

+66
-5
lines changed

3 files changed

+66
-5
lines changed

Sming/Core/Data/Range.h

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
#pragma once
1212

1313
#include <WString.h>
14+
#include <limits>
1415
#include <esp_systemapi.h>
1516

1617
/**
@@ -80,7 +81,7 @@ template <typename T> struct TRange {
8081
/**
8182
* @brief Determine if range contains a value
8283
*/
83-
bool contains(T value) const
84+
template <typename V> constexpr bool contains(V value) const
8485
{
8586
return (value >= min) && (value <= max);
8687
}
@@ -96,21 +97,24 @@ template <typename T> struct TRange {
9697
/**
9798
* @brief Clip values to within the range
9899
*/
99-
T clip(T value) const
100+
template <typename V> constexpr T clip(V value) const
100101
{
101-
return (value < min) ? min : (value > max) ? max : value;
102+
return (value < min) ? min : (value > max) ? max : T(value);
102103
}
103104

104105
/**
105106
* @brief Return a random value within the range
106107
*/
107108
T random() const
108109
{
109-
auto n = 1 + max - min;
110+
uint64_t n = 1 + max - min;
110111
if(n == 0) {
111112
return 0;
112113
}
113-
auto value = os_random();
114+
T value = os_random();
115+
if(n > std::numeric_limits<uint32_t>::max()) {
116+
value |= uint64_t(os_random()) << 32;
117+
}
114118
return min + value % n;
115119
}
116120

tests/HostTests/include/modules.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
XX(Libc) \
2323
XX(PreCache) \
2424
XX(BitSet) \
25+
XX(Range) \
2526
XX(String) \
2627
XX(ArduinoString) \
2728
XX(Wiring) \

tests/HostTests/modules/Range.cpp

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
#include <HostTests.h>
2+
#include <Data/Range.h>
3+
4+
class RangeTest : public TestGroup
5+
{
6+
public:
7+
RangeTest() : TestGroup(_F("Range"))
8+
{
9+
}
10+
11+
void execute() override
12+
{
13+
TEST_CASE("constexpr")
14+
{
15+
constexpr TRange<int> range(0, 100);
16+
constexpr auto int64 = 120000000000LL;
17+
constexpr auto val = range.clip(int64);
18+
static_assert(val == 100, "Bad clip");
19+
20+
constexpr int64_t tmp = 0x8000000000LL;
21+
static_assert(!range.contains(tmp));
22+
23+
for(unsigned i = 0; i < 10; ++i) {
24+
Serial << range.random() << endl;
25+
}
26+
}
27+
28+
TEST_CASE("truncation")
29+
{
30+
constexpr TRange<int8_t> range(0, 100);
31+
int val = 0x1020;
32+
val = range.clip(val);
33+
REQUIRE_EQ(val, 100);
34+
}
35+
36+
TEST_CASE("Membership")
37+
{
38+
constexpr TRange<int8_t> range(0, 100);
39+
int val = 0x8000;
40+
REQUIRE(!range.contains(val));
41+
}
42+
43+
TEST_CASE("Random")
44+
{
45+
constexpr TRange<int64_t> range(-0x10000000000LL, 0x10000000000LL);
46+
for(unsigned i = 0; i < 10; ++i) {
47+
Serial << range.random() << endl;
48+
}
49+
}
50+
}
51+
};
52+
53+
void REGISTER_TEST(Range)
54+
{
55+
registerGroup<RangeTest>();
56+
}

0 commit comments

Comments
 (0)