Change underlying data structure of QPDF_Array in non-sparse mode to std::vector

libqpdf/OHArray.cc

@@ -5,120 +5,82 @@
 
 #include <stdexcept>
 
-OHArray::OHArray() :
+static const QPDFObjectHandle null_oh = QPDFObjectHandle::newNull();
-    n_elements(0)
+
+OHArray::OHArray()
 {
 }
 
 size_t
 OHArray::size() const
 {
-    return this->n_elements;
+    return elements.size();
 }
 
 void
 OHArray::append(QPDFObjectHandle oh)
 {
-    if (!oh.isDirectNull()) {
+    elements.push_back(oh.getObj());
-        this->elements[this->n_elements] = oh;
-    }
-    ++this->n_elements;
 }
 
 void
 OHArray::append(std::shared_ptr<QPDFObject>&& obj)
 {
-    if (obj->getTypeCode() != ::ot_null || !obj->getObjGen().isIndirect()) {
+    elements.push_back(std::move(obj));
-        this->elements[this->n_elements] = std::move(obj);
-    }
-    ++this->n_elements;
 }
 
 QPDFObjectHandle
 OHArray::at(size_t idx) const
 {
-    if (idx >= this->n_elements) {
+    if (idx >= elements.size()) {
         throw std::logic_error(
             "INTERNAL ERROR: bounds error accessing OHArray element");
     }
-    auto const& iter = this->elements.find(idx);
+    auto const& obj = elements.at(idx);
-    if (iter == this->elements.end()) {
+    return obj ? obj : null_oh;
-        return QPDFObjectHandle::newNull();
-    } else {
-        return (*iter).second;
-    }
-}
-
-void
-OHArray::remove_last()
-{
-    if (this->n_elements == 0) {
-        throw std::logic_error("INTERNAL ERROR: attempt to remove"
-                               " last item from empty OHArray");
-    }
-    --this->n_elements;
-    this->elements.erase(this->n_elements);
 }
 
 void
 OHArray::disconnect()
 {
-    for (auto& iter: this->elements) {
+    for (auto const& iter: elements) {
-        QPDFObjectHandle::DisconnectAccess::disconnect(iter.second);
+        if (iter) {
+            QPDFObjectHandle oh = iter;
+            QPDFObjectHandle::DisconnectAccess::disconnect(oh);
+        }
     }
 }
 
 void
 OHArray::setAt(size_t idx, QPDFObjectHandle oh)
 {
-    if (idx >= this->n_elements) {
+    if (idx >= elements.size()) {
         throw std::logic_error("bounds error setting item in OHArray");
     }
-    if (oh.isDirectNull()) {
+    elements[idx] = oh.getObj();
-        this->elements.erase(idx);
-    } else {
-        this->elements[idx] = oh;
-    }
 }
 
 void
 OHArray::erase(size_t idx)
 {
-    if (idx >= this->n_elements) {
+    if (idx >= elements.size()) {
         throw std::logic_error("bounds error erasing item from OHArray");
     }
-    decltype(this->elements) dest;
+    int n = int(idx);
-    for (auto const& iter: this->elements) {
+    elements.erase(elements.cbegin() + n);
-        if (iter.first < idx) {
-            dest.insert(iter);
-        } else if (iter.first > idx) {
-            dest[iter.first - 1] = iter.second;
-        }
-    }
-    this->elements = dest;
-    --this->n_elements;
 }
 
 void
 OHArray::insert(size_t idx, QPDFObjectHandle oh)
 {
-    if (idx > this->n_elements) {
+    if (idx > elements.size()) {
         throw std::logic_error("bounds error inserting item to OHArray");
-    } else if (idx == this->n_elements) {
+    } else if (idx == elements.size()) {
         // Allow inserting to the last position
-        append(oh);
+        append(oh.getObj());
     } else {
-        decltype(this->elements) dest;
+        int n = int(idx);
-        for (auto const& iter: this->elements) {
+        elements.insert(elements.cbegin() + n, oh.getObj());
-            if (iter.first < idx) {
-                dest.insert(iter);
-            } else {
-                dest[iter.first + 1] = iter.second;
-            }
-        }
-        this->elements = dest;
-        this->elements[idx] = oh;
-        ++this->n_elements;
     }
 }
 
@@ -126,23 +88,12 @@ OHArray
 OHArray::copy()
 {
     OHArray result;
-    result.n_elements = this->n_elements;
+    result.elements.reserve(elements.size());
-    for (auto const& element: this->elements) {
+    for (auto const& element: elements) {
-        auto value = element.second;
+        result.elements.push_back(
-        result.elements[element.first] =
+            element ? (element->getObjGen().isIndirect() ? element
-            value.isIndirect() ? value : value.shallowCopy();
+                                                         : element->copy())
+                    : element);
     }
     return result;
 }
-
-OHArray::const_iterator
-OHArray::begin() const
-{
-    return this->elements.begin();
-}
-
-OHArray::const_iterator
-OHArray::end() const
-{
-    return this->elements.end();
-}

libqpdf/QPDF_Array.cc

@@ -205,11 +205,7 @@ QPDF_Array::setFromVector(std::vector<std::shared_ptr<QPDFObject>>&& v)
     } else {
         elements = OHArray();
         for (auto&& item: v) {
-            if (item) {
+            elements.append(std::move(item));
-                elements.append(item);
-            } else {
-                ++elements.n_elements;
-            }
         }
     }
 }

libqpdf/qpdf/OHArray.hh

@@ -2,7 +2,9 @@
 #define QPDF_OHARRAY_HH
 
 #include <qpdf/QPDFObjectHandle.hh>
-#include <unordered_map>
+#include <qpdf/QPDFObject_private.hh>
+
+#include <vector>
 
 class QPDF_Array;
 
@@ -14,22 +16,15 @@ class OHArray
     void append(QPDFObjectHandle oh);
     void append(std::shared_ptr<QPDFObject>&& obj);
     QPDFObjectHandle at(size_t idx) const;
-    void remove_last();
     void setAt(size_t idx, QPDFObjectHandle oh);
     void erase(size_t idx);
     void insert(size_t idx, QPDFObjectHandle oh);
     OHArray copy();
     void disconnect();
 
-    typedef std::unordered_map<size_t, QPDFObjectHandle>::const_iterator
-        const_iterator;
-    const_iterator begin() const;
-    const_iterator end() const;
-
   private:
     friend class QPDF_Array;
-    std::unordered_map<size_t, QPDFObjectHandle> elements;
+    std::vector<std::shared_ptr<QPDFObject>> elements;
-    size_t n_elements;
 };
 
 #endif // QPDF_OHARRAY_HH