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.

diff --git a/pv/data/analogsegment.cpp b/pv/data/analogsegment.cpp
index 73a3084dde3c1b8afdb2df57f8c24d7eb24003e0..342612aae02e15954671d2044f3bdb337682cd09 100644 (file)
--- a/pv/data/analogsegment.cpp
+++ b/pv/data/analogsegment.cpp
@@ -116,19 +116,11 @@ const pair<float, float> 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 7c74e77678c6e5693f9465dd0bfd221b46c71c3d..df25f0b74a669109663a08e0df4dbeff3b5a7641 100644 (file)
--- 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<float, float> 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 8f5eab67a6eeaa1d1ca7e7ea6c405b555bed9bfd..b77c102d8ba2266de397f42223ffc9d5b4b0e3a5 100644 (file)
--- 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<EdgePair> &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 a18a59f2e154130de84f5b7c009c2f9f50199fff..8cbc1c5dc050d7aac6c03625908ce0be337eecf9 100644 (file)
--- 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 5022d6a900f7064c683c5f0d3ab9f80d5feeb100..a71d49ca6d27b9f2098a844b4c425c9fdfe765f3 100644 (file)
--- a/pv/data/segment.cpp
+++ b/pv/data/segment.cpp
@@ -24,6 +24,8 @@
 #include <cstdlib>
 #include <cstring>
 
+#include <QDebug>
+
 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 9ea9629dcbd0d8e294b07f46c81573a9fc2c4681..4c6c36af3c3b0c0b6245ae0e257a2f1000992ae2 100644 (file)
--- 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 74aed95039c5ddde8cb93e07f1ebb7ec3aa10ddc..1a67899c952c7bd6a95378ab027bc47d6e59f6a5 100644 (file)
--- 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()