Add new method `BaseHandle::size`

Refactor array handling in `QPDFObjectHandle`: replace `int` with `size_t` for size and index operations, introduce utility functions for type conversions, and simplify sparse array logic.

Add new method `BaseHandle::size`
Refactor array handling in `QPDFObjectHandle`: replace `int` with `size_t` for size and index operations, introduce utility functions for type conversions, and simplify sparse array logic.
m-holger
1 parent 06db6493
Showing 6 changed files with 124 additions and 62 deletions
include/qpdf/ObjectHandle.hh
libqpdf/QPDFObjectHandle.cc
libqpdf/QPDF_Array.cc
libqpdf/qpdf/QPDFObjectHandle_private.hh
libqpdf/qpdf/QPDFObject_private.hh
libtests/sparse_array.cc
@@ -61,6 +61,11 @@ namespace qpdf
         // The rest of the header file is for qpdf internal use only.
+        // For arrays, return the number of items in the array.
+        // For null-like objects, return 0.
+        // For all other objects, return 1.
+        size_t size() const;
+
         std::shared_ptr<QPDFObject> copy(bool shallow = false) const;
         // Recursively remove association with any QPDF object. This method may only be called
         // during final destruction.
@@ -352,16 +352,15 @@ BaseHandle::unparse() const
             auto const& a = std::get<QPDF_Array>(obj->value);
             std::string result = "[ ";
             if (a.sp) {
-                int next = 0;
-                for (auto& item: a.sp->elements) {
-                    int key = item.first;
-                    for (int j = next; j < key; ++j) {
+                size_t next = 0;
+                for (auto& [key, value]: a.sp->elements) {
+                    for (size_t j = next; j < key; ++j) {
                         result += "null ";
                     }
-                    result += item.second.unparse() + " ";
-                    next = ++key;
+                    result += value.unparse() + " ";
+                    next = key + 1;
                 }
-                for (int j = next; j < a.sp->size; ++j) {
+                for (size_t j = next; j < a.sp->size; ++j) {
                     result += "null ";
                 }
             } else {
@@ -467,22 +466,21 @@ BaseHandle::write_json(int json_version, JSON::Writer&amp; p) const
             auto const& a = std::get<QPDF_Array>(obj->value);
             p.writeStart('[');
             if (a.sp) {
-                int next = 0;
-                for (auto& item: a.sp->elements) {
-                    int key = item.first;
-                    for (int j = next; j < key; ++j) {
+                size_t next = 0;
+                for (auto& [key, value]: a.sp->elements) {
+                    for (size_t j = next; j < key; ++j) {
                         p.writeNext() << "null";
                     }
                     p.writeNext();
-                    auto item_og = item.second.getObj()->getObjGen();
+                    auto item_og = value.getObj()->getObjGen();
                     if (item_og.isIndirect()) {
                         p << "\"" << item_og.unparse(' ') << " R\"";
                     } else {
-                        item.second.write_json(json_version, p);
+                        value.write_json(json_version, p);
                     }
-                    next = ++key;
+                    next = key + 1;
                 }
-                for (int j = next; j < a.sp->size; ++j) {
+                for (size_t j = next; j < a.sp->size; ++j) {
                     p.writeNext() << "null";
                 }
             } else {
@@ -1234,7 +1232,7 @@ QPDFObjectHandle::arrayOrStreamToStreamArray(
     all_description = description;
     std::vector<QPDFObjectHandle> result;
     if (auto array = as_array(strict)) {
-        int n_items = array.size();
+        int n_items = static_cast<int>(array.size());
         for (int i = 0; i < n_items; ++i) {
             QPDFObjectHandle item = array.at(i).second;
             if (item.isStream()) {
@@ -2,11 +2,25 @@
 #include <qpdf/QTC.hh>
+#include <utility>
+
 using namespace std::literals;
 using namespace qpdf;
 static const QPDFObjectHandle null_oh = QPDFObjectHandle::newNull();
+static size_t
+to_s(int n)
+{
+    return static_cast<size_t>(n);
+}
+
+static int
+to_i(size_t n)
+{
+    return static_cast<int>(n);
+}
+
 inline void
 Array::checkOwnership(QPDFObjectHandle const& item) const
 {
@@ -142,24 +156,24 @@ Array::null() const
     return null_oh;
 }
-int
+size_t
 Array::size() const
 {
     auto a = array();
-    return a->sp ? a->sp->size : int(a->elements.size());
+    return a->sp ? a->sp->size : a->elements.size();
 }
 std::pair<bool, QPDFObjectHandle>
 Array::at(int n) const
 {
     auto a = array();
-    if (n < 0 || n >= size()) {
+    if (n < 0 || std::cmp_greater_equal(n, size())) {
         return {false, {}};
     }
     if (!a->sp) {
-        return {true, a->elements[size_t(n)]};
+        return {true, a->elements[to_s(n)]};
     }
-    auto const& iter = a->sp->elements.find(n);
+    auto const& iter = a->sp->elements.find(to_s(n));
     return {true, iter == a->sp->elements.end() ? null() : iter->second};
 }
@@ -169,12 +183,12 @@ Array::getAsVector() const
     auto a = array();
     if (a->sp) {
         std::vector<QPDFObjectHandle> v;
-        v.reserve(size_t(size()));
+        v.reserve(size());
         for (auto const& item: a->sp->elements) {
-            v.resize(size_t(item.first), null_oh);
+            v.resize(item.first, null_oh);
             v.emplace_back(item.second);
         }
-        v.resize(size_t(size()), null_oh);
+        v.resize(size(), null_oh);
         return v;
     } else {
         return a->elements;
@@ -184,15 +198,15 @@ Array::getAsVector() const
 bool
 Array::setAt(int at, QPDFObjectHandle const& oh)
 {
-    if (at < 0 || at >= size()) {
+    if (at < 0 || std::cmp_greater_equal(at, size())) {
         return false;
     }
     auto a = array();
     checkOwnership(oh);
     if (a->sp) {
-        a->sp->elements[at] = oh;
+        a->sp->elements[to_s(at)] = oh;
     } else {
-        a->elements[size_t(at)] = oh;
+        a->elements[to_s(at)] = oh;
     }
     return true;
 }
@@ -210,34 +224,39 @@ Array::setFromVector(std::vector&lt;QPDFObjectHandle&gt; const&amp; v)
 }
 bool
-Array::insert(int at, QPDFObjectHandle const& item)
+Array::insert(int at_i, QPDFObjectHandle const& item)
 {
     auto a = array();
-    int sz = size();
-    if (at < 0 || at > sz) {
-        // As special case, also allow insert beyond the end
+    size_t sz = size();
+    if (at_i < 0) {
+        return false;
+    }
+    size_t at = to_s(at_i);
+    if (at > sz) {
         return false;
-    } else if (at == sz) {
+    }
+    if (at == sz) {
+        // As special case, also allow insert beyond the end
         push_back(item);
-    } else {
-        checkOwnership(item);
-        if (a->sp) {
-            auto iter = a->sp->elements.crbegin();
-            while (iter != a->sp->elements.crend()) {
-                auto key = (iter++)->first;
-                if (key >= at) {
-                    auto nh = a->sp->elements.extract(key);
-                    ++nh.key();
-                    a->sp->elements.insert(std::move(nh));
-                } else {
-                    break;
-                }
+        return true;
+    }
+    checkOwnership(item);
+    if (a->sp) {
+        auto iter = a->sp->elements.crbegin();
+        while (iter != a->sp->elements.crend()) {
+            auto key = (iter++)->first;
+            if (key >= at) {
+                auto nh = a->sp->elements.extract(key);
+                ++nh.key();
+                a->sp->elements.insert(std::move(nh));
+            } else {
+                break;
             }
-            a->sp->elements[at] = item.getObj();
-            ++a->sp->size;
-        } else {
-            a->elements.insert(a->elements.cbegin() + at, item.getObj());
         }
+        a->sp->elements[at] = item.getObj();
+        ++a->sp->size;
+    } else {
+        a->elements.insert(a->elements.cbegin() + at_i, item.getObj());
     }
     return true;
 }
@@ -255,10 +274,14 @@ Array::push_back(QPDFObjectHandle const&amp; item)
 }
 bool
-Array::erase(int at)
+Array::erase(int at_i)
 {
     auto a = array();
-    if (at < 0 || at >= size()) {
+    if (at_i < 0) {
+        return false;
+    }
+    size_t at = to_s(at_i);
+    if (at >= size()) {
         return false;
     }
     if (a->sp) {
@@ -277,7 +300,7 @@ Array::erase(int at)
         }
         --(a->sp->size);
     } else {
-        a->elements.erase(a->elements.cbegin() + at);
+        a->elements.erase(a->elements.cbegin() + at_i);
     }
     return true;
 }
@@ -286,7 +309,7 @@ int
 QPDFObjectHandle::getArrayNItems() const
 {
     if (auto array = as_array(strict)) {
-        return array.size();
+        return to_i(array.size());
     }
     typeWarning("array", "treating as empty");
     QTC::TC("qpdf", "QPDFObjectHandle array treating as empty");
@@ -471,7 +494,37 @@ QPDFObjectHandle
 QPDFObjectHandle::eraseItemAndGetOld(int at)
 {
     auto array = as_array(strict);
-    auto result = (array && at < array.size() && at >= 0) ? array.at(at).second : newNull();
+    auto result =
+        (array && std::cmp_less(at, array.size()) && at >= 0) ? array.at(at).second : newNull();
     eraseItem(at);
     return result;
 }
+
+size_t
+BaseHandle::size() const
+{
+    switch (resolved_type_code()) {
+    case ::ot_array:
+        return as<QPDF_Array>()->size();
+    case ::ot_uninitialized:
+    case ::ot_reserved:
+    case ::ot_null:
+    case ::ot_destroyed:
+    case ::ot_unresolved:
+    case ::ot_reference:
+        return 0;
+    case ::ot_boolean:
+    case ::ot_integer:
+    case ::ot_real:
+    case ::ot_string:
+    case ::ot_name:
+    case ::ot_dictionary:
+    case ::ot_stream:
+    case ::ot_inlineimage:
+    case ::ot_operator:
+        return 1;
+    default:
+        throw std::logic_error("Unexpected type code in size"); // unreachable
+        return 0;                                               // unreachable
+    }
+}
@@ -38,7 +38,7 @@ namespace qpdf
         const_reverse_iterator crend();
-        int size() const;
+        size_t size() const;
         std::pair<bool, QPDFObjectHandle> at(int n) const;
         bool setAt(int at, QPDFObjectHandle const& oh);
         bool insert(int at, QPDFObjectHandle const& item);
@@ -35,8 +35,8 @@ class QPDF_Array final
   private:
     struct Sparse
     {
-        int size{0};
-        std::map<int, QPDFObjectHandle> elements;
+        size_t size{0};
+        std::map<size_t, QPDFObjectHandle> elements;
     };
   public:
@@ -65,10 +65,10 @@ class QPDF_Array final
     {
     }
-    int
+    size_t
     size() const
     {
-        return sp ? sp->size : int(elements.size());
+        return sp ? sp->size : elements.size();
     }
     std::unique_ptr<Sparse> sp;
@@ -7,6 +7,12 @@
 #include <iostream>
 int
+to_i(size_t n)
+{
+    return static_cast<int>(n);
+}
+
+int
 main()
 {
     auto obj = QPDFObject::create<QPDF_Array>(std::vector<QPDFObjectHandle>(), true);
@@ -65,20 +71,20 @@ main()
     a.setAt(4, QPDFObjectHandle::newNull());
     assert(a.at(4).second.isNull());
-    a.erase(a.size() - 1);
+    a.erase(to_i(a.size()) - 1);
     assert(a.size() == 5);
     assert(a.at(0).second.isName() && (a.at(0).second.getName() == "/First"));
     assert(a.at(1).second.isInteger() && (a.at(1).second.getIntValue() == 1));
     assert(a.at(3).second.isName() && (a.at(3).second.getName() == "/Third"));
     assert(a.at(4).second.isNull());
-    a.erase(a.size() - 1);
+    a.erase(to_i(a.size()) - 1);
     assert(a.size() == 4);
     assert(a.at(0).second.isName() && (a.at(0).second.getName() == "/First"));
     assert(a.at(1).second.isInteger() && (a.at(1).second.getIntValue() == 1));
     assert(a.at(3).second.isName() && (a.at(3).second.getName() == "/Third"));
-    a.erase(a.size() - 1);
+    a.erase(to_i(a.size()) - 1);
     assert(a.size() == 3);
     assert(a.at(0).second.isName() && (a.at(0).second.getName() == "/First"));
     assert(a.at(1).second.isInteger() && (a.at(1).second.getIntValue() == 1));