Refactor QPDF_Array::eraseItem and rename to erase

libqpdf/QPDFObjectHandle.cc

@@ -972,19 +972,16 @@ QPDFObjectHandle::appendItemAndGetNew(QPDFObjectHandle const& item)
 void
 QPDFObjectHandle::eraseItem(int at)
 {
-    auto array = asArray();
-    if (array && at < array->size() && at >= 0) {
-        array->eraseItem(at);
-    } else {
-        if (array) {
+    if (auto array = asArray()) {
+        if (!array->erase(at)) {
             objectWarning("ignoring attempt to erase out of bounds array item");
             QTC::TC("qpdf", "QPDFObjectHandle erase array bounds");
+        }
     } else {
         typeWarning("array", "ignoring attempt to erase item");
         QTC::TC("qpdf", "QPDFObjectHandle array ignoring erase item");
     }
 }
-}
 
 QPDFObjectHandle
 QPDFObjectHandle::eraseItemAndGetOld(int at)

libqpdf/QPDF_Array.cc

@@ -267,17 +267,16 @@ QPDF_Array::push_back(QPDFObjectHandle const& item)
     }
 }
 
-void
-QPDF_Array::eraseItem(int at)
+bool
+QPDF_Array::erase(int at)
 {
+    if (at < 0 || at >= size()) {
+        return false;
+    }
     if (sparse) {
         sp_elements.erase(at);
     } else {
-        size_t idx = QIntC::to_size(at);
-        if (idx >= elements.size()) {
-            throw std::logic_error("bounds error erasing item from OHArray");
-        }
-        int n = int(idx);
-        elements.erase(elements.cbegin() + n);
+        elements.erase(elements.cbegin() + at);
     }
+    return true;
 }

libqpdf/SparseOHArray.cc

@@ -44,21 +44,22 @@ SparseOHArray::setAt(int idx, QPDFObjectHandle oh)
 }
 
 void
-SparseOHArray::erase(int idx)
+SparseOHArray::erase(int at)
 {
-    if (idx >= this->n_elements) {
-        throw std::logic_error("bounds error erasing item from SparseOHArray");
+    auto end = elements.end();
+    if (auto iter = elements.lower_bound(at); iter != end) {
+        if (iter->first == at) {
+            iter++;
+            elements.erase(at);
         }
-    decltype(this->elements) dest;
-    for (auto const& iter: this->elements) {
-        if (iter.first < idx) {
-            dest.insert(iter);
-        } else if (iter.first > idx) {
-            dest[iter.first - 1] = iter.second;
+
+        while (iter != end) {
+            auto nh = elements.extract(iter++);
+            --nh.key();
+            elements.insert(std::move(nh));
         }
     }
-    this->elements = dest;
-    --this->n_elements;
+    --n_elements;
 }
 
 void

libqpdf/qpdf/QPDF_Array.hh

@@ -35,7 +35,7 @@ class QPDF_Array: public QPDFValue
     void setFromVector(std::vector<std::shared_ptr<QPDFObject>>&& items);
     bool insert(int at, QPDFObjectHandle const& item);
     void push_back(QPDFObjectHandle const& item);
-    void eraseItem(int at);
+    bool erase(int at);
 
   private:
     QPDF_Array(std::vector<QPDFObjectHandle> const& items);

qpdf/qtest/qpdf/test88.out

@@ -1,2 +1,3 @@
 WARNING: test array: ignoring attempt to erase out of bounds array item
+WARNING: minimal.pdf, object 1 0 at offset 19: operation for array attempted on object of type dictionary: ignoring attempt to erase item
 test 88 done

qpdf/test_driver.cc

@@ -3283,6 +3283,7 @@ test_88(QPDF& pdf, char const* arg2)
     auto arr2 = pdf.getRoot().replaceKeyAndGetNew("/QTest", "[1 2]"_qpdf);
     arr2.setObjectDescription(&pdf, "test array");
     assert(arr2.eraseItemAndGetOld(50).isNull());
+    assert(pdf.getRoot().eraseItemAndGetOld(0).isNull());
 }
 
 static void