Skip to content

Commit 7be08b8

Browse files
committed
Merge #85: Fixes for integer precision loss
00fb4a4 Avoid or make integer precision conversion explicit (Cory Fields) 9d62a4d Avoid the need to cast/convert to size_t for vector operations (Cory Fields) 19e06cc Prevent overflows from large capacity/max_elements (Cory Fields) Pull request description: These are the obvious naive fixes. I'll add comments for some of the alternatives that I see. Fixes #84. ACKs for top commit: sipa: ACK 00fb4a4 Tree-SHA512: 737c4f332936abae2c1b9c6488b0da237400c6c4179565de270c08686e6f9e2825cd784e954c93d4830b9482b1fbe46414677b793f4b616c938ecb7f93094adf
2 parents 3472e2f + 00fb4a4 commit 7be08b8

File tree

5 files changed

+11
-8
lines changed

5 files changed

+11
-8
lines changed

src/false_positives.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,8 @@ uint64_t BaseFPBits(uint32_t bits, uint32_t capacity) {
8181

8282
size_t ComputeCapacity(uint32_t bits, size_t max_elements, uint32_t fpbits) {
8383
if (bits == 0) return 0;
84-
uint64_t base_fpbits = BaseFPBits(bits, max_elements);
84+
if (max_elements > 0xffffffff) return max_elements;
85+
uint64_t base_fpbits = BaseFPBits(bits, static_cast<uint32_t>(max_elements));
8586
// The fpbits provided by the base max_elements==capacity case are sufficient.
8687
if (base_fpbits >= fpbits) return max_elements;
8788
// Otherwise, increment capacity by ceil(fpbits / bits) beyond that.
@@ -90,6 +91,7 @@ size_t ComputeCapacity(uint32_t bits, size_t max_elements, uint32_t fpbits) {
9091

9192
size_t ComputeMaxElements(uint32_t bits, size_t capacity, uint32_t fpbits) {
9293
if (bits == 0) return 0;
94+
if (capacity > 0xffffffff) return capacity;
9395
// Start with max_elements=capacity, and decrease max_elements until the corresponding capacity is capacity.
9496
size_t max_elements = capacity;
9597
while (true) {

src/int_utils.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ class BitsInt {
210210
static constexpr inline int TopBits(I val) {
211211
static_assert(Count > 0, "BitsInt::TopBits needs Count > 0");
212212
static_assert(Count <= BITS, "BitsInt::TopBits needs Offset <= BITS");
213-
return val >> (BITS - Count);
213+
return static_cast<int>(val >> (BITS - Count));
214214
}
215215

216216
static inline constexpr I CondXorWith(I val, bool cond, I v) {

src/minisketch.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -468,7 +468,7 @@ size_t minisketch_merge(minisketch* sketch, const minisketch* other_sketch) {
468468
ssize_t minisketch_decode(const minisketch* sketch, size_t max_elements, uint64_t* output) {
469469
const Sketch* s = (const Sketch*)sketch;
470470
s->Check();
471-
return s->Decode(max_elements, output);
471+
return s->Decode(static_cast<int>(max_elements), output);
472472
}
473473

474474
void minisketch_set_seed(minisketch* sketch, uint64_t seed) {

src/sketch.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ class Sketch
2929
virtual ~Sketch() {}
3030
virtual size_t Syndromes() const = 0;
3131

32-
virtual void Init(int syndromes) = 0;
32+
virtual void Init(size_t syndromes) = 0;
3333
virtual void Add(uint64_t element) = 0;
3434
virtual void Serialize(unsigned char*) const = 0;
3535
virtual void Deserialize(const unsigned char*) = 0;

src/sketch_impl.h

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,8 @@ template<typename F>
9292
void Sqr(std::vector<typename F::Elem>& poly, const F& field) {
9393
if (poly.size() == 0) return;
9494
poly.resize(poly.size() * 2 - 1);
95-
for (int x = poly.size() - 1; x >= 0; --x) {
95+
for (size_t i = 0; i < poly.size(); ++i) {
96+
auto x = poly.size() - i - 1;
9697
poly[x] = (x & 1) ? 0 : field.Sqr(poly[x / 2]);
9798
}
9899
}
@@ -297,7 +298,7 @@ std::vector<typename F::Elem> BerlekampMassey(const std::vector<typename F::Elem
297298
auto discrepancy = syndromes[n];
298299
for (size_t i = 1; i < current.size(); ++i) discrepancy ^= table[n - i](current[i]);
299300
if (discrepancy != 0) {
300-
int x = n + 1 - (current.size() - 1) - (prev.size() - 1);
301+
int x = static_cast<int>(n + 1 - (current.size() - 1) - (prev.size() - 1));
301302
if (!b_have_inv) {
302303
b_inv = field.Inv(b);
303304
b_have_inv = true;
@@ -366,7 +367,7 @@ class SketchImpl final : public Sketch
366367
}
367368

368369
size_t Syndromes() const override { return m_syndromes.size(); }
369-
void Init(int count) override { m_syndromes.assign(count, 0); }
370+
void Init(size_t count) override { m_syndromes.assign(count, 0); }
370371

371372
void Add(uint64_t val) override
372373
{
@@ -405,7 +406,7 @@ class SketchImpl final : public Sketch
405406
for (const auto& root : roots) {
406407
*(out++) = m_field.ToUint64(root);
407408
}
408-
return roots.size();
409+
return static_cast<int>(roots.size());
409410
}
410411

411412
size_t Merge(const Sketch* other_sketch) override

0 commit comments

Comments
 (0)