From 65c92359634f672e5f472a5214719dabc7e20883 Mon Sep 17 00:00:00 2001 From: Soeren Apel Date: Tue, 9 Oct 2018 08:10:39 +0200 Subject: [PATCH] Segments: Fix iterator access to underlying value Before this change, it->value was accessed even when the iterator went past the end of the underlying buffer. --- pv/data/analogsegment.cpp | 32 ++++++++++++-------------------- pv/data/analogsegment.hpp | 10 +--------- pv/data/logicsegment.cpp | 25 +++++-------------------- pv/data/logicsegment.hpp | 10 ---------- pv/data/segment.cpp | 24 +++++++++++++----------- pv/data/segment.hpp | 10 +++++----- test/data/segment.cpp | 9 ++++----- 7 files changed, 40 insertions(+), 80 deletions(-) diff --git a/pv/data/analogsegment.cpp b/pv/data/analogsegment.cpp index 73a3084d..342612aa 100644 --- a/pv/data/analogsegment.cpp +++ b/pv/data/analogsegment.cpp @@ -116,19 +116,11 @@ const pair AnalogSegment::get_min_max() const return make_pair(min_value_, max_value_); } -SegmentAnalogDataIterator* AnalogSegment::begin_sample_iteration(uint64_t start) +float* AnalogSegment::get_iterator_value_ptr(SegmentDataIterator* it) { - return (SegmentAnalogDataIterator*)begin_raw_sample_iteration(start); -} + assert(it->sample_index <= (sample_count_ - 1)); -void AnalogSegment::continue_sample_iteration(SegmentAnalogDataIterator* it, uint64_t increase) -{ - Segment::continue_raw_sample_iteration((SegmentRawDataIterator*)it, increase); -} - -void AnalogSegment::end_sample_iteration(SegmentAnalogDataIterator* it) -{ - Segment::end_raw_sample_iteration((SegmentRawDataIterator*)it); + return (float*)(it->chunk + it->chunk_offs); } void AnalogSegment::get_envelope_section(EnvelopeSection &s, @@ -171,7 +163,7 @@ void AnalogSegment::append_payload_to_envelope_levels() Envelope &e0 = envelope_levels_[0]; uint64_t prev_length; EnvelopeSample *dest_ptr; - SegmentRawDataIterator* it; + SegmentDataIterator* it; // Expand the data buffer to fit the new samples prev_length = e0.length; @@ -180,16 +172,16 @@ void AnalogSegment::append_payload_to_envelope_levels() // Calculate min/max values in case we have too few samples for an envelope const float old_min_value = min_value_, old_max_value = max_value_; if (sample_count_ < EnvelopeScaleFactor) { - it = begin_raw_sample_iteration(0); + it = begin_sample_iteration(0); for (uint64_t i = 0; i < sample_count_; i++) { - const float sample = *((float*)it->value); + const float sample = *get_iterator_value_ptr(it); if (sample < min_value_) min_value_ = sample; if (sample > max_value_) max_value_ = sample; - continue_raw_sample_iteration(it, 1); + continue_sample_iteration(it, 1); } - end_raw_sample_iteration(it); + end_sample_iteration(it); } // Break off if there are no new samples to compute @@ -204,9 +196,9 @@ void AnalogSegment::append_payload_to_envelope_levels() uint64_t start_sample = prev_length * EnvelopeScaleFactor; uint64_t end_sample = e0.length * EnvelopeScaleFactor; - it = begin_raw_sample_iteration(start_sample); + it = begin_sample_iteration(start_sample); for (uint64_t i = start_sample; i < end_sample; i += EnvelopeScaleFactor) { - const float* samples = (float*)it->value; + const float* samples = get_iterator_value_ptr(it); const EnvelopeSample sub_sample = { *min_element(samples, samples + EnvelopeScaleFactor), @@ -218,10 +210,10 @@ void AnalogSegment::append_payload_to_envelope_levels() if (sub_sample.max > max_value_) max_value_ = sub_sample.max; - continue_raw_sample_iteration(it, EnvelopeScaleFactor); + continue_sample_iteration(it, EnvelopeScaleFactor); *dest_ptr++ = sub_sample; } - end_raw_sample_iteration(it); + end_sample_iteration(it); // Compute higher level mipmaps for (unsigned int level = 1; level < ScaleStepCount; level++) { diff --git a/pv/data/analogsegment.hpp b/pv/data/analogsegment.hpp index 7c74e776..df25f0b7 100644 --- a/pv/data/analogsegment.hpp +++ b/pv/data/analogsegment.hpp @@ -38,12 +38,6 @@ namespace data { class Analog; -typedef struct { - uint64_t sample_index, chunk_num, chunk_offs; - uint8_t* chunk; - float* value; -} SegmentAnalogDataIterator; - class AnalogSegment : public Segment { Q_OBJECT @@ -90,9 +84,7 @@ public: const pair get_min_max() const; - SegmentAnalogDataIterator* begin_sample_iteration(uint64_t start); - void continue_sample_iteration(SegmentAnalogDataIterator* it, uint64_t increase); - void end_sample_iteration(SegmentAnalogDataIterator* it); + float* get_iterator_value_ptr(SegmentDataIterator* it); void get_envelope_section(EnvelopeSection &s, uint64_t start, uint64_t end, float min_length) const; diff --git a/pv/data/logicsegment.cpp b/pv/data/logicsegment.cpp index 8f5eab67..b77c102d 100644 --- a/pv/data/logicsegment.cpp +++ b/pv/data/logicsegment.cpp @@ -184,21 +184,6 @@ void LogicSegment::get_samples(int64_t start_sample, get_raw_samples(start_sample, (end_sample - start_sample), dest); } -SegmentLogicDataIterator* LogicSegment::begin_sample_iteration(uint64_t start) -{ - return (SegmentLogicDataIterator*)begin_raw_sample_iteration(start); -} - -void LogicSegment::continue_sample_iteration(SegmentLogicDataIterator* it, uint64_t increase) -{ - Segment::continue_raw_sample_iteration((SegmentRawDataIterator*)it, increase); -} - -void LogicSegment::end_sample_iteration(SegmentLogicDataIterator* it) -{ - Segment::end_raw_sample_iteration((SegmentRawDataIterator*)it); -} - void LogicSegment::get_subsampled_edges( vector &edges, uint64_t start, uint64_t end, @@ -429,7 +414,7 @@ void LogicSegment::append_payload_to_mipmap() MipMapLevel &m0 = mip_map_[0]; uint64_t prev_length; uint8_t *dest_ptr; - SegmentRawDataIterator* it; + SegmentDataIterator* it; uint64_t accumulator; unsigned int diff_counter; @@ -449,23 +434,23 @@ void LogicSegment::append_payload_to_mipmap() const uint64_t start_sample = prev_length * MipMapScaleFactor; const uint64_t end_sample = m0.length * MipMapScaleFactor; - it = begin_raw_sample_iteration(start_sample); + it = begin_sample_iteration(start_sample); for (uint64_t i = start_sample; i < end_sample;) { // Accumulate transitions which have occurred in this sample accumulator = 0; diff_counter = MipMapScaleFactor; while (diff_counter-- > 0) { - const uint64_t sample = unpack_sample(it->value); + const uint64_t sample = unpack_sample(get_iterator_value(it)); accumulator |= last_append_sample_ ^ sample; last_append_sample_ = sample; - continue_raw_sample_iteration(it, 1); + continue_sample_iteration(it, 1); i++; } pack_sample(dest_ptr, accumulator); dest_ptr += unit_size_; } - end_raw_sample_iteration(it); + end_sample_iteration(it); // Compute higher level mipmaps for (unsigned int level = 1; level < ScaleStepCount; level++) { diff --git a/pv/data/logicsegment.hpp b/pv/data/logicsegment.hpp index a18a59f2..8cbc1c5d 100644 --- a/pv/data/logicsegment.hpp +++ b/pv/data/logicsegment.hpp @@ -48,12 +48,6 @@ namespace data { class Logic; -typedef struct { - uint64_t sample_index, chunk_num, chunk_offs; - uint8_t* chunk; - uint8_t* value; -} SegmentLogicDataIterator; - class LogicSegment : public Segment { Q_OBJECT @@ -86,10 +80,6 @@ public: void get_samples(int64_t start_sample, int64_t end_sample, uint8_t* dest) const; - SegmentLogicDataIterator* begin_sample_iteration(uint64_t start); - void continue_sample_iteration(SegmentLogicDataIterator* it, uint64_t increase); - void end_sample_iteration(SegmentLogicDataIterator* it); - /** * Parses a logic data segment to generate a list of transitions * in a time interval to a given level of detail. diff --git a/pv/data/segment.cpp b/pv/data/segment.cpp index 5022d6a9..a71d49ca 100644 --- a/pv/data/segment.cpp +++ b/pv/data/segment.cpp @@ -24,6 +24,8 @@ #include #include +#include + using std::bad_alloc; using std::lock_guard; using std::min; @@ -240,9 +242,9 @@ void Segment::get_raw_samples(uint64_t start, uint64_t count, } } -SegmentRawDataIterator* Segment::begin_raw_sample_iteration(uint64_t start) +SegmentDataIterator* Segment::begin_sample_iteration(uint64_t start) { - SegmentRawDataIterator* it = new SegmentRawDataIterator; + SegmentDataIterator* it = new SegmentDataIterator; assert(start < sample_count_); @@ -252,17 +254,12 @@ SegmentRawDataIterator* Segment::begin_raw_sample_iteration(uint64_t start) it->chunk_num = (start * unit_size_) / chunk_size_; it->chunk_offs = (start * unit_size_) % chunk_size_; it->chunk = data_chunks_[it->chunk_num]; - it->value = it->chunk + it->chunk_offs; return it; } -void Segment::continue_raw_sample_iteration(SegmentRawDataIterator* it, uint64_t increase) +void Segment::continue_sample_iteration(SegmentDataIterator* it, uint64_t increase) { - // Fail gracefully if we are asked to deliver data we don't have - if (it->sample_index > sample_count_) - return; - it->sample_index += increase; it->chunk_offs += (increase * unit_size_); @@ -271,11 +268,9 @@ void Segment::continue_raw_sample_iteration(SegmentRawDataIterator* it, uint64_t it->chunk_offs -= chunk_size_; it->chunk = data_chunks_[it->chunk_num]; } - - it->value = it->chunk + it->chunk_offs; } -void Segment::end_raw_sample_iteration(SegmentRawDataIterator* it) +void Segment::end_sample_iteration(SegmentDataIterator* it) { delete it; @@ -287,5 +282,12 @@ void Segment::end_raw_sample_iteration(SegmentRawDataIterator* it) } } +uint8_t* Segment::get_iterator_value(SegmentDataIterator* it) +{ + assert(it->sample_index <= (sample_count_ - 1)); + + return (it->chunk + it->chunk_offs); +} + } // namespace data } // namespace pv diff --git a/pv/data/segment.hpp b/pv/data/segment.hpp index 9ea9629d..4c6c36af 100644 --- a/pv/data/segment.hpp +++ b/pv/data/segment.hpp @@ -51,8 +51,7 @@ namespace data { typedef struct { uint64_t sample_index, chunk_num, chunk_offs; uint8_t* chunk; - uint8_t* value; -} SegmentRawDataIterator; +} SegmentDataIterator; class Segment : public QObject { @@ -87,9 +86,10 @@ protected: void append_samples(void *data, uint64_t samples); void get_raw_samples(uint64_t start, uint64_t count, uint8_t *dest) const; - SegmentRawDataIterator* begin_raw_sample_iteration(uint64_t start); - void continue_raw_sample_iteration(SegmentRawDataIterator* it, uint64_t increase); - void end_raw_sample_iteration(SegmentRawDataIterator* it); + SegmentDataIterator* begin_sample_iteration(uint64_t start); + void continue_sample_iteration(SegmentDataIterator* it, uint64_t increase); + void end_sample_iteration(SegmentDataIterator* it); + uint8_t* get_iterator_value(SegmentDataIterator* it); uint32_t segment_id_; mutable recursive_mutex mutex_; diff --git a/test/data/segment.cpp b/test/data/segment.cpp index 74aed950..1a67899c 100644 --- a/test/data/segment.cpp +++ b/test/data/segment.cpp @@ -286,15 +286,14 @@ BOOST_AUTO_TEST_CASE(MaxSize32MultiIterated) BOOST_CHECK(s.get_sample_count() == num_samples); - pv::data::SegmentRawDataIterator* it = s.begin_raw_sample_iteration(0); + pv::data::SegmentDataIterator* it = s.begin_sample_iteration(0); for (uint32_t i = 0; i < num_samples; i++) { - uint8_t* sample_data = it->value; - BOOST_CHECK_EQUAL(*((uint32_t*)sample_data), i); - s.continue_raw_sample_iteration(it, 1); + BOOST_CHECK_EQUAL(*((uint32_t*)s.get_iterator_value(it)), i); + s.continue_sample_iteration(it, 1); } - s.end_raw_sample_iteration(it); + s.end_sample_iteration(it); } BOOST_AUTO_TEST_SUITE_END() -- 2.30.2