Change Array to use std::vector<QPDFObjectHandle> for storage

m-holger
1 parent 8d9b9a1a
Showing 6 changed files with 48 additions and 69 deletions
libqpdf/QPDFObjectHandle.cc
libqpdf/QPDFParser.cc
libqpdf/QPDF_Array.cc
libqpdf/qpdf/QPDFObject_private.hh
libqpdf/qpdf/QPDFParser.hh
libtests/sparse_array.cc
@@ -322,17 +322,18 @@ QPDFObject::copy(bool shallow)
                     for (auto const& element: a.sp->elements) {
                         auto const& obj = element.second;
                         result.sp->elements[element.first] =
-                            obj->getObjGen().isIndirect() ? obj : obj->copy();
+                            obj.getObj()->getObjGen().isIndirect() ? obj : obj.getObj()->copy();
                     }
                     return QPDFObject::create<QPDF_Array>(std::move(result));
                 } else {
-                    std::vector<std::shared_ptr<QPDFObject>> result;
+                    std::vector<QPDFObjectHandle> result;
                     result.reserve(a.elements.size());
                     for (auto const& element: a.elements) {
                         result.push_back(
-                            element
-                                ? (element->getObjGen().isIndirect() ? element : element->copy())
-                                : element);
+                            element ? (element.getObj()->getObjGen().isIndirect()
+                                           ? element
+                                           : element.getObj()->copy())
+                                    : element);
                     }
                     return QPDFObject::create<QPDF_Array>(std::move(result), false);
                 }
@@ -404,9 +405,9 @@ QPDFObject::unparse()
                     for (int j = next; j < key; ++j) {
                         result += "null ";
                     }
-                    auto item_og = item.second->resolved_object()->getObjGen();
+                    auto item_og = item.second.getObj()->resolved_object()->getObjGen();
                     result += item_og.isIndirect() ? item_og.unparse(' ') + " R "
-                                                   : item.second->unparse() + " ";
+                                                   : item.second.getObj()->unparse() + " ";
                     next = ++key;
                 }
                 for (int j = next; j < a.sp->size; ++j) {
@@ -414,9 +415,9 @@ QPDFObject::unparse()
                 }
             } else {
                 for (auto const& item: a.elements) {
-                    auto item_og = item->resolved_object()->getObjGen();
-                    result +=
-                        item_og.isIndirect() ? item_og.unparse(' ') + " R " : item->unparse() + " ";
+                    auto item_og = item.getObj()->resolved_object()->getObjGen();
+                    result += item_og.isIndirect() ? item_og.unparse(' ') + " R "
+                                                   : item.getObj()->unparse() + " ";
                 }
             }
             result += "]";
@@ -524,11 +525,11 @@ QPDFObject::write_json(int json_version, JSON::Writer&amp; p)
                         p.writeNext() << "null";
                     }
                     p.writeNext();
-                    auto item_og = item.second->getObjGen();
+                    auto item_og = item.second.getObj()->getObjGen();
                     if (item_og.isIndirect()) {
                         p << "\"" << item_og.unparse(' ') << " R\"";
                     } else {
-                        item.second->write_json(json_version, p);
+                        item.second.getObj()->write_json(json_version, p);
                     }
                     next = ++key;
                 }
@@ -538,11 +539,11 @@ QPDFObject::write_json(int json_version, JSON::Writer&amp; p)
             } else {
                 for (auto const& item: a.elements) {
                     p.writeNext();
-                    auto item_og = item->getObjGen();
+                    auto item_og = item.getObj()->getObjGen();
                     if (item_og.isIndirect()) {
                         p << "\"" << item_og.unparse(' ') << " R\"";
                     } else {
-                        item->write_json(json_version, p);
+                        item.getObj()->write_json(json_version, p);
                     }
                 }
             }
@@ -598,14 +599,14 @@ QPDFObject::disconnect()
             if (a.sp) {
                 for (auto& item: a.sp->elements) {
                     auto& obj = item.second;
-                    if (!obj->getObjGen().isIndirect()) {
-                        obj->disconnect();
+                    if (!obj.getObj()->getObjGen().isIndirect()) {
+                        obj.getObj()->disconnect();
                     }
                 }
             } else {
                 for (auto& obj: a.elements) {
-                    if (!obj->getObjGen().isIndirect()) {
-                        obj->disconnect();
+                    if (!obj.getObj()->getObjGen().isIndirect()) {
+                        obj.getObj()->disconnect();
                     }
                 }
             }
@@ -209,8 +209,9 @@ QPDFParser::parseRemainder(bool content_stream)
                 return {QPDFObject::create<QPDF_Null>()};
             }
             if (frame->state == st_array) {
-                auto object = QPDFObject::create<QPDF_Array>(
-                    std::move(frame->olist), frame->null_count > 100);
+                auto object = frame->null_count > 100
+                    ? QPDFObject::create<QPDF_Array>(std::move(frame->olist), true)
+                    : QPDFObject::create<QPDF_Array>(std::move(frame->olist));
                 setDescription(object, frame->offset - 1);
                 // The `offset` points to the next of "[".  Set the rewind offset to point to the
                 // beginning of "[". This has been explicitly tested with whitespace surrounding the
@@ -451,8 +452,8 @@ QPDFParser::fixMissingKeys()
 {
     std::set<std::string> names;
     for (auto& obj: frame->olist) {
-        if (obj->getTypeCode() == ::ot_name) {
-            names.insert(obj->getStringValue());
+        if (obj.getObj()->getTypeCode() == ::ot_name) {
+            names.insert(obj.getObj()->getStringValue());
         }
     }
     int next_fake_key = 1;
@@ -8,17 +8,6 @@ using namespace qpdf;
 static const QPDFObjectHandle null_oh = QPDFObjectHandle::newNull();
 inline void
-QPDF_Array::checkOwnership(QPDFObjectHandle const& item) const
-{
-    // This is only called from QPDF_Array::setFromVector, which in turn is only called from create.
-    // At his point qpdf is a nullptr and therefore the ownership check reduces to an uninitialized
-    // check
-    if (!item.getObjectPtr()) {
-        throw std::logic_error("Attempting to add an uninitialized object to a QPDF_Array.");
-    }
-}
-
-inline void
 Array::checkOwnership(QPDFObjectHandle const& item) const
 {
     if (auto o = item.getObjectPtr()) {
@@ -36,17 +25,13 @@ Array::checkOwnership(QPDFObjectHandle const&amp; item) const
     }
 }
-QPDF_Array::QPDF_Array(std::vector<QPDFObjectHandle> const& v)
-{
-    setFromVector(v);
-}
-
-QPDF_Array::QPDF_Array(std::vector<std::shared_ptr<QPDFObject>>&& v, bool sparse)
+QPDF_Array::QPDF_Array(std::vector<QPDFObjectHandle>&& v, bool sparse)
 {
     if (sparse) {
         sp = std::make_unique<Sparse>();
         for (auto&& item: v) {
-            if (item->getTypeCode() != ::ot_null || item->getObjGen().isIndirect()) {
+            if (item.getObj()->getTypeCode() != ::ot_null ||
+                item.getObj()->getObjGen().isIndirect()) {
                 sp->elements[sp->size] = std::move(item);
             }
             ++sp->size;
@@ -108,7 +93,7 @@ Array::getAsVector() const
         v.resize(size_t(size()), null_oh);
         return v;
     } else {
-        return {a->elements.cbegin(), a->elements.cend()};
+        return a->elements;
     }
 }
@@ -121,25 +106,14 @@ Array::setAt(int at, QPDFObjectHandle const&amp; oh)
     auto a = array();
     checkOwnership(oh);
     if (a->sp) {
-        a->sp->elements[at] = oh.getObj();
+        a->sp->elements[at] = oh;
     } else {
-        a->elements[size_t(at)] = oh.getObj();
+        a->elements[size_t(at)] = oh;
     }
     return true;
 }
 void
-QPDF_Array::setFromVector(std::vector<QPDFObjectHandle> const& v)
-{
-    elements.resize(0);
-    elements.reserve(v.size());
-    for (auto const& item: v) {
-        checkOwnership(item);
-        elements.push_back(item.getObj());
-    }
-}
-
-void
 Array::setFromVector(std::vector<QPDFObjectHandle> const& v)
 {
     auto a = array();
@@ -147,7 +121,7 @@ Array::setFromVector(std::vector&lt;QPDFObjectHandle&gt; const&amp; v)
     a->elements.reserve(v.size());
     for (auto const& item: v) {
         checkOwnership(item);
-        a->elements.push_back(item.getObj());
+        a->elements.emplace_back(item);
     }
 }
@@ -190,9 +164,9 @@ Array::push_back(QPDFObjectHandle const&amp; item)
     auto a = array();
     checkOwnership(item);
     if (a->sp) {
-        a->sp->elements[(a->sp->size)++] = item.getObj();
+        a->sp->elements[(a->sp->size)++] = item;
     } else {
-        a->elements.push_back(item.getObj());
+        a->elements.emplace_back(item);
     }
 }
@@ -35,7 +35,7 @@ class QPDF_Array final
     struct Sparse
     {
         int size{0};
-        std::map<int, std::shared_ptr<QPDFObject>> elements;
+        std::map<int, QPDFObjectHandle> elements;
     };
   public:
@@ -51,19 +51,25 @@ class QPDF_Array final
   private:
     friend class QPDFObject;
     friend class qpdf::Array;
-    QPDF_Array(std::vector<QPDFObjectHandle> const& items);
-    QPDF_Array(std::vector<std::shared_ptr<QPDFObject>>&& items, bool sparse);
+    QPDF_Array(std::vector<QPDFObjectHandle> const& items) :
+        elements(items)
+    {
+    }
+    QPDF_Array(std::vector<QPDFObjectHandle>&& items, bool sparse);
+
+    QPDF_Array(std::vector<QPDFObjectHandle>&& items) :
+        elements(std::move(items))
+    {
+    }
     int
     size() const
     {
         return sp ? sp->size : int(elements.size());
     }
-    void setFromVector(std::vector<QPDFObjectHandle> const& items);
-    void checkOwnership(QPDFObjectHandle const& item) const;
     std::unique_ptr<Sparse> sp;
-    std::vector<std::shared_ptr<QPDFObject>> elements;
+    std::vector<QPDFObjectHandle> elements;
 };
 class QPDF_Bool final
@@ -202,9 +208,6 @@ class QPDF_Stream final
         }
       private:
-        void replaceFilterData(
-            QPDFObjectHandle const& filter, QPDFObjectHandle const& decode_parms, size_t length);
-
         bool filter_on_write{true};
         QPDFObjectHandle stream_dict;
         size_t length{0};
@@ -46,7 +46,7 @@ class QPDFParser
         {
         }
-        std::vector<std::shared_ptr<QPDFObject>> olist;
+        std::vector<QPDFObjectHandle> olist;
         std::map<std::string, QPDFObjectHandle> dict;
         parser_state_e state;
         std::string key;
@@ -9,7 +9,7 @@
 int
 main()
 {
-    auto obj = QPDFObject::create<QPDF_Array>(std::vector<std::shared_ptr<QPDFObject>>(), true);
+    auto obj = QPDFObject::create<QPDF_Array>(std::vector<QPDFObjectHandle>(), true);
     auto a = qpdf::Array(obj);
     assert(a.size() == 0);
@@ -88,7 +88,7 @@ main()
     pdf.emptyPDF();
     obj = QPDFObject::create<QPDF_Array>(
-        std::vector<std::shared_ptr<QPDFObject>>{10, "null"_qpdf.getObj()}, true);
+        std::vector<QPDFObjectHandle>{10, "null"_qpdf.getObj()}, true);
     auto b = qpdf::Array(obj);
     b.setAt(5, pdf.newIndirectNull());
     b.setAt(7, "[0 1 2 3]"_qpdf);