Refactor QPDF_Array::setItem and rename to setAt

5 files changed, 20 insertions, 32 deletions

diff --git a/libqpdf/QPDFObjectHandle.cc b/libqpdf/QPDFObjectHandle.cc
index e113089a..396e2063 100644
--- a/libqpdf/QPDFObjectHandle.cc
+++ b/libqpdf/QPDFObjectHandle.cc
@@ -904,16 +904,16 @@ QPDFObjectHandle::getArrayAsVector()
 void
 QPDFObjectHandle::setArrayItem(int n, QPDFObjectHandle const& item)
 {
-    auto array = asArray();
-    if (array) {
-        checkOwnership(item);
-        array->setItem(n, item);
+    if (auto array = asArray()) {
+        if (!array->setAt(n, item)) {
+            objectWarning("ignoring attempt to set out of bounds array item");
+            QTC::TC("qpdf", "QPDFObjectHandle set array bounds");
+        }
     } else {
         typeWarning("array", "ignoring attempt to set item");
         QTC::TC("qpdf", "QPDFObjectHandle array ignoring set item");
     }
 }
-
 void
 QPDFObjectHandle::setArrayFromVector(std::vector<QPDFObjectHandle> const& items)
 {
diff --git a/libqpdf/QPDF_Array.cc b/libqpdf/QPDF_Array.cc
index e4e1668e..d7505468 100644
--- a/libqpdf/QPDF_Array.cc
+++ b/libqpdf/QPDF_Array.cc
@@ -1,9 +1,6 @@
 #include <qpdf/QPDF_Array.hh>
 
-#include <qpdf/QIntC.hh>
 #include <qpdf/QPDFObject_private.hh>
-#include <qpdf/QUtil.hh>
-#include <stdexcept>
 
 static const QPDFObjectHandle null_oh = QPDFObjectHandle::newNull();
 
@@ -188,18 +185,19 @@ QPDF_Array::getAsVector(std::vector<QPDFObjectHandle>& v) const
     }
 }
 
-void
-QPDF_Array::setItem(int n, QPDFObjectHandle const& oh)
+bool
+QPDF_Array::setAt(int at, QPDFObjectHandle const& oh)
 {
+    if (at < 0 || at >= size()) {
+        return false;
+    }
+    checkOwnership(oh);
     if (sparse) {
-        sp_elements.setAt(n, oh);
+        sp_elements.setAt(at, oh);
     } else {
-        size_t idx = size_t(n);
-        if (n < 0 || idx >= elements.size()) {
-            throw std::logic_error("bounds error setting item in QPDF_Array");
-        }
-        elements[idx] = oh.getObj();
+        elements[size_t(at)] = oh.getObj();
     }
+    return true;
 }
 
 void
diff --git a/libqpdf/SparseOHArray.cc b/libqpdf/SparseOHArray.cc
index 773d7309..9904af1b 100644
--- a/libqpdf/SparseOHArray.cc
+++ b/libqpdf/SparseOHArray.cc
@@ -31,19 +31,6 @@ SparseOHArray::disconnect()
 }
 
 void
-SparseOHArray::setAt(int idx, QPDFObjectHandle oh)
-{
-    if (idx >= this->n_elements) {
-        throw std::logic_error("bounds error setting item in SparseOHArray");
-    }
-    if (oh.isDirectNull()) {
-        this->elements.erase(idx);
-    } else {
-        this->elements[idx] = oh.getObj();
-    }
-}
-
-void
 SparseOHArray::erase(int at)
 {
     auto end = elements.end();
diff --git a/libqpdf/qpdf/QPDF_Array.hh b/libqpdf/qpdf/QPDF_Array.hh
index edda54ec..f4d1fc3d 100644
--- a/libqpdf/qpdf/QPDF_Array.hh
+++ b/libqpdf/qpdf/QPDF_Array.hh
@@ -28,9 +28,8 @@ class QPDF_Array: public QPDFValue
         return sparse ? sp_elements.size() : int(elements.size());
     }
     QPDFObjectHandle at(int n) const noexcept;
+    bool setAt(int n, QPDFObjectHandle const& oh);
     void getAsVector(std::vector<QPDFObjectHandle>&) const;
-
-    void setItem(int, QPDFObjectHandle const&);
     void setFromVector(std::vector<QPDFObjectHandle> const& items);
     void setFromVector(std::vector<std::shared_ptr<QPDFObject>>&& items);
     bool insert(int at, QPDFObjectHandle const& item);
diff --git a/libqpdf/qpdf/SparseOHArray.hh b/libqpdf/qpdf/SparseOHArray.hh
index f2872618..19aa49f0 100644
--- a/libqpdf/qpdf/SparseOHArray.hh
+++ b/libqpdf/qpdf/SparseOHArray.hh
@@ -28,7 +28,11 @@ class SparseOHArray
     }
     QPDFObjectHandle at(int idx) const;
     void remove_last();
-    void setAt(int idx, QPDFObjectHandle oh);
+    void
+    setAt(int idx, QPDFObjectHandle oh)
+    {
+        elements[idx] = oh.getObj();
+    }
     void erase(int idx);
     void insert(int idx, QPDFObjectHandle oh);
     SparseOHArray copy();