From de29fd56c487dcf08f2af66f47576f147b20be82 Mon Sep 17 00:00:00 2001 From: m-holger Date: Sat, 25 Mar 2023 14:06:57 +0000 Subject: Remove redundant QPDF_Array::addExplicitElementsToList --- libqpdf/QPDF_Array.cc | 8 -------- libqpdf/qpdf/QPDF_Array.hh | 5 ----- 2 files changed, 13 deletions(-) diff --git a/libqpdf/QPDF_Array.cc b/libqpdf/QPDF_Array.cc index de34103e..93fbf928 100644 --- a/libqpdf/QPDF_Array.cc +++ b/libqpdf/QPDF_Array.cc @@ -154,11 +154,3 @@ QPDF_Array::eraseItem(int at) { this->elements.erase(QIntC::to_size(at)); } - -void -QPDF_Array::addExplicitElementsToList(std::list& l) const -{ - for (auto const& iter: this->elements) { - l.push_back(iter.second); - } -} diff --git a/libqpdf/qpdf/QPDF_Array.hh b/libqpdf/qpdf/QPDF_Array.hh index 56c0101f..88397ba7 100644 --- a/libqpdf/qpdf/QPDF_Array.hh +++ b/libqpdf/qpdf/QPDF_Array.hh @@ -32,11 +32,6 @@ class QPDF_Array: public QPDFValue void appendItem(QPDFObjectHandle const& item); void eraseItem(int at); - // Helper methods for QPDF and QPDFObjectHandle -- these are - // public methods since the whole class is not part of the public - // API. Otherwise, these would be wrapped in accessor classes. - void addExplicitElementsToList(std::list&) const; - private: QPDF_Array(std::vector const& items); QPDF_Array(std::vector>&& items); -- cgit v1.2.3-54-g00ecf From 38cf7c16283ec2d476514a54e2b1a7016b0b770a Mon Sep 17 00:00:00 2001 From: m-holger Date: Sat, 25 Mar 2023 15:30:52 +0000 Subject: Add separate sparse mode to QPDF_Array Add temporary clone of SparseOHArray to implement non-sparse mode. --- include/qpdf/QPDFObjectHandle.hh | 1 + libqpdf/CMakeLists.txt | 1 + libqpdf/OHArray.cc | 148 ++++++++++++++++++++++++++++++++ libqpdf/QPDF_Array.cc | 181 ++++++++++++++++++++++++++++++--------- libqpdf/qpdf/OHArray.hh | 35 ++++++++ libqpdf/qpdf/QPDF_Array.hh | 7 +- 6 files changed, 331 insertions(+), 42 deletions(-) create mode 100644 libqpdf/OHArray.cc create mode 100644 libqpdf/qpdf/OHArray.hh diff --git a/include/qpdf/QPDFObjectHandle.hh b/include/qpdf/QPDFObjectHandle.hh index 77fe680f..b722b083 100644 --- a/include/qpdf/QPDFObjectHandle.hh +++ b/include/qpdf/QPDFObjectHandle.hh @@ -1497,6 +1497,7 @@ class QPDFObjectHandle friend class QPDF_Dictionary; friend class QPDF_Stream; friend class SparseOHArray; + friend class OHArray; private: static void diff --git a/libqpdf/CMakeLists.txt b/libqpdf/CMakeLists.txt index 5e3a628e..2d857f45 100644 --- a/libqpdf/CMakeLists.txt +++ b/libqpdf/CMakeLists.txt @@ -116,6 +116,7 @@ set(libqpdf_SOURCES SecureRandomDataProvider.cc SF_FlateLzwDecode.cc SparseOHArray.cc + OHArray.cc qpdf-c.cc qpdfjob-c.cc qpdflogger-c.cc) diff --git a/libqpdf/OHArray.cc b/libqpdf/OHArray.cc new file mode 100644 index 00000000..436fa4f2 --- /dev/null +++ b/libqpdf/OHArray.cc @@ -0,0 +1,148 @@ +#include + +#include +#include + +#include + +OHArray::OHArray() : + n_elements(0) +{ +} + +size_t +OHArray::size() const +{ + return this->n_elements; +} + +void +OHArray::append(QPDFObjectHandle oh) +{ + if (!oh.isDirectNull()) { + this->elements[this->n_elements] = oh; + } + ++this->n_elements; +} + +void +OHArray::append(std::shared_ptr&& obj) +{ + if (obj->getTypeCode() != ::ot_null || !obj->getObjGen().isIndirect()) { + this->elements[this->n_elements] = std::move(obj); + } + ++this->n_elements; +} + +QPDFObjectHandle +OHArray::at(size_t idx) const +{ + if (idx >= this->n_elements) { + throw std::logic_error( + "INTERNAL ERROR: bounds error accessing OHArray element"); + } + auto const& iter = this->elements.find(idx); + if (iter == this->elements.end()) { + 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) { + QPDFObjectHandle::DisconnectAccess::disconnect(iter.second); + } +} + +void +OHArray::setAt(size_t idx, QPDFObjectHandle oh) +{ + if (idx >= this->n_elements) { + throw std::logic_error("bounds error setting item in OHArray"); + } + if (oh.isDirectNull()) { + this->elements.erase(idx); + } else { + this->elements[idx] = oh; + } +} + +void +OHArray::erase(size_t idx) +{ + if (idx >= this->n_elements) { + throw std::logic_error("bounds error erasing item from OHArray"); + } + 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; + } + } + this->elements = dest; + --this->n_elements; +} + +void +OHArray::insert(size_t idx, QPDFObjectHandle oh) +{ + if (idx > this->n_elements) { + throw std::logic_error("bounds error inserting item to OHArray"); + } else if (idx == this->n_elements) { + // Allow inserting to the last position + append(oh); + } else { + decltype(this->elements) dest; + for (auto const& iter: this->elements) { + if (iter.first < idx) { + dest.insert(iter); + } else { + dest[iter.first + 1] = iter.second; + } + } + this->elements = dest; + this->elements[idx] = oh; + ++this->n_elements; + } +} + +OHArray +OHArray::copy() +{ + OHArray result; + result.n_elements = this->n_elements; + for (auto const& element: this->elements) { + auto value = element.second; + result.elements[element.first] = + value.isIndirect() ? value : value.shallowCopy(); + } + return result; +} + +OHArray::const_iterator +OHArray::begin() const +{ + return this->elements.begin(); +} + +OHArray::const_iterator +OHArray::end() const +{ + return this->elements.end(); +} diff --git a/libqpdf/QPDF_Array.cc b/libqpdf/QPDF_Array.cc index 93fbf928..ed43245f 100644 --- a/libqpdf/QPDF_Array.cc +++ b/libqpdf/QPDF_Array.cc @@ -19,6 +19,13 @@ QPDF_Array::QPDF_Array(std::vector>&& v) : QPDF_Array::QPDF_Array(SparseOHArray const& items) : QPDFValue(::ot_array, "array"), + sp_elements(items) +{ +} + +QPDF_Array::QPDF_Array(OHArray const& items) : + QPDFValue(::ot_array, "array"), + sparse(false), elements(items) { } @@ -41,93 +48,168 @@ QPDF_Array::create(SparseOHArray const& items) return do_create(new QPDF_Array(items)); } +std::shared_ptr +QPDF_Array::create(OHArray const& items) +{ + return do_create(new QPDF_Array(items)); +} + std::shared_ptr QPDF_Array::copy(bool shallow) { - return create(shallow ? elements : elements.copy()); + if (sparse) { + return create(shallow ? sp_elements : sp_elements.copy()); + } else { + return create(shallow ? elements : elements.copy()); + } } void QPDF_Array::disconnect() { - elements.disconnect(); + if (sparse) { + sp_elements.disconnect(); + } else { + elements.disconnect(); + } } std::string QPDF_Array::unparse() { - std::string result = "[ "; - size_t size = this->elements.size(); - for (size_t i = 0; i < size; ++i) { - result += this->elements.at(i).unparse(); - result += " "; + if (sparse) { + std::string result = "[ "; + size_t size = sp_elements.size(); + for (size_t i = 0; i < size; ++i) { + result += sp_elements.at(i).unparse(); + result += " "; + } + result += "]"; + return result; + } else { + std::string result = "[ "; + size_t size = elements.size(); + for (size_t i = 0; i < size; ++i) { + result += elements.at(i).unparse(); + result += " "; + } + result += "]"; + return result; } - result += "]"; - return result; } JSON QPDF_Array::getJSON(int json_version) { - JSON j = JSON::makeArray(); - size_t size = this->elements.size(); - for (size_t i = 0; i < size; ++i) { - j.addArrayElement(this->elements.at(i).getJSON(json_version)); + if (sparse) { + JSON j = JSON::makeArray(); + size_t size = sp_elements.size(); + for (size_t i = 0; i < size; ++i) { + j.addArrayElement(sp_elements.at(i).getJSON(json_version)); + } + return j; + } else { + JSON j = JSON::makeArray(); + size_t size = elements.size(); + for (size_t i = 0; i < size; ++i) { + j.addArrayElement(elements.at(i).getJSON(json_version)); + } + return j; } - return j; } int QPDF_Array::getNItems() const { - // This should really return a size_t, but changing it would break - // a lot of code. - return QIntC::to_int(this->elements.size()); + if (sparse) { + // This should really return a size_t, but changing it would break + // a lot of code. + return QIntC::to_int(sp_elements.size()); + } else { + return QIntC::to_int(elements.size()); + } } QPDFObjectHandle QPDF_Array::getItem(int n) const { - if ((n < 0) || (n >= QIntC::to_int(elements.size()))) { - throw std::logic_error( - "INTERNAL ERROR: bounds error accessing QPDF_Array element"); + if (sparse) { + if ((n < 0) || (n >= QIntC::to_int(sp_elements.size()))) { + throw std::logic_error( + "INTERNAL ERROR: bounds error accessing QPDF_Array element"); + } + return sp_elements.at(QIntC::to_size(n)); + } else { + if ((n < 0) || (n >= QIntC::to_int(elements.size()))) { + throw std::logic_error( + "INTERNAL ERROR: bounds error accessing QPDF_Array element"); + } + return elements.at(QIntC::to_size(n)); } - return this->elements.at(QIntC::to_size(n)); } void QPDF_Array::getAsVector(std::vector& v) const { - size_t size = this->elements.size(); - for (size_t i = 0; i < size; ++i) { - v.push_back(this->elements.at(i)); + if (sparse) { + size_t size = sp_elements.size(); + for (size_t i = 0; i < size; ++i) { + v.push_back(sp_elements.at(i)); + } + } else { + size_t size = elements.size(); + for (size_t i = 0; i < size; ++i) { + v.push_back(elements.at(i)); + } } } void QPDF_Array::setItem(int n, QPDFObjectHandle const& oh) { - this->elements.setAt(QIntC::to_size(n), oh); + if (sparse) { + sp_elements.setAt(QIntC::to_size(n), oh); + } else { + elements.setAt(QIntC::to_size(n), oh); + } } void QPDF_Array::setFromVector(std::vector const& v) { - this->elements = SparseOHArray(); - for (auto const& iter: v) { - this->elements.append(iter); + if (sparse) { + sp_elements = SparseOHArray(); + for (auto const& iter: v) { + sp_elements.append(iter); + } + } else { + elements = OHArray(); + for (auto const& iter: v) { + elements.append(iter); + } } } void QPDF_Array::setFromVector(std::vector>&& v) { - this->elements = SparseOHArray(); - for (auto&& item: v) { - if (item) { - this->elements.append(item); - } else { - ++this->elements.n_elements; + if (sparse) { + sp_elements = SparseOHArray(); + for (auto&& item: v) { + if (item) { + sp_elements.append(item); + } else { + ++sp_elements.n_elements; + } + } + } else { + elements = OHArray(); + for (auto&& item: v) { + if (item) { + elements.append(item); + } else { + ++elements.n_elements; + } } } } @@ -135,22 +217,39 @@ QPDF_Array::setFromVector(std::vector>&& v) void QPDF_Array::insertItem(int at, QPDFObjectHandle const& item) { - // As special case, also allow insert beyond the end - if ((at < 0) || (at > QIntC::to_int(this->elements.size()))) { - throw std::logic_error( - "INTERNAL ERROR: bounds error accessing QPDF_Array element"); + if (sparse) { + // As special case, also allow insert beyond the end + if ((at < 0) || (at > QIntC::to_int(sp_elements.size()))) { + throw std::logic_error( + "INTERNAL ERROR: bounds error accessing QPDF_Array element"); + } + sp_elements.insert(QIntC::to_size(at), item); + } else { + // As special case, also allow insert beyond the end + if ((at < 0) || (at > QIntC::to_int(elements.size()))) { + throw std::logic_error( + "INTERNAL ERROR: bounds error accessing QPDF_Array element"); + } + elements.insert(QIntC::to_size(at), item); } - this->elements.insert(QIntC::to_size(at), item); } void QPDF_Array::appendItem(QPDFObjectHandle const& item) { - this->elements.append(item); + if (sparse) { + sp_elements.append(item); + } else { + elements.append(item); + } } void QPDF_Array::eraseItem(int at) { - this->elements.erase(QIntC::to_size(at)); + if (sparse) { + sp_elements.erase(QIntC::to_size(at)); + } else { + elements.erase(QIntC::to_size(at)); + } } diff --git a/libqpdf/qpdf/OHArray.hh b/libqpdf/qpdf/OHArray.hh new file mode 100644 index 00000000..66223c4f --- /dev/null +++ b/libqpdf/qpdf/OHArray.hh @@ -0,0 +1,35 @@ +#ifndef QPDF_OHARRAY_HH +#define QPDF_OHARRAY_HH + +#include +#include + +class QPDF_Array; + +class OHArray +{ + public: + OHArray(); + size_t size() const; + void append(QPDFObjectHandle oh); + void append(std::shared_ptr&& 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::const_iterator + const_iterator; + const_iterator begin() const; + const_iterator end() const; + + private: + friend class QPDF_Array; + std::unordered_map elements; + size_t n_elements; +}; + +#endif // QPDF_OHARRAY_HH diff --git a/libqpdf/qpdf/QPDF_Array.hh b/libqpdf/qpdf/QPDF_Array.hh index 88397ba7..1c4227ba 100644 --- a/libqpdf/qpdf/QPDF_Array.hh +++ b/libqpdf/qpdf/QPDF_Array.hh @@ -3,6 +3,7 @@ #include +#include #include #include #include @@ -16,6 +17,7 @@ class QPDF_Array: public QPDFValue static std::shared_ptr create(std::vector>&& items); static std::shared_ptr create(SparseOHArray const& items); + static std::shared_ptr create(OHArray const& items); virtual std::shared_ptr copy(bool shallow = false); virtual std::string unparse(); virtual JSON getJSON(int json_version); @@ -36,7 +38,10 @@ class QPDF_Array: public QPDFValue QPDF_Array(std::vector const& items); QPDF_Array(std::vector>&& items); QPDF_Array(SparseOHArray const& items); - SparseOHArray elements; + QPDF_Array(OHArray const& items); + bool sparse{false}; + SparseOHArray sp_elements; + OHArray elements; }; #endif // QPDF_ARRAY_HH -- cgit v1.2.3-54-g00ecf From 18c1ffe0df335a46cddbeb96e2cb939d850df9fa Mon Sep 17 00:00:00 2001 From: m-holger Date: Sat, 25 Mar 2023 12:07:04 +0000 Subject: Change underlying data structure of QPDF_Array in non-sparse mode to std::vector --- libqpdf/OHArray.cc | 109 +++++++++++++----------------------------------- libqpdf/QPDF_Array.cc | 6 +-- libqpdf/qpdf/OHArray.hh | 13 ++---- 3 files changed, 35 insertions(+), 93 deletions(-) diff --git a/libqpdf/OHArray.cc b/libqpdf/OHArray.cc index 436fa4f2..e25e1239 100644 --- a/libqpdf/OHArray.cc +++ b/libqpdf/OHArray.cc @@ -5,120 +5,82 @@ #include -OHArray::OHArray() : - n_elements(0) +static const QPDFObjectHandle null_oh = QPDFObjectHandle::newNull(); + +OHArray::OHArray() { } size_t OHArray::size() const { - return this->n_elements; + return elements.size(); } void OHArray::append(QPDFObjectHandle oh) { - if (!oh.isDirectNull()) { - this->elements[this->n_elements] = oh; - } - ++this->n_elements; + elements.push_back(oh.getObj()); } void OHArray::append(std::shared_ptr&& obj) { - if (obj->getTypeCode() != ::ot_null || !obj->getObjGen().isIndirect()) { - this->elements[this->n_elements] = std::move(obj); - } - ++this->n_elements; + elements.push_back(std::move(obj)); } 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); - if (iter == this->elements.end()) { - 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); + auto const& obj = elements.at(idx); + return obj ? obj : null_oh; } void OHArray::disconnect() { - for (auto& iter: this->elements) { - QPDFObjectHandle::DisconnectAccess::disconnect(iter.second); + for (auto const& iter: elements) { + 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()) { - this->elements.erase(idx); - } else { - this->elements[idx] = oh; - } + elements[idx] = oh.getObj(); } 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; - for (auto const& iter: this->elements) { - if (iter.first < idx) { - dest.insert(iter); - } else if (iter.first > idx) { - dest[iter.first - 1] = iter.second; - } - } - this->elements = dest; - --this->n_elements; + int n = int(idx); + elements.erase(elements.cbegin() + n); } 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; - for (auto const& iter: this->elements) { - if (iter.first < idx) { - dest.insert(iter); - } else { - dest[iter.first + 1] = iter.second; - } - } - this->elements = dest; - this->elements[idx] = oh; - ++this->n_elements; + int n = int(idx); + elements.insert(elements.cbegin() + n, oh.getObj()); } } @@ -126,23 +88,12 @@ OHArray OHArray::copy() { OHArray result; - result.n_elements = this->n_elements; - for (auto const& element: this->elements) { - auto value = element.second; - result.elements[element.first] = - value.isIndirect() ? value : value.shallowCopy(); + result.elements.reserve(elements.size()); + for (auto const& element: elements) { + result.elements.push_back( + element ? (element->getObjGen().isIndirect() ? element + : 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(); -} diff --git a/libqpdf/QPDF_Array.cc b/libqpdf/QPDF_Array.cc index ed43245f..7a3a29ca 100644 --- a/libqpdf/QPDF_Array.cc +++ b/libqpdf/QPDF_Array.cc @@ -205,11 +205,7 @@ QPDF_Array::setFromVector(std::vector>&& v) } else { elements = OHArray(); for (auto&& item: v) { - if (item) { - elements.append(item); - } else { - ++elements.n_elements; - } + elements.append(std::move(item)); } } } diff --git a/libqpdf/qpdf/OHArray.hh b/libqpdf/qpdf/OHArray.hh index 66223c4f..e6dc9524 100644 --- a/libqpdf/qpdf/OHArray.hh +++ b/libqpdf/qpdf/OHArray.hh @@ -2,7 +2,9 @@ #define QPDF_OHARRAY_HH #include -#include +#include + +#include class QPDF_Array; @@ -14,22 +16,15 @@ class OHArray void append(QPDFObjectHandle oh); void append(std::shared_ptr&& 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::const_iterator - const_iterator; - const_iterator begin() const; - const_iterator end() const; - private: friend class QPDF_Array; - std::unordered_map elements; - size_t n_elements; + std::vector> elements; }; #endif // QPDF_OHARRAY_HH -- cgit v1.2.3-54-g00ecf From 8fdc3f09648ad2c79455363255b9f8fdac9e65f3 Mon Sep 17 00:00:00 2001 From: m-holger Date: Sun, 26 Mar 2023 20:02:49 +0100 Subject: Optimize QPDFParser for non-sparse QPDF_Arrays Stop using nullptr to represent null objects. Count null array elements and trigger creation of sparse arrays if null count is greater than 100. --- libqpdf/QPDFParser.cc | 26 +++++++++++++++----------- libqpdf/QPDF_Array.cc | 18 ++++++++++++------ libqpdf/qpdf/QPDF_Array.hh | 4 ++-- 3 files changed, 29 insertions(+), 19 deletions(-) diff --git a/libqpdf/QPDFParser.cc b/libqpdf/QPDFParser.cc index 09bf1601..4c43e487 100644 --- a/libqpdf/QPDFParser.cc +++ b/libqpdf/QPDFParser.cc @@ -27,16 +27,15 @@ namespace struct StackFrame { StackFrame(std::shared_ptr input) : - offset(input->tell()), - contents_string(""), - contents_offset(-1) + offset(input->tell()) { } std::vector> olist; qpdf_offset_t offset; - std::string contents_string; - qpdf_offset_t contents_offset; + std::string contents_string{""}; + qpdf_offset_t contents_offset{-1}; + int null_count{0}; }; } // namespace @@ -50,6 +49,7 @@ QPDFParser::parse(bool& empty, bool content_stream) // this, it will cause a logic error to be thrown from // QPDF::inParse(). + const static std::shared_ptr null_oh = QPDF_Null::create(); QPDF::ParseGuard pg(context); empty = false; @@ -67,7 +67,6 @@ QPDFParser::parse(bool& empty, bool content_stream) int good_count = 0; bool b_contents = false; bool is_null = false; - auto null_oh = QPDF_Null::create(); while (!done) { bool bad = false; @@ -156,6 +155,8 @@ QPDFParser::parse(bool& empty, bool content_stream) case QPDFTokenizer::tt_null: is_null = true; + ++frame.null_count; + break; case QPDFTokenizer::tt_integer: @@ -301,9 +302,11 @@ QPDFParser::parse(bool& empty, bool content_stream) case st_dictionary: case st_array: - if (!indirect_ref && !is_null) { - // No need to set description for direct nulls - they will - // become implicit. + if (is_null) { + object = null_oh; + // No need to set description for direct nulls - they probably + // will become implicit. + } else if (!indirect_ref) { setDescription(object, input->getLastOffset()); } set_offset = true; @@ -326,7 +329,8 @@ QPDFParser::parse(bool& empty, bool content_stream) parser_state_e old_state = state_stack.back(); state_stack.pop_back(); if (old_state == st_array) { - object = QPDF_Array::create(std::move(olist)); + object = QPDF_Array::create( + std::move(olist), frame.null_count > 100); setDescription(object, offset - 1); // The `offset` points to the next of "[". Set the rewind // offset to point to the beginning of "[". This has been @@ -381,7 +385,7 @@ QPDFParser::parse(bool& empty, bool content_stream) // Calculate value. std::shared_ptr val; if (iter != olist.end()) { - val = *iter ? *iter : QPDF_Null::create(); + val = *iter; ++iter; } else { QTC::TC("qpdf", "QPDFParser no val for last key"); diff --git a/libqpdf/QPDF_Array.cc b/libqpdf/QPDF_Array.cc index 7a3a29ca..31abd8eb 100644 --- a/libqpdf/QPDF_Array.cc +++ b/libqpdf/QPDF_Array.cc @@ -11,15 +11,19 @@ QPDF_Array::QPDF_Array(std::vector const& v) : setFromVector(v); } -QPDF_Array::QPDF_Array(std::vector>&& v) : - QPDFValue(::ot_array, "array") +QPDF_Array::QPDF_Array( + std::vector>&& v, bool sparse) : + QPDFValue(::ot_array, "array"), + sparse(sparse) { setFromVector(std::move(v)); } QPDF_Array::QPDF_Array(SparseOHArray const& items) : QPDFValue(::ot_array, "array"), + sparse(true), sp_elements(items) + { } @@ -37,9 +41,10 @@ QPDF_Array::create(std::vector const& items) } std::shared_ptr -QPDF_Array::create(std::vector>&& items) +QPDF_Array::create( + std::vector>&& items, bool sparse) { - return do_create(new QPDF_Array(std::move(items))); + return do_create(new QPDF_Array(std::move(items), sparse)); } std::shared_ptr @@ -196,8 +201,9 @@ QPDF_Array::setFromVector(std::vector>&& v) if (sparse) { sp_elements = SparseOHArray(); for (auto&& item: v) { - if (item) { - sp_elements.append(item); + if (item->getTypeCode() != ::ot_null || + item->getObjGen().isIndirect()) { + sp_elements.append(std::move(item)); } else { ++sp_elements.n_elements; } diff --git a/libqpdf/qpdf/QPDF_Array.hh b/libqpdf/qpdf/QPDF_Array.hh index 1c4227ba..00c7f59d 100644 --- a/libqpdf/qpdf/QPDF_Array.hh +++ b/libqpdf/qpdf/QPDF_Array.hh @@ -15,7 +15,7 @@ class QPDF_Array: public QPDFValue static std::shared_ptr create(std::vector const& items); static std::shared_ptr - create(std::vector>&& items); + create(std::vector>&& items, bool sparse); static std::shared_ptr create(SparseOHArray const& items); static std::shared_ptr create(OHArray const& items); virtual std::shared_ptr copy(bool shallow = false); @@ -36,7 +36,7 @@ class QPDF_Array: public QPDFValue private: QPDF_Array(std::vector const& items); - QPDF_Array(std::vector>&& items); + QPDF_Array(std::vector>&& items, bool sparse); QPDF_Array(SparseOHArray const& items); QPDF_Array(OHArray const& items); bool sparse{false}; -- cgit v1.2.3-54-g00ecf From ad2875a4aa6cc22e32bbaa848ae71619ff51c138 Mon Sep 17 00:00:00 2001 From: m-holger Date: Sat, 25 Mar 2023 16:37:47 +0000 Subject: Remove temporary OHArray::size, append and remove_last Also, add const overload of QPDFObjectHandle::getObj --- include/qpdf/QPDFObjectHandle.hh | 5 +++++ libqpdf/OHArray.cc | 20 +------------------- libqpdf/QPDF_Array.cc | 18 +++++++++--------- libqpdf/qpdf/OHArray.hh | 3 --- 4 files changed, 15 insertions(+), 31 deletions(-) diff --git a/include/qpdf/QPDFObjectHandle.hh b/include/qpdf/QPDFObjectHandle.hh index b722b083..5166e198 100644 --- a/include/qpdf/QPDFObjectHandle.hh +++ b/include/qpdf/QPDFObjectHandle.hh @@ -1578,6 +1578,11 @@ class QPDFObjectHandle { return obj; } + std::shared_ptr + getObj() const + { + return obj; + } QPDFObject* getObjectPtr() { diff --git a/libqpdf/OHArray.cc b/libqpdf/OHArray.cc index e25e1239..22b99b13 100644 --- a/libqpdf/OHArray.cc +++ b/libqpdf/OHArray.cc @@ -11,24 +11,6 @@ OHArray::OHArray() { } -size_t -OHArray::size() const -{ - return elements.size(); -} - -void -OHArray::append(QPDFObjectHandle oh) -{ - elements.push_back(oh.getObj()); -} - -void -OHArray::append(std::shared_ptr&& obj) -{ - elements.push_back(std::move(obj)); -} - QPDFObjectHandle OHArray::at(size_t idx) const { @@ -77,7 +59,7 @@ OHArray::insert(size_t idx, QPDFObjectHandle oh) throw std::logic_error("bounds error inserting item to OHArray"); } else if (idx == elements.size()) { // Allow inserting to the last position - append(oh.getObj()); + elements.push_back(oh.getObj()); } else { int n = int(idx); elements.insert(elements.cbegin() + n, oh.getObj()); diff --git a/libqpdf/QPDF_Array.cc b/libqpdf/QPDF_Array.cc index 31abd8eb..afe33827 100644 --- a/libqpdf/QPDF_Array.cc +++ b/libqpdf/QPDF_Array.cc @@ -93,7 +93,7 @@ QPDF_Array::unparse() return result; } else { std::string result = "[ "; - size_t size = elements.size(); + size_t size = elements.elements.size(); for (size_t i = 0; i < size; ++i) { result += elements.at(i).unparse(); result += " "; @@ -115,7 +115,7 @@ QPDF_Array::getJSON(int json_version) return j; } else { JSON j = JSON::makeArray(); - size_t size = elements.size(); + size_t size = elements.elements.size(); for (size_t i = 0; i < size; ++i) { j.addArrayElement(elements.at(i).getJSON(json_version)); } @@ -131,7 +131,7 @@ QPDF_Array::getNItems() const // a lot of code. return QIntC::to_int(sp_elements.size()); } else { - return QIntC::to_int(elements.size()); + return QIntC::to_int(elements.elements.size()); } } @@ -145,7 +145,7 @@ QPDF_Array::getItem(int n) const } return sp_elements.at(QIntC::to_size(n)); } else { - if ((n < 0) || (n >= QIntC::to_int(elements.size()))) { + if ((n < 0) || (n >= QIntC::to_int(elements.elements.size()))) { throw std::logic_error( "INTERNAL ERROR: bounds error accessing QPDF_Array element"); } @@ -162,7 +162,7 @@ QPDF_Array::getAsVector(std::vector& v) const v.push_back(sp_elements.at(i)); } } else { - size_t size = elements.size(); + size_t size = elements.elements.size(); for (size_t i = 0; i < size; ++i) { v.push_back(elements.at(i)); } @@ -190,7 +190,7 @@ QPDF_Array::setFromVector(std::vector const& v) } else { elements = OHArray(); for (auto const& iter: v) { - elements.append(iter); + elements.elements.push_back(iter.getObj()); } } } @@ -211,7 +211,7 @@ QPDF_Array::setFromVector(std::vector>&& v) } else { elements = OHArray(); for (auto&& item: v) { - elements.append(std::move(item)); + elements.elements.push_back(std::move(item)); } } } @@ -228,7 +228,7 @@ QPDF_Array::insertItem(int at, QPDFObjectHandle const& item) sp_elements.insert(QIntC::to_size(at), item); } else { // As special case, also allow insert beyond the end - if ((at < 0) || (at > QIntC::to_int(elements.size()))) { + if ((at < 0) || (at > QIntC::to_int(elements.elements.size()))) { throw std::logic_error( "INTERNAL ERROR: bounds error accessing QPDF_Array element"); } @@ -242,7 +242,7 @@ QPDF_Array::appendItem(QPDFObjectHandle const& item) if (sparse) { sp_elements.append(item); } else { - elements.append(item); + elements.elements.push_back(item.getObj()); } } diff --git a/libqpdf/qpdf/OHArray.hh b/libqpdf/qpdf/OHArray.hh index e6dc9524..a87cd6b5 100644 --- a/libqpdf/qpdf/OHArray.hh +++ b/libqpdf/qpdf/OHArray.hh @@ -12,9 +12,6 @@ class OHArray { public: OHArray(); - size_t size() const; - void append(QPDFObjectHandle oh); - void append(std::shared_ptr&& obj); QPDFObjectHandle at(size_t idx) const; void setAt(size_t idx, QPDFObjectHandle oh); void erase(size_t idx); -- cgit v1.2.3-54-g00ecf From 1367226002d48bc0d67c08cae498fd92dad41977 Mon Sep 17 00:00:00 2001 From: m-holger Date: Sat, 25 Mar 2023 16:59:49 +0000 Subject: Remove temporary OHArray::at --- libqpdf/OHArray.cc | 11 ----------- libqpdf/QPDF_Array.cc | 21 +++++++++++---------- libqpdf/qpdf/OHArray.hh | 1 - 3 files changed, 11 insertions(+), 22 deletions(-) diff --git a/libqpdf/OHArray.cc b/libqpdf/OHArray.cc index 22b99b13..dc01421f 100644 --- a/libqpdf/OHArray.cc +++ b/libqpdf/OHArray.cc @@ -11,17 +11,6 @@ OHArray::OHArray() { } -QPDFObjectHandle -OHArray::at(size_t idx) const -{ - if (idx >= elements.size()) { - throw std::logic_error( - "INTERNAL ERROR: bounds error accessing OHArray element"); - } - auto const& obj = elements.at(idx); - return obj ? obj : null_oh; -} - void OHArray::disconnect() { diff --git a/libqpdf/QPDF_Array.cc b/libqpdf/QPDF_Array.cc index afe33827..b86c3f0a 100644 --- a/libqpdf/QPDF_Array.cc +++ b/libqpdf/QPDF_Array.cc @@ -5,6 +5,8 @@ #include #include +static const QPDFObjectHandle null_oh = QPDFObjectHandle::newNull(); + QPDF_Array::QPDF_Array(std::vector const& v) : QPDFValue(::ot_array, "array") { @@ -93,9 +95,9 @@ QPDF_Array::unparse() return result; } else { std::string result = "[ "; - size_t size = elements.elements.size(); - for (size_t i = 0; i < size; ++i) { - result += elements.at(i).unparse(); + auto size = elements.elements.size(); + for (int i = 0; i < int(size); ++i) { + result += getItem(i).unparse(); result += " "; } result += "]"; @@ -116,8 +118,8 @@ QPDF_Array::getJSON(int json_version) } else { JSON j = JSON::makeArray(); size_t size = elements.elements.size(); - for (size_t i = 0; i < size; ++i) { - j.addArrayElement(elements.at(i).getJSON(json_version)); + for (int i = 0; i < int(size); ++i) { + j.addArrayElement(getItem(i).getJSON(json_version)); } return j; } @@ -149,7 +151,8 @@ QPDF_Array::getItem(int n) const throw std::logic_error( "INTERNAL ERROR: bounds error accessing QPDF_Array element"); } - return elements.at(QIntC::to_size(n)); + auto const& obj = elements.elements.at(size_t(n)); + return obj ? obj : null_oh; } } @@ -162,10 +165,8 @@ QPDF_Array::getAsVector(std::vector& v) const v.push_back(sp_elements.at(i)); } } else { - size_t size = elements.elements.size(); - for (size_t i = 0; i < size; ++i) { - v.push_back(elements.at(i)); - } + v = std::vector( + elements.elements.cbegin(), elements.elements.cend()); } } diff --git a/libqpdf/qpdf/OHArray.hh b/libqpdf/qpdf/OHArray.hh index a87cd6b5..e8abc8b3 100644 --- a/libqpdf/qpdf/OHArray.hh +++ b/libqpdf/qpdf/OHArray.hh @@ -12,7 +12,6 @@ class OHArray { public: OHArray(); - QPDFObjectHandle at(size_t idx) const; void setAt(size_t idx, QPDFObjectHandle oh); void erase(size_t idx); void insert(size_t idx, QPDFObjectHandle oh); -- cgit v1.2.3-54-g00ecf From 0db65e79120e66069a37906845a984457b711b6f Mon Sep 17 00:00:00 2001 From: m-holger Date: Sat, 25 Mar 2023 17:23:19 +0000 Subject: Remove temporary OHArray::disconnect and setAt --- include/qpdf/QPDFObjectHandle.hh | 4 ++-- libqpdf/OHArray.cc | 20 -------------------- libqpdf/QPDF_Array.cc | 12 ++++++++++-- libqpdf/qpdf/OHArray.hh | 2 -- 4 files changed, 12 insertions(+), 26 deletions(-) diff --git a/include/qpdf/QPDFObjectHandle.hh b/include/qpdf/QPDFObjectHandle.hh index 5166e198..ecb50ba0 100644 --- a/include/qpdf/QPDFObjectHandle.hh +++ b/include/qpdf/QPDFObjectHandle.hh @@ -1494,14 +1494,14 @@ class QPDFObjectHandle // disconnected(). class DisconnectAccess { + friend class QPDF_Array; friend class QPDF_Dictionary; friend class QPDF_Stream; friend class SparseOHArray; - friend class OHArray; private: static void - disconnect(QPDFObjectHandle& o) + disconnect(QPDFObjectHandle o) { o.disconnect(); } diff --git a/libqpdf/OHArray.cc b/libqpdf/OHArray.cc index dc01421f..ce6a595e 100644 --- a/libqpdf/OHArray.cc +++ b/libqpdf/OHArray.cc @@ -11,26 +11,6 @@ OHArray::OHArray() { } -void -OHArray::disconnect() -{ - for (auto const& iter: elements) { - if (iter) { - QPDFObjectHandle oh = iter; - QPDFObjectHandle::DisconnectAccess::disconnect(oh); - } - } -} - -void -OHArray::setAt(size_t idx, QPDFObjectHandle oh) -{ - if (idx >= elements.size()) { - throw std::logic_error("bounds error setting item in OHArray"); - } - elements[idx] = oh.getObj(); -} - void OHArray::erase(size_t idx) { diff --git a/libqpdf/QPDF_Array.cc b/libqpdf/QPDF_Array.cc index b86c3f0a..ce08332a 100644 --- a/libqpdf/QPDF_Array.cc +++ b/libqpdf/QPDF_Array.cc @@ -77,7 +77,11 @@ QPDF_Array::disconnect() if (sparse) { sp_elements.disconnect(); } else { - elements.disconnect(); + for (auto const& iter: elements.elements) { + if (iter) { + QPDFObjectHandle::DisconnectAccess::disconnect(iter); + } + } } } @@ -176,7 +180,11 @@ QPDF_Array::setItem(int n, QPDFObjectHandle const& oh) if (sparse) { sp_elements.setAt(QIntC::to_size(n), oh); } else { - elements.setAt(QIntC::to_size(n), oh); + size_t idx = size_t(n); + if (n < 0 || idx >= elements.elements.size()) { + throw std::logic_error("bounds error setting item in QPDF_Array"); + } + elements.elements[idx] = oh.getObj(); } } diff --git a/libqpdf/qpdf/OHArray.hh b/libqpdf/qpdf/OHArray.hh index e8abc8b3..e4d880db 100644 --- a/libqpdf/qpdf/OHArray.hh +++ b/libqpdf/qpdf/OHArray.hh @@ -12,11 +12,9 @@ class OHArray { public: OHArray(); - void setAt(size_t idx, QPDFObjectHandle oh); void erase(size_t idx); void insert(size_t idx, QPDFObjectHandle oh); OHArray copy(); - void disconnect(); private: friend class QPDF_Array; -- cgit v1.2.3-54-g00ecf From 9e30de80326ad88c155725c66e3d444232119deb Mon Sep 17 00:00:00 2001 From: m-holger Date: Sat, 25 Mar 2023 18:05:54 +0000 Subject: Remove temporary OHArray::erase, insert and copy --- libqpdf/OHArray.cc | 45 --------------------------------------------- libqpdf/QPDF_Array.cc | 32 +++++++++++++++++++++++++++++--- libqpdf/qpdf/OHArray.hh | 3 --- 3 files changed, 29 insertions(+), 51 deletions(-) diff --git a/libqpdf/OHArray.cc b/libqpdf/OHArray.cc index ce6a595e..377b1a36 100644 --- a/libqpdf/OHArray.cc +++ b/libqpdf/OHArray.cc @@ -1,50 +1,5 @@ #include -#include -#include - -#include - -static const QPDFObjectHandle null_oh = QPDFObjectHandle::newNull(); - OHArray::OHArray() { } - -void -OHArray::erase(size_t idx) -{ - if (idx >= elements.size()) { - throw std::logic_error("bounds error erasing item from OHArray"); - } - int n = int(idx); - elements.erase(elements.cbegin() + n); -} - -void -OHArray::insert(size_t idx, QPDFObjectHandle oh) -{ - if (idx > elements.size()) { - throw std::logic_error("bounds error inserting item to OHArray"); - } else if (idx == elements.size()) { - // Allow inserting to the last position - elements.push_back(oh.getObj()); - } else { - int n = int(idx); - elements.insert(elements.cbegin() + n, oh.getObj()); - } -} - -OHArray -OHArray::copy() -{ - OHArray result; - result.elements.reserve(elements.size()); - for (auto const& element: elements) { - result.elements.push_back( - element ? (element->getObjGen().isIndirect() ? element - : element->copy()) - : element); - } - return result; -} diff --git a/libqpdf/QPDF_Array.cc b/libqpdf/QPDF_Array.cc index ce08332a..c7a9f315 100644 --- a/libqpdf/QPDF_Array.cc +++ b/libqpdf/QPDF_Array.cc @@ -67,7 +67,20 @@ QPDF_Array::copy(bool shallow) if (sparse) { return create(shallow ? sp_elements : sp_elements.copy()); } else { - return create(shallow ? elements : elements.copy()); + if (shallow) { + return create(elements); + } else { + OHArray result; + result.elements.reserve(elements.elements.size()); + for (auto const& element: elements.elements) { + result.elements.push_back( + element + ? (element->getObjGen().isIndirect() ? element + : element->copy()) + : element); + } + return create(result); + } } } @@ -237,11 +250,19 @@ QPDF_Array::insertItem(int at, QPDFObjectHandle const& item) sp_elements.insert(QIntC::to_size(at), item); } else { // As special case, also allow insert beyond the end + size_t idx = QIntC::to_size(at); if ((at < 0) || (at > QIntC::to_int(elements.elements.size()))) { throw std::logic_error( "INTERNAL ERROR: bounds error accessing QPDF_Array element"); } - elements.insert(QIntC::to_size(at), item); + if (idx == elements.elements.size()) { + // Allow inserting to the last position + elements.elements.push_back(item.getObj()); + } else { + int n = int(idx); + elements.elements.insert( + elements.elements.cbegin() + n, item.getObj()); + } } } @@ -261,6 +282,11 @@ QPDF_Array::eraseItem(int at) if (sparse) { sp_elements.erase(QIntC::to_size(at)); } else { - elements.erase(QIntC::to_size(at)); + size_t idx = QIntC::to_size(at); + if (idx >= elements.elements.size()) { + throw std::logic_error("bounds error erasing item from OHArray"); + } + int n = int(idx); + elements.elements.erase(elements.elements.cbegin() + n); } } diff --git a/libqpdf/qpdf/OHArray.hh b/libqpdf/qpdf/OHArray.hh index e4d880db..563145c2 100644 --- a/libqpdf/qpdf/OHArray.hh +++ b/libqpdf/qpdf/OHArray.hh @@ -12,9 +12,6 @@ class OHArray { public: OHArray(); - void erase(size_t idx); - void insert(size_t idx, QPDFObjectHandle oh); - OHArray copy(); private: friend class QPDF_Array; -- cgit v1.2.3-54-g00ecf From ea5164938e77cb968003fe06f98b87f648319bbf Mon Sep 17 00:00:00 2001 From: m-holger Date: Sat, 25 Mar 2023 18:23:39 +0000 Subject: Remove temporary OHArray --- libqpdf/CMakeLists.txt | 1 - libqpdf/OHArray.cc | 5 ----- libqpdf/QPDF_Array.cc | 55 +++++++++++++++++++++------------------------- libqpdf/qpdf/OHArray.hh | 21 ------------------ libqpdf/qpdf/QPDF_Array.hh | 9 ++++---- 5 files changed, 29 insertions(+), 62 deletions(-) delete mode 100644 libqpdf/OHArray.cc delete mode 100644 libqpdf/qpdf/OHArray.hh diff --git a/libqpdf/CMakeLists.txt b/libqpdf/CMakeLists.txt index 2d857f45..5e3a628e 100644 --- a/libqpdf/CMakeLists.txt +++ b/libqpdf/CMakeLists.txt @@ -116,7 +116,6 @@ set(libqpdf_SOURCES SecureRandomDataProvider.cc SF_FlateLzwDecode.cc SparseOHArray.cc - OHArray.cc qpdf-c.cc qpdfjob-c.cc qpdflogger-c.cc) diff --git a/libqpdf/OHArray.cc b/libqpdf/OHArray.cc deleted file mode 100644 index 377b1a36..00000000 --- a/libqpdf/OHArray.cc +++ /dev/null @@ -1,5 +0,0 @@ -#include - -OHArray::OHArray() -{ -} diff --git a/libqpdf/QPDF_Array.cc b/libqpdf/QPDF_Array.cc index c7a9f315..620b75eb 100644 --- a/libqpdf/QPDF_Array.cc +++ b/libqpdf/QPDF_Array.cc @@ -29,7 +29,7 @@ QPDF_Array::QPDF_Array(SparseOHArray const& items) : { } -QPDF_Array::QPDF_Array(OHArray const& items) : +QPDF_Array::QPDF_Array(std::vector> const& items) : QPDFValue(::ot_array, "array"), sparse(false), elements(items) @@ -56,7 +56,7 @@ QPDF_Array::create(SparseOHArray const& items) } std::shared_ptr -QPDF_Array::create(OHArray const& items) +QPDF_Array::create(std::vector> const& items) { return do_create(new QPDF_Array(items)); } @@ -70,10 +70,10 @@ QPDF_Array::copy(bool shallow) if (shallow) { return create(elements); } else { - OHArray result; - result.elements.reserve(elements.elements.size()); - for (auto const& element: elements.elements) { - result.elements.push_back( + std::vector> result; + result.reserve(elements.size()); + for (auto const& element: elements) { + result.push_back( element ? (element->getObjGen().isIndirect() ? element : element->copy()) @@ -90,7 +90,7 @@ QPDF_Array::disconnect() if (sparse) { sp_elements.disconnect(); } else { - for (auto const& iter: elements.elements) { + for (auto const& iter: elements) { if (iter) { QPDFObjectHandle::DisconnectAccess::disconnect(iter); } @@ -112,7 +112,7 @@ QPDF_Array::unparse() return result; } else { std::string result = "[ "; - auto size = elements.elements.size(); + auto size = elements.size(); for (int i = 0; i < int(size); ++i) { result += getItem(i).unparse(); result += " "; @@ -134,7 +134,7 @@ QPDF_Array::getJSON(int json_version) return j; } else { JSON j = JSON::makeArray(); - size_t size = elements.elements.size(); + size_t size = elements.size(); for (int i = 0; i < int(size); ++i) { j.addArrayElement(getItem(i).getJSON(json_version)); } @@ -150,7 +150,7 @@ QPDF_Array::getNItems() const // a lot of code. return QIntC::to_int(sp_elements.size()); } else { - return QIntC::to_int(elements.elements.size()); + return QIntC::to_int(elements.size()); } } @@ -164,11 +164,11 @@ QPDF_Array::getItem(int n) const } return sp_elements.at(QIntC::to_size(n)); } else { - if ((n < 0) || (n >= QIntC::to_int(elements.elements.size()))) { + if ((n < 0) || (n >= QIntC::to_int(elements.size()))) { throw std::logic_error( "INTERNAL ERROR: bounds error accessing QPDF_Array element"); } - auto const& obj = elements.elements.at(size_t(n)); + auto const& obj = elements.at(size_t(n)); return obj ? obj : null_oh; } } @@ -182,8 +182,7 @@ QPDF_Array::getAsVector(std::vector& v) const v.push_back(sp_elements.at(i)); } } else { - v = std::vector( - elements.elements.cbegin(), elements.elements.cend()); + v = std::vector(elements.cbegin(), elements.cend()); } } @@ -194,10 +193,10 @@ QPDF_Array::setItem(int n, QPDFObjectHandle const& oh) sp_elements.setAt(QIntC::to_size(n), oh); } else { size_t idx = size_t(n); - if (n < 0 || idx >= elements.elements.size()) { + if (n < 0 || idx >= elements.size()) { throw std::logic_error("bounds error setting item in QPDF_Array"); } - elements.elements[idx] = oh.getObj(); + elements[idx] = oh.getObj(); } } @@ -210,9 +209,9 @@ QPDF_Array::setFromVector(std::vector const& v) sp_elements.append(iter); } } else { - elements = OHArray(); + elements.resize(0); for (auto const& iter: v) { - elements.elements.push_back(iter.getObj()); + elements.push_back(iter.getObj()); } } } @@ -231,10 +230,7 @@ QPDF_Array::setFromVector(std::vector>&& v) } } } else { - elements = OHArray(); - for (auto&& item: v) { - elements.elements.push_back(std::move(item)); - } + elements = std::move(v); } } @@ -251,17 +247,16 @@ QPDF_Array::insertItem(int at, QPDFObjectHandle const& item) } else { // As special case, also allow insert beyond the end size_t idx = QIntC::to_size(at); - if ((at < 0) || (at > QIntC::to_int(elements.elements.size()))) { + if ((at < 0) || (at > QIntC::to_int(elements.size()))) { throw std::logic_error( "INTERNAL ERROR: bounds error accessing QPDF_Array element"); } - if (idx == elements.elements.size()) { + if (idx == elements.size()) { // Allow inserting to the last position - elements.elements.push_back(item.getObj()); + elements.push_back(item.getObj()); } else { int n = int(idx); - elements.elements.insert( - elements.elements.cbegin() + n, item.getObj()); + elements.insert(elements.cbegin() + n, item.getObj()); } } } @@ -272,7 +267,7 @@ QPDF_Array::appendItem(QPDFObjectHandle const& item) if (sparse) { sp_elements.append(item); } else { - elements.elements.push_back(item.getObj()); + elements.push_back(item.getObj()); } } @@ -283,10 +278,10 @@ QPDF_Array::eraseItem(int at) sp_elements.erase(QIntC::to_size(at)); } else { size_t idx = QIntC::to_size(at); - if (idx >= elements.elements.size()) { + if (idx >= elements.size()) { throw std::logic_error("bounds error erasing item from OHArray"); } int n = int(idx); - elements.elements.erase(elements.elements.cbegin() + n); + elements.erase(elements.cbegin() + n); } } diff --git a/libqpdf/qpdf/OHArray.hh b/libqpdf/qpdf/OHArray.hh deleted file mode 100644 index 563145c2..00000000 --- a/libqpdf/qpdf/OHArray.hh +++ /dev/null @@ -1,21 +0,0 @@ -#ifndef QPDF_OHARRAY_HH -#define QPDF_OHARRAY_HH - -#include -#include - -#include - -class QPDF_Array; - -class OHArray -{ - public: - OHArray(); - - private: - friend class QPDF_Array; - std::vector> elements; -}; - -#endif // QPDF_OHARRAY_HH diff --git a/libqpdf/qpdf/QPDF_Array.hh b/libqpdf/qpdf/QPDF_Array.hh index 00c7f59d..b52efeb6 100644 --- a/libqpdf/qpdf/QPDF_Array.hh +++ b/libqpdf/qpdf/QPDF_Array.hh @@ -3,9 +3,7 @@ #include -#include #include -#include #include class QPDF_Array: public QPDFValue @@ -17,7 +15,8 @@ class QPDF_Array: public QPDFValue static std::shared_ptr create(std::vector>&& items, bool sparse); static std::shared_ptr create(SparseOHArray const& items); - static std::shared_ptr create(OHArray const& items); + static std::shared_ptr + create(std::vector> const& items); virtual std::shared_ptr copy(bool shallow = false); virtual std::string unparse(); virtual JSON getJSON(int json_version); @@ -38,10 +37,10 @@ class QPDF_Array: public QPDFValue QPDF_Array(std::vector const& items); QPDF_Array(std::vector>&& items, bool sparse); QPDF_Array(SparseOHArray const& items); - QPDF_Array(OHArray const& items); + QPDF_Array(std::vector> const& items); bool sparse{false}; SparseOHArray sp_elements; - OHArray elements; + std::vector> elements; }; #endif // QPDF_ARRAY_HH -- cgit v1.2.3-54-g00ecf From e6db8ddeba560cd472f34be2e20e31ea2f4bfee3 Mon Sep 17 00:00:00 2001 From: m-holger Date: Wed, 16 Nov 2022 18:50:13 +0000 Subject: Change SparseOHArray index type to int and elements type to map There are no reasons other than historical to use size_t. On balance, using map is more efficient. Hold shared pointers to QPDFObjects rather than QPDFObjectHandles for consistencey with QPDF_Array. --- libqpdf/QPDF_Array.cc | 22 +++++++++++----------- libqpdf/SparseOHArray.cc | 28 ++++++++++------------------ libqpdf/qpdf/SparseOHArray.hh | 21 +++++++++++---------- 3 files changed, 32 insertions(+), 39 deletions(-) diff --git a/libqpdf/QPDF_Array.cc b/libqpdf/QPDF_Array.cc index 620b75eb..6568db96 100644 --- a/libqpdf/QPDF_Array.cc +++ b/libqpdf/QPDF_Array.cc @@ -103,8 +103,8 @@ QPDF_Array::unparse() { if (sparse) { std::string result = "[ "; - size_t size = sp_elements.size(); - for (size_t i = 0; i < size; ++i) { + int size = sp_elements.size(); + for (int i = 0; i < size; ++i) { result += sp_elements.at(i).unparse(); result += " "; } @@ -127,8 +127,8 @@ QPDF_Array::getJSON(int json_version) { if (sparse) { JSON j = JSON::makeArray(); - size_t size = sp_elements.size(); - for (size_t i = 0; i < size; ++i) { + int size = sp_elements.size(); + for (int i = 0; i < size; ++i) { j.addArrayElement(sp_elements.at(i).getJSON(json_version)); } return j; @@ -162,7 +162,7 @@ QPDF_Array::getItem(int n) const throw std::logic_error( "INTERNAL ERROR: bounds error accessing QPDF_Array element"); } - return sp_elements.at(QIntC::to_size(n)); + return sp_elements.at(n); } else { if ((n < 0) || (n >= QIntC::to_int(elements.size()))) { throw std::logic_error( @@ -177,8 +177,8 @@ void QPDF_Array::getAsVector(std::vector& v) const { if (sparse) { - size_t size = sp_elements.size(); - for (size_t i = 0; i < size; ++i) { + int size = sp_elements.size(); + for (int i = 0; i < size; ++i) { v.push_back(sp_elements.at(i)); } } else { @@ -190,7 +190,7 @@ void QPDF_Array::setItem(int n, QPDFObjectHandle const& oh) { if (sparse) { - sp_elements.setAt(QIntC::to_size(n), oh); + sp_elements.setAt(n, oh); } else { size_t idx = size_t(n); if (n < 0 || idx >= elements.size()) { @@ -239,11 +239,11 @@ QPDF_Array::insertItem(int at, QPDFObjectHandle const& item) { if (sparse) { // As special case, also allow insert beyond the end - if ((at < 0) || (at > QIntC::to_int(sp_elements.size()))) { + if ((at < 0) || (at > sp_elements.size())) { throw std::logic_error( "INTERNAL ERROR: bounds error accessing QPDF_Array element"); } - sp_elements.insert(QIntC::to_size(at), item); + sp_elements.insert(at, item); } else { // As special case, also allow insert beyond the end size_t idx = QIntC::to_size(at); @@ -275,7 +275,7 @@ void QPDF_Array::eraseItem(int at) { if (sparse) { - sp_elements.erase(QIntC::to_size(at)); + sp_elements.erase(at); } else { size_t idx = QIntC::to_size(at); if (idx >= elements.size()) { diff --git a/libqpdf/SparseOHArray.cc b/libqpdf/SparseOHArray.cc index 5f64f50b..7adba00f 100644 --- a/libqpdf/SparseOHArray.cc +++ b/libqpdf/SparseOHArray.cc @@ -1,16 +1,8 @@ #include -#include -#include - #include -SparseOHArray::SparseOHArray() : - n_elements(0) -{ -} - -size_t +int SparseOHArray::size() const { return this->n_elements; @@ -20,7 +12,7 @@ void SparseOHArray::append(QPDFObjectHandle oh) { if (!oh.isDirectNull()) { - this->elements[this->n_elements] = oh; + this->elements[this->n_elements] = oh.getObj(); } ++this->n_elements; } @@ -35,9 +27,9 @@ SparseOHArray::append(std::shared_ptr&& obj) } QPDFObjectHandle -SparseOHArray::at(size_t idx) const +SparseOHArray::at(int idx) const { - if (idx >= this->n_elements) { + if (idx < 0 || idx >= this->n_elements) { throw std::logic_error( "INTERNAL ERROR: bounds error accessing SparseOHArray element"); } @@ -69,7 +61,7 @@ SparseOHArray::disconnect() } void -SparseOHArray::setAt(size_t idx, QPDFObjectHandle oh) +SparseOHArray::setAt(int idx, QPDFObjectHandle oh) { if (idx >= this->n_elements) { throw std::logic_error("bounds error setting item in SparseOHArray"); @@ -77,12 +69,12 @@ SparseOHArray::setAt(size_t idx, QPDFObjectHandle oh) if (oh.isDirectNull()) { this->elements.erase(idx); } else { - this->elements[idx] = oh; + this->elements[idx] = oh.getObj(); } } void -SparseOHArray::erase(size_t idx) +SparseOHArray::erase(int idx) { if (idx >= this->n_elements) { throw std::logic_error("bounds error erasing item from SparseOHArray"); @@ -100,7 +92,7 @@ SparseOHArray::erase(size_t idx) } void -SparseOHArray::insert(size_t idx, QPDFObjectHandle oh) +SparseOHArray::insert(int idx, QPDFObjectHandle oh) { if (idx > this->n_elements) { throw std::logic_error("bounds error inserting item to SparseOHArray"); @@ -117,7 +109,7 @@ SparseOHArray::insert(size_t idx, QPDFObjectHandle oh) } } this->elements = dest; - this->elements[idx] = oh; + this->elements[idx] = oh.getObj(); ++this->n_elements; } } @@ -130,7 +122,7 @@ SparseOHArray::copy() for (auto const& element: this->elements) { auto value = element.second; result.elements[element.first] = - value.isIndirect() ? value : value.shallowCopy(); + value->getObjGen().isIndirect() ? value : value->copy(); } return result; } diff --git a/libqpdf/qpdf/SparseOHArray.hh b/libqpdf/qpdf/SparseOHArray.hh index 26ae3dc0..191d6915 100644 --- a/libqpdf/qpdf/SparseOHArray.hh +++ b/libqpdf/qpdf/SparseOHArray.hh @@ -2,34 +2,35 @@ #define QPDF_SPARSEOHARRAY_HH #include -#include +#include +#include class QPDF_Array; class SparseOHArray { public: - SparseOHArray(); - size_t size() const; + SparseOHArray() = default; + int size() const; void append(QPDFObjectHandle oh); void append(std::shared_ptr&& obj); - QPDFObjectHandle at(size_t idx) const; + QPDFObjectHandle at(int idx) const; void remove_last(); - void setAt(size_t idx, QPDFObjectHandle oh); - void erase(size_t idx); - void insert(size_t idx, QPDFObjectHandle oh); + void setAt(int idx, QPDFObjectHandle oh); + void erase(int idx); + void insert(int idx, QPDFObjectHandle oh); SparseOHArray copy(); void disconnect(); - typedef std::unordered_map::const_iterator + typedef std::map>::const_iterator const_iterator; const_iterator begin() const; const_iterator end() const; private: friend class QPDF_Array; - std::unordered_map elements; - size_t n_elements; + std::map> elements; + int n_elements{0}; }; #endif // QPDF_SPARSEOHARRAY_HH -- cgit v1.2.3-54-g00ecf From 51d350c98c549ff59dd6423e98d993385f57fa9c Mon Sep 17 00:00:00 2001 From: m-holger Date: Fri, 24 Mar 2023 12:58:36 +0000 Subject: Inline QPDF_Array::getNItems and rename to size --- libqpdf/QPDFObjectHandle.cc | 26 +++++++++++--------------- libqpdf/QPDF_Array.cc | 12 ------------ libqpdf/SparseOHArray.cc | 6 ------ libqpdf/qpdf/QPDF_Array.hh | 6 +++++- libqpdf/qpdf/SparseOHArray.hh | 6 +++++- 5 files changed, 21 insertions(+), 35 deletions(-) diff --git a/libqpdf/QPDFObjectHandle.cc b/libqpdf/QPDFObjectHandle.cc index d474dcce..9fd8684b 100644 --- a/libqpdf/QPDFObjectHandle.cc +++ b/libqpdf/QPDFObjectHandle.cc @@ -789,9 +789,8 @@ QPDFObjectHandle::aitems() int QPDFObjectHandle::getArrayNItems() { - auto array = asArray(); - if (array) { - return array->getNItems(); + if (auto array = asArray()) { + return array->size(); } else { typeWarning("array", "treating as empty"); QTC::TC("qpdf", "QPDFObjectHandle array treating as empty"); @@ -803,7 +802,7 @@ QPDFObjectHandle QPDFObjectHandle::getArrayItem(int n) { auto array = asArray(); - if (array && (n < array->getNItems()) && (n >= 0)) { + if (array && n < array->size() && n >= 0) { return array->getItem(n); } else { if (array) { @@ -823,7 +822,7 @@ bool QPDFObjectHandle::isRectangle() { auto array = asArray(); - if ((array == nullptr) || (array->getNItems() != 4)) { + if (array == nullptr || array->size() != 4) { return false; } for (int i = 0; i < 4; ++i) { @@ -838,7 +837,7 @@ bool QPDFObjectHandle::isMatrix() { auto array = asArray(); - if ((array == nullptr) || (array->getNItems() != 6)) { + if (array == nullptr || array->size() != 6) { return false; } for (int i = 0; i < 6; ++i) { @@ -975,7 +974,7 @@ void QPDFObjectHandle::eraseItem(int at) { auto array = asArray(); - if (array && (at < array->getNItems()) && (at >= 0)) { + if (array && at < array->size() && at >= 0) { array->eraseItem(at); } else { if (array) { @@ -991,11 +990,9 @@ QPDFObjectHandle::eraseItem(int at) QPDFObjectHandle QPDFObjectHandle::eraseItemAndGetOld(int at) { - auto result = QPDFObjectHandle::newNull(); auto array = asArray(); - if (array && (at < array->getNItems()) && (at >= 0)) { - result = array->getItem(at); - } + auto result = (array && at < array->size() && at >= 0) ? array->getItem(at) + : newNull(); eraseItem(at); return result; } @@ -1515,9 +1512,8 @@ QPDFObjectHandle::arrayOrStreamToStreamArray( { all_description = description; std::vector result; - auto array = asArray(); - if (array) { - int n_items = array->getNItems(); + if (auto array = asArray()) { + int n_items = array->size(); for (int i = 0; i < n_items; ++i) { QPDFObjectHandle item = array->getItem(i); if (item.isStream()) { @@ -2217,7 +2213,7 @@ QPDFObjectHandle::makeDirect( } else if (isArray()) { std::vector items; auto array = asArray(); - int n = array->getNItems(); + int n = array->size(); for (int i = 0; i < n; ++i) { items.push_back(array->getItem(i)); items.back().makeDirect(visited, stop_at_streams); diff --git a/libqpdf/QPDF_Array.cc b/libqpdf/QPDF_Array.cc index 6568db96..9368ca51 100644 --- a/libqpdf/QPDF_Array.cc +++ b/libqpdf/QPDF_Array.cc @@ -142,18 +142,6 @@ QPDF_Array::getJSON(int json_version) } } -int -QPDF_Array::getNItems() const -{ - if (sparse) { - // This should really return a size_t, but changing it would break - // a lot of code. - return QIntC::to_int(sp_elements.size()); - } else { - return QIntC::to_int(elements.size()); - } -} - QPDFObjectHandle QPDF_Array::getItem(int n) const { diff --git a/libqpdf/SparseOHArray.cc b/libqpdf/SparseOHArray.cc index 7adba00f..3f6376a6 100644 --- a/libqpdf/SparseOHArray.cc +++ b/libqpdf/SparseOHArray.cc @@ -2,12 +2,6 @@ #include -int -SparseOHArray::size() const -{ - return this->n_elements; -} - void SparseOHArray::append(QPDFObjectHandle oh) { diff --git a/libqpdf/qpdf/QPDF_Array.hh b/libqpdf/qpdf/QPDF_Array.hh index b52efeb6..558704c7 100644 --- a/libqpdf/qpdf/QPDF_Array.hh +++ b/libqpdf/qpdf/QPDF_Array.hh @@ -22,7 +22,11 @@ class QPDF_Array: public QPDFValue virtual JSON getJSON(int json_version); virtual void disconnect(); - int getNItems() const; + int + size() const noexcept + { + return sparse ? sp_elements.size() : int(elements.size()); + } QPDFObjectHandle getItem(int n) const; void getAsVector(std::vector&) const; diff --git a/libqpdf/qpdf/SparseOHArray.hh b/libqpdf/qpdf/SparseOHArray.hh index 191d6915..468e3a39 100644 --- a/libqpdf/qpdf/SparseOHArray.hh +++ b/libqpdf/qpdf/SparseOHArray.hh @@ -11,7 +11,11 @@ class SparseOHArray { public: SparseOHArray() = default; - int size() const; + int + size() const + { + return n_elements; + } void append(QPDFObjectHandle oh); void append(std::shared_ptr&& obj); QPDFObjectHandle at(int idx) const; -- cgit v1.2.3-54-g00ecf From a1a8f35b63bcad7e0cac31a205e109d6a0d70622 Mon Sep 17 00:00:00 2001 From: m-holger Date: Fri, 24 Mar 2023 15:01:40 +0000 Subject: Refactor QPDF_Array::getItem and rename to at --- libqpdf/QPDFObjectHandle.cc | 114 +++++++++++++++++++++--------------------- libqpdf/QPDF_Array.cc | 30 +++++------ libqpdf/SparseOHArray.cc | 14 ++---- libqpdf/qpdf/QPDF_Array.hh | 2 +- libqpdf/qpdf/SparseOHArray.hh | 2 +- 5 files changed, 74 insertions(+), 88 deletions(-) diff --git a/libqpdf/QPDFObjectHandle.cc b/libqpdf/QPDFObjectHandle.cc index 9fd8684b..e08c675d 100644 --- a/libqpdf/QPDFObjectHandle.cc +++ b/libqpdf/QPDFObjectHandle.cc @@ -801,90 +801,88 @@ QPDFObjectHandle::getArrayNItems() QPDFObjectHandle QPDFObjectHandle::getArrayItem(int n) { - auto array = asArray(); - if (array && n < array->size() && n >= 0) { - return array->getItem(n); - } else { - if (array) { + if (auto array = asArray()) { + if (auto result = array->at(n); result.obj != nullptr) { + return result; + } else { objectWarning("returning null for out of bounds array access"); QTC::TC("qpdf", "QPDFObjectHandle array bounds"); - } else { - typeWarning("array", "returning null"); - QTC::TC("qpdf", "QPDFObjectHandle array null for non-array"); } - static auto constexpr msg = - " -> null returned from invalid array access"sv; - return QPDF_Null::create(obj, msg, ""); + } else { + typeWarning("array", "returning null"); + QTC::TC("qpdf", "QPDFObjectHandle array null for non-array"); } + static auto constexpr msg = " -> null returned from invalid array access"sv; + return QPDF_Null::create(obj, msg, ""); } bool QPDFObjectHandle::isRectangle() { - auto array = asArray(); - if (array == nullptr || array->size() != 4) { - return false; - } - for (int i = 0; i < 4; ++i) { - if (!array->getItem(i).isNumber()) { - return false; + if (auto array = asArray()) { + for (int i = 0; i < 4; ++i) { + if (auto item = array->at(i); !(item.obj && item.isNumber())) { + return false; + } } + return array->size() == 4; } - return true; + return false; } bool QPDFObjectHandle::isMatrix() { - auto array = asArray(); - if (array == nullptr || array->size() != 6) { - return false; - } - for (int i = 0; i < 6; ++i) { - if (!array->getItem(i).isNumber()) { - return false; + if (auto array = asArray()) { + for (int i = 0; i < 6; ++i) { + if (auto item = array->at(i); !(item.obj && item.isNumber())) { + return false; + } } + return array->size() == 6; } - return true; + return false; } QPDFObjectHandle::Rectangle QPDFObjectHandle::getArrayAsRectangle() { - Rectangle result; - if (isRectangle()) { - auto array = asArray(); - // Rectangle coordinates are always supposed to be llx, lly, - // urx, ury, but files have been found in the wild where - // llx > urx or lly > ury. - double i0 = array->getItem(0).getNumericValue(); - double i1 = array->getItem(1).getNumericValue(); - double i2 = array->getItem(2).getNumericValue(); - double i3 = array->getItem(3).getNumericValue(); - result = Rectangle( - std::min(i0, i2), - std::min(i1, i3), - std::max(i0, i2), - std::max(i1, i3)); + if (auto array = asArray()) { + if (array->size() != 4) { + return {}; + } + double items[4]; + for (int i = 0; i < 4; ++i) { + if (!array->at(i).getValueAsNumber(items[i])) { + return {}; + } + } + return Rectangle( + std::min(items[0], items[2]), + std::min(items[1], items[3]), + std::max(items[0], items[2]), + std::max(items[1], items[3])); } - return result; + return {}; } QPDFObjectHandle::Matrix QPDFObjectHandle::getArrayAsMatrix() { - Matrix result; - if (isMatrix()) { - auto array = asArray(); - result = Matrix( - array->getItem(0).getNumericValue(), - array->getItem(1).getNumericValue(), - array->getItem(2).getNumericValue(), - array->getItem(3).getNumericValue(), - array->getItem(4).getNumericValue(), - array->getItem(5).getNumericValue()); + if (auto array = asArray()) { + if (array->size() != 6) { + return {}; + } + double items[6]; + for (int i = 0; i < 6; ++i) { + if (!array->at(i).getValueAsNumber(items[i])) { + return {}; + } + } + return Matrix( + items[0], items[1], items[2], items[3], items[4], items[5]); } - return result; + return {}; } std::vector @@ -991,8 +989,8 @@ QPDFObjectHandle QPDFObjectHandle::eraseItemAndGetOld(int at) { auto array = asArray(); - auto result = (array && at < array->size() && at >= 0) ? array->getItem(at) - : newNull(); + auto result = + (array && at < array->size() && at >= 0) ? array->at(at) : newNull(); eraseItem(at); return result; } @@ -1515,7 +1513,7 @@ QPDFObjectHandle::arrayOrStreamToStreamArray( if (auto array = asArray()) { int n_items = array->size(); for (int i = 0; i < n_items; ++i) { - QPDFObjectHandle item = array->getItem(i); + QPDFObjectHandle item = array->at(i); if (item.isStream()) { result.push_back(item); } else { @@ -2215,7 +2213,7 @@ QPDFObjectHandle::makeDirect( auto array = asArray(); int n = array->size(); for (int i = 0; i < n; ++i) { - items.push_back(array->getItem(i)); + items.push_back(array->at(i)); items.back().makeDirect(visited, stop_at_streams); } this->obj = QPDF_Array::create(items); diff --git a/libqpdf/QPDF_Array.cc b/libqpdf/QPDF_Array.cc index 9368ca51..e9d216a5 100644 --- a/libqpdf/QPDF_Array.cc +++ b/libqpdf/QPDF_Array.cc @@ -105,7 +105,7 @@ QPDF_Array::unparse() std::string result = "[ "; int size = sp_elements.size(); for (int i = 0; i < size; ++i) { - result += sp_elements.at(i).unparse(); + result += at(i).unparse(); result += " "; } result += "]"; @@ -114,7 +114,7 @@ QPDF_Array::unparse() std::string result = "[ "; auto size = elements.size(); for (int i = 0; i < int(size); ++i) { - result += getItem(i).unparse(); + result += at(i).unparse(); result += " "; } result += "]"; @@ -129,35 +129,29 @@ QPDF_Array::getJSON(int json_version) JSON j = JSON::makeArray(); int size = sp_elements.size(); for (int i = 0; i < size; ++i) { - j.addArrayElement(sp_elements.at(i).getJSON(json_version)); + j.addArrayElement(at(i).getJSON(json_version)); } return j; } else { JSON j = JSON::makeArray(); size_t size = elements.size(); for (int i = 0; i < int(size); ++i) { - j.addArrayElement(getItem(i).getJSON(json_version)); + j.addArrayElement(at(i).getJSON(json_version)); } return j; } } QPDFObjectHandle -QPDF_Array::getItem(int n) const +QPDF_Array::at(int n) const noexcept { - if (sparse) { - if ((n < 0) || (n >= QIntC::to_int(sp_elements.size()))) { - throw std::logic_error( - "INTERNAL ERROR: bounds error accessing QPDF_Array element"); - } - return sp_elements.at(n); + if (n < 0 || n >= size()) { + return {}; + } else if (sparse) { + auto const& iter = sp_elements.elements.find(n); + return iter == sp_elements.elements.end() ? null_oh : (*iter).second; } else { - if ((n < 0) || (n >= QIntC::to_int(elements.size()))) { - throw std::logic_error( - "INTERNAL ERROR: bounds error accessing QPDF_Array element"); - } - auto const& obj = elements.at(size_t(n)); - return obj ? obj : null_oh; + return elements[size_t(n)]; } } @@ -167,7 +161,7 @@ QPDF_Array::getAsVector(std::vector& v) const if (sparse) { int size = sp_elements.size(); for (int i = 0; i < size; ++i) { - v.push_back(sp_elements.at(i)); + v.push_back(at(i)); } } else { v = std::vector(elements.cbegin(), elements.cend()); diff --git a/libqpdf/SparseOHArray.cc b/libqpdf/SparseOHArray.cc index 3f6376a6..c82bf145 100644 --- a/libqpdf/SparseOHArray.cc +++ b/libqpdf/SparseOHArray.cc @@ -2,6 +2,8 @@ #include +static const QPDFObjectHandle null_oh = QPDFObjectHandle::newNull(); + void SparseOHArray::append(QPDFObjectHandle oh) { @@ -23,16 +25,8 @@ SparseOHArray::append(std::shared_ptr&& obj) QPDFObjectHandle SparseOHArray::at(int idx) const { - if (idx < 0 || idx >= this->n_elements) { - throw std::logic_error( - "INTERNAL ERROR: bounds error accessing SparseOHArray element"); - } - auto const& iter = this->elements.find(idx); - if (iter == this->elements.end()) { - return QPDFObjectHandle::newNull(); - } else { - return (*iter).second; - } + auto const& iter = elements.find(idx); + return iter == elements.end() ? null_oh : (*iter).second; } void diff --git a/libqpdf/qpdf/QPDF_Array.hh b/libqpdf/qpdf/QPDF_Array.hh index 558704c7..4947d09c 100644 --- a/libqpdf/qpdf/QPDF_Array.hh +++ b/libqpdf/qpdf/QPDF_Array.hh @@ -27,7 +27,7 @@ class QPDF_Array: public QPDFValue { return sparse ? sp_elements.size() : int(elements.size()); } - QPDFObjectHandle getItem(int n) const; + QPDFObjectHandle at(int n) const noexcept; void getAsVector(std::vector&) const; void setItem(int, QPDFObjectHandle const&); diff --git a/libqpdf/qpdf/SparseOHArray.hh b/libqpdf/qpdf/SparseOHArray.hh index 468e3a39..80de95e5 100644 --- a/libqpdf/qpdf/SparseOHArray.hh +++ b/libqpdf/qpdf/SparseOHArray.hh @@ -12,7 +12,7 @@ class SparseOHArray public: SparseOHArray() = default; int - size() const + size() const noexcept { return n_elements; } -- cgit v1.2.3-54-g00ecf From c6179da9615057a14e74180f640e2e77fdcbf234 Mon Sep 17 00:00:00 2001 From: m-holger Date: Thu, 17 Nov 2022 17:35:13 +0000 Subject: Add new method QPDFValue::checkOwnership --- libqpdf/QPDF_Array.cc | 20 ++++++++++++++++++++ libqpdf/qpdf/QPDF_Array.hh | 3 +++ 2 files changed, 23 insertions(+) diff --git a/libqpdf/QPDF_Array.cc b/libqpdf/QPDF_Array.cc index e9d216a5..def40e9b 100644 --- a/libqpdf/QPDF_Array.cc +++ b/libqpdf/QPDF_Array.cc @@ -7,6 +7,26 @@ static const QPDFObjectHandle null_oh = QPDFObjectHandle::newNull(); +inline void +QPDF_Array::checkOwnership(QPDFObjectHandle const& item) const +{ + if (auto obj = item.getObjectPtr()) { + if (qpdf) { + if (auto item_qpdf = obj->getQPDF()) { + if (qpdf != item_qpdf) { + throw std::logic_error( + "Attempting to add an object from a different QPDF. " + "Use QPDF::copyForeignObject to add objects from " + "another file."); + } + } + } + } else { + throw std::logic_error( + "Attempting to add an uninitialized object to a QPDF_Array."); + } +} + QPDF_Array::QPDF_Array(std::vector const& v) : QPDFValue(::ot_array, "array") { diff --git a/libqpdf/qpdf/QPDF_Array.hh b/libqpdf/qpdf/QPDF_Array.hh index 4947d09c..b27ce122 100644 --- a/libqpdf/qpdf/QPDF_Array.hh +++ b/libqpdf/qpdf/QPDF_Array.hh @@ -42,6 +42,9 @@ class QPDF_Array: public QPDFValue QPDF_Array(std::vector>&& items, bool sparse); QPDF_Array(SparseOHArray const& items); QPDF_Array(std::vector> const& items); + + void checkOwnership(QPDFObjectHandle const& item) const; + bool sparse{false}; SparseOHArray sp_elements; std::vector> elements; -- cgit v1.2.3-54-g00ecf From cedb37caa153abfd92a91b2e39a6f32601be826c Mon Sep 17 00:00:00 2001 From: m-holger Date: Sat, 10 Dec 2022 19:05:12 +0000 Subject: Refactor QPDF_Array::appendItem and rename to push_back --- libqpdf/QPDFObjectHandle.cc | 6 ++---- libqpdf/QPDF_Array.cc | 5 +++-- libqpdf/SparseOHArray.cc | 18 ------------------ libqpdf/qpdf/QPDF_Array.hh | 2 +- libqpdf/qpdf/SparseOHArray.hh | 12 ++++++++++-- 5 files changed, 16 insertions(+), 27 deletions(-) diff --git a/libqpdf/QPDFObjectHandle.cc b/libqpdf/QPDFObjectHandle.cc index e08c675d..0c4d45ae 100644 --- a/libqpdf/QPDFObjectHandle.cc +++ b/libqpdf/QPDFObjectHandle.cc @@ -951,10 +951,8 @@ QPDFObjectHandle::insertItemAndGetNew(int at, QPDFObjectHandle const& item) void QPDFObjectHandle::appendItem(QPDFObjectHandle const& item) { - auto array = asArray(); - if (array) { - checkOwnership(item); - array->appendItem(item); + if (auto array = asArray()) { + array->push_back(item); } else { typeWarning("array", "ignoring attempt to append item"); QTC::TC("qpdf", "QPDFObjectHandle array ignoring append item"); diff --git a/libqpdf/QPDF_Array.cc b/libqpdf/QPDF_Array.cc index def40e9b..5a80cdf5 100644 --- a/libqpdf/QPDF_Array.cc +++ b/libqpdf/QPDF_Array.cc @@ -264,10 +264,11 @@ QPDF_Array::insertItem(int at, QPDFObjectHandle const& item) } void -QPDF_Array::appendItem(QPDFObjectHandle const& item) +QPDF_Array::push_back(QPDFObjectHandle const& item) { + checkOwnership(item); if (sparse) { - sp_elements.append(item); + sp_elements.elements[sp_elements.n_elements++] = item.getObj(); } else { elements.push_back(item.getObj()); } diff --git a/libqpdf/SparseOHArray.cc b/libqpdf/SparseOHArray.cc index c82bf145..48716deb 100644 --- a/libqpdf/SparseOHArray.cc +++ b/libqpdf/SparseOHArray.cc @@ -4,24 +4,6 @@ static const QPDFObjectHandle null_oh = QPDFObjectHandle::newNull(); -void -SparseOHArray::append(QPDFObjectHandle oh) -{ - if (!oh.isDirectNull()) { - this->elements[this->n_elements] = oh.getObj(); - } - ++this->n_elements; -} - -void -SparseOHArray::append(std::shared_ptr&& obj) -{ - if (obj->getTypeCode() != ::ot_null || !obj->getObjGen().isIndirect()) { - this->elements[this->n_elements] = std::move(obj); - } - ++this->n_elements; -} - QPDFObjectHandle SparseOHArray::at(int idx) const { diff --git a/libqpdf/qpdf/QPDF_Array.hh b/libqpdf/qpdf/QPDF_Array.hh index b27ce122..61f14d0f 100644 --- a/libqpdf/qpdf/QPDF_Array.hh +++ b/libqpdf/qpdf/QPDF_Array.hh @@ -34,7 +34,7 @@ class QPDF_Array: public QPDFValue void setFromVector(std::vector const& items); void setFromVector(std::vector>&& items); void insertItem(int at, QPDFObjectHandle const& item); - void appendItem(QPDFObjectHandle const& item); + void push_back(QPDFObjectHandle const& item); void eraseItem(int at); private: diff --git a/libqpdf/qpdf/SparseOHArray.hh b/libqpdf/qpdf/SparseOHArray.hh index 80de95e5..f2872618 100644 --- a/libqpdf/qpdf/SparseOHArray.hh +++ b/libqpdf/qpdf/SparseOHArray.hh @@ -16,8 +16,16 @@ class SparseOHArray { return n_elements; } - void append(QPDFObjectHandle oh); - void append(std::shared_ptr&& obj); + void + append(QPDFObjectHandle oh) + { + elements[n_elements++] = oh.getObj(); + } + void + append(std::shared_ptr&& obj) + { + elements[n_elements++] = std::move(obj); + } QPDFObjectHandle at(int idx) const; void remove_last(); void setAt(int idx, QPDFObjectHandle oh); -- cgit v1.2.3-54-g00ecf From 1bb23d0545dfe2d651cb22b6135d99c1c9ef85d5 Mon Sep 17 00:00:00 2001 From: m-holger Date: Sat, 10 Dec 2022 19:48:07 +0000 Subject: Refactor QPDF_Array::insertItem and rename to insert --- libqpdf/QPDFObjectHandle.cc | 9 ++++++--- libqpdf/QPDF_Array.cc | 31 ++++++++++++------------------- libqpdf/SparseOHArray.cc | 22 +++++++++++----------- libqpdf/qpdf/QPDF_Array.hh | 2 +- qpdf/qpdf.testcov | 1 + qpdf/qtest/qpdf/object-types-os.out | 1 + qpdf/qtest/qpdf/object-types.out | 1 + qpdf/test_driver.cc | 1 + 8 files changed, 34 insertions(+), 34 deletions(-) diff --git a/libqpdf/QPDFObjectHandle.cc b/libqpdf/QPDFObjectHandle.cc index 0c4d45ae..fa8b9136 100644 --- a/libqpdf/QPDFObjectHandle.cc +++ b/libqpdf/QPDFObjectHandle.cc @@ -932,9 +932,12 @@ QPDFObjectHandle::setArrayFromVector(std::vector const& items) void QPDFObjectHandle::insertItem(int at, QPDFObjectHandle const& item) { - auto array = asArray(); - if (array) { - array->insertItem(at, item); + if (auto array = asArray()) { + if (!array->insert(at, item)) { + objectWarning( + "ignoring attempt to insert out of bounds array item"); + QTC::TC("qpdf", "QPDFObjectHandle insert array bounds"); + } } else { typeWarning("array", "ignoring attempt to insert item"); QTC::TC("qpdf", "QPDFObjectHandle array ignoring insert item"); diff --git a/libqpdf/QPDF_Array.cc b/libqpdf/QPDF_Array.cc index 5a80cdf5..7633266e 100644 --- a/libqpdf/QPDF_Array.cc +++ b/libqpdf/QPDF_Array.cc @@ -236,31 +236,24 @@ QPDF_Array::setFromVector(std::vector>&& v) } } -void -QPDF_Array::insertItem(int at, QPDFObjectHandle const& item) +bool +QPDF_Array::insert(int at, QPDFObjectHandle const& item) { - if (sparse) { + int sz = size(); + if (at < 0 || at > sz) { // As special case, also allow insert beyond the end - if ((at < 0) || (at > sp_elements.size())) { - throw std::logic_error( - "INTERNAL ERROR: bounds error accessing QPDF_Array element"); - } - sp_elements.insert(at, item); + return false; + } else if (at == sz) { + push_back(item); } else { - // As special case, also allow insert beyond the end - size_t idx = QIntC::to_size(at); - if ((at < 0) || (at > QIntC::to_int(elements.size()))) { - throw std::logic_error( - "INTERNAL ERROR: bounds error accessing QPDF_Array element"); - } - if (idx == elements.size()) { - // Allow inserting to the last position - elements.push_back(item.getObj()); + checkOwnership(item); + if (sparse) { + sp_elements.insert(at, item); } else { - int n = int(idx); - elements.insert(elements.cbegin() + n, item.getObj()); + elements.insert(elements.cbegin() + at, item.getObj()); } } + return true; } void diff --git a/libqpdf/SparseOHArray.cc b/libqpdf/SparseOHArray.cc index 48716deb..8f6c02d7 100644 --- a/libqpdf/SparseOHArray.cc +++ b/libqpdf/SparseOHArray.cc @@ -64,23 +64,23 @@ SparseOHArray::erase(int idx) void SparseOHArray::insert(int idx, QPDFObjectHandle oh) { - if (idx > this->n_elements) { - throw std::logic_error("bounds error inserting item to SparseOHArray"); - } else if (idx == this->n_elements) { + if (idx == n_elements) { // Allow inserting to the last position append(oh); } else { - decltype(this->elements) dest; - for (auto const& iter: this->elements) { - if (iter.first < idx) { - dest.insert(iter); + auto iter = elements.crbegin(); + while (iter != elements.crend()) { + auto key = (iter++)->first; + if (key >= idx) { + auto nh = elements.extract(key); + ++nh.key(); + elements.insert(std::move(nh)); } else { - dest[iter.first + 1] = iter.second; + break; } } - this->elements = dest; - this->elements[idx] = oh.getObj(); - ++this->n_elements; + elements[idx] = oh.getObj(); + ++n_elements; } } diff --git a/libqpdf/qpdf/QPDF_Array.hh b/libqpdf/qpdf/QPDF_Array.hh index 61f14d0f..8768c9ad 100644 --- a/libqpdf/qpdf/QPDF_Array.hh +++ b/libqpdf/qpdf/QPDF_Array.hh @@ -33,7 +33,7 @@ class QPDF_Array: public QPDFValue void setItem(int, QPDFObjectHandle const&); void setFromVector(std::vector const& items); void setFromVector(std::vector>&& items); - void insertItem(int at, QPDFObjectHandle const& item); + bool insert(int at, QPDFObjectHandle const& item); void push_back(QPDFObjectHandle const& item); void eraseItem(int at); diff --git a/qpdf/qpdf.testcov b/qpdf/qpdf.testcov index cad67565..736c02b4 100644 --- a/qpdf/qpdf.testcov +++ b/qpdf/qpdf.testcov @@ -305,6 +305,7 @@ QPDFObjectHandle array treating as empty vector 0 QPDFObjectHandle array ignoring set item 0 QPDFObjectHandle array ignoring replace items 0 QPDFObjectHandle array ignoring insert item 0 +QPDFObjectHandle insert array bounds 0 QPDFObjectHandle array ignoring append item 0 QPDFObjectHandle array ignoring erase item 0 QPDFObjectHandle dictionary false for hasKey 0 diff --git a/qpdf/qtest/qpdf/object-types-os.out b/qpdf/qtest/qpdf/object-types-os.out index a95fa4d8..1ba84d50 100644 --- a/qpdf/qtest/qpdf/object-types-os.out +++ b/qpdf/qtest/qpdf/object-types-os.out @@ -5,6 +5,7 @@ WARNING: object-types-os.pdf object stream 1, object 7 0 at offset 429: operatio WARNING: object-types-os.pdf object stream 1, object 7 0 at offset 429: operation for array attempted on object of type integer: ignoring attempt to append item WARNING: object-types-os.pdf object stream 1, object 7 0 at offset 384: ignoring attempt to erase out of bounds array item WARNING: object-types-os.pdf object stream 1, object 7 0 at offset 384: ignoring attempt to erase out of bounds array item +WARNING: object-types-os.pdf object stream 1, object 7 0 at offset 384: ignoring attempt to insert out of bounds array item WARNING: object-types-os.pdf object stream 1, object 7 0 at offset 429: operation for array attempted on object of type integer: ignoring attempt to erase item WARNING: object-types-os.pdf object stream 1, object 7 0 at offset 429: operation for array attempted on object of type integer: ignoring attempt to insert item WARNING: object-types-os.pdf object stream 1, object 7 0 at offset 429: operation for array attempted on object of type integer: ignoring attempt to replace items diff --git a/qpdf/qtest/qpdf/object-types.out b/qpdf/qtest/qpdf/object-types.out index 718105db..bf41044e 100644 --- a/qpdf/qtest/qpdf/object-types.out +++ b/qpdf/qtest/qpdf/object-types.out @@ -5,6 +5,7 @@ WARNING: object-types.pdf, object 8 0 at offset 669: operation for array attempt WARNING: object-types.pdf, object 8 0 at offset 669: operation for array attempted on object of type integer: ignoring attempt to append item WARNING: object-types.pdf, object 8 0 at offset 717: ignoring attempt to erase out of bounds array item WARNING: object-types.pdf, object 8 0 at offset 717: ignoring attempt to erase out of bounds array item +WARNING: object-types.pdf, object 8 0 at offset 717: ignoring attempt to insert out of bounds array item WARNING: object-types.pdf, object 8 0 at offset 669: operation for array attempted on object of type integer: ignoring attempt to erase item WARNING: object-types.pdf, object 8 0 at offset 669: operation for array attempted on object of type integer: ignoring attempt to insert item WARNING: object-types.pdf, object 8 0 at offset 669: operation for array attempted on object of type integer: ignoring attempt to replace items diff --git a/qpdf/test_driver.cc b/qpdf/test_driver.cc index 5bae8b54..8c54ad01 100644 --- a/qpdf/test_driver.cc +++ b/qpdf/test_driver.cc @@ -1506,6 +1506,7 @@ test_42(QPDF& pdf, char const* arg2) integer.appendItem(null); array.eraseItem(-1); array.eraseItem(16059); + array.insertItem(42, "/Dontpanic"_qpdf); integer.eraseItem(0); integer.insertItem(0, null); integer.setArrayFromVector(std::vector()); -- cgit v1.2.3-54-g00ecf From 4d37389befc705b671d8fa7a1da2b7117b50f454 Mon Sep 17 00:00:00 2001 From: m-holger Date: Mon, 12 Dec 2022 13:29:52 +0000 Subject: Refactor QPDF_Array::eraseItem and rename to erase --- libqpdf/QPDFObjectHandle.cc | 13 +++++-------- libqpdf/QPDF_Array.cc | 15 +++++++-------- libqpdf/SparseOHArray.cc | 25 +++++++++++++------------ libqpdf/qpdf/QPDF_Array.hh | 2 +- qpdf/qtest/qpdf/test88.out | 1 + qpdf/test_driver.cc | 1 + 6 files changed, 28 insertions(+), 29 deletions(-) diff --git a/libqpdf/QPDFObjectHandle.cc b/libqpdf/QPDFObjectHandle.cc index fa8b9136..e113089a 100644 --- a/libqpdf/QPDFObjectHandle.cc +++ b/libqpdf/QPDFObjectHandle.cc @@ -972,17 +972,14 @@ 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"); } + } else { + typeWarning("array", "ignoring attempt to erase item"); + QTC::TC("qpdf", "QPDFObjectHandle array ignoring erase item"); } } diff --git a/libqpdf/QPDF_Array.cc b/libqpdf/QPDF_Array.cc index 7633266e..e4e1668e 100644 --- a/libqpdf/QPDF_Array.cc +++ b/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; } diff --git a/libqpdf/SparseOHArray.cc b/libqpdf/SparseOHArray.cc index 8f6c02d7..773d7309 100644 --- a/libqpdf/SparseOHArray.cc +++ b/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"); - } - 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; + auto end = elements.end(); + if (auto iter = elements.lower_bound(at); iter != end) { + if (iter->first == at) { + iter++; + elements.erase(at); + } + + while (iter != end) { + auto nh = elements.extract(iter++); + --nh.key(); + elements.insert(std::move(nh)); } } - this->elements = dest; - --this->n_elements; + --n_elements; } void diff --git a/libqpdf/qpdf/QPDF_Array.hh b/libqpdf/qpdf/QPDF_Array.hh index 8768c9ad..edda54ec 100644 --- a/libqpdf/qpdf/QPDF_Array.hh +++ b/libqpdf/qpdf/QPDF_Array.hh @@ -35,7 +35,7 @@ class QPDF_Array: public QPDFValue void setFromVector(std::vector>&& 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 const& items); diff --git a/qpdf/qtest/qpdf/test88.out b/qpdf/qtest/qpdf/test88.out index 327212b6..9d5cf504 100644 --- a/qpdf/qtest/qpdf/test88.out +++ b/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 diff --git a/qpdf/test_driver.cc b/qpdf/test_driver.cc index 8c54ad01..82361ba0 100644 --- a/qpdf/test_driver.cc +++ b/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 -- cgit v1.2.3-54-g00ecf From 182c2480df3d2dee3e430e02afe6bbee3e45eadd Mon Sep 17 00:00:00 2001 From: m-holger Date: Sun, 1 Jan 2023 12:47:03 +0000 Subject: Refactor QPDF_Array::setItem and rename to setAt --- libqpdf/QPDFObjectHandle.cc | 10 +++++----- libqpdf/QPDF_Array.cc | 20 +++++++++----------- libqpdf/SparseOHArray.cc | 13 ------------- libqpdf/qpdf/QPDF_Array.hh | 3 +-- libqpdf/qpdf/SparseOHArray.hh | 6 +++++- qpdf/qpdf.testcov | 1 + qpdf/qtest/qpdf/object-types-os.out | 1 + qpdf/qtest/qpdf/object-types.out | 1 + qpdf/test_driver.cc | 1 + 9 files changed, 24 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 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 -#include #include -#include -#include static const QPDFObjectHandle null_oh = QPDFObjectHandle::newNull(); @@ -188,18 +185,19 @@ QPDF_Array::getAsVector(std::vector& 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 @@ -30,19 +30,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) { 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&) const; - - void setItem(int, QPDFObjectHandle const&); void setFromVector(std::vector const& items); void setFromVector(std::vector>&& 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(); diff --git a/qpdf/qpdf.testcov b/qpdf/qpdf.testcov index 736c02b4..014ea571 100644 --- a/qpdf/qpdf.testcov +++ b/qpdf/qpdf.testcov @@ -303,6 +303,7 @@ QPDFObjectHandle array treating as empty 0 QPDFObjectHandle array null for non-array 0 QPDFObjectHandle array treating as empty vector 0 QPDFObjectHandle array ignoring set item 0 +QPDFObjectHandle set array bounds 0 QPDFObjectHandle array ignoring replace items 0 QPDFObjectHandle array ignoring insert item 0 QPDFObjectHandle insert array bounds 0 diff --git a/qpdf/qtest/qpdf/object-types-os.out b/qpdf/qtest/qpdf/object-types-os.out index 1ba84d50..4b02d156 100644 --- a/qpdf/qtest/qpdf/object-types-os.out +++ b/qpdf/qtest/qpdf/object-types-os.out @@ -6,6 +6,7 @@ WARNING: object-types-os.pdf object stream 1, object 7 0 at offset 429: operatio WARNING: object-types-os.pdf object stream 1, object 7 0 at offset 384: ignoring attempt to erase out of bounds array item WARNING: object-types-os.pdf object stream 1, object 7 0 at offset 384: ignoring attempt to erase out of bounds array item WARNING: object-types-os.pdf object stream 1, object 7 0 at offset 384: ignoring attempt to insert out of bounds array item +WARNING: object-types-os.pdf object stream 1, object 7 0 at offset 384: ignoring attempt to set out of bounds array item WARNING: object-types-os.pdf object stream 1, object 7 0 at offset 429: operation for array attempted on object of type integer: ignoring attempt to erase item WARNING: object-types-os.pdf object stream 1, object 7 0 at offset 429: operation for array attempted on object of type integer: ignoring attempt to insert item WARNING: object-types-os.pdf object stream 1, object 7 0 at offset 429: operation for array attempted on object of type integer: ignoring attempt to replace items diff --git a/qpdf/qtest/qpdf/object-types.out b/qpdf/qtest/qpdf/object-types.out index bf41044e..b7089b6b 100644 --- a/qpdf/qtest/qpdf/object-types.out +++ b/qpdf/qtest/qpdf/object-types.out @@ -6,6 +6,7 @@ WARNING: object-types.pdf, object 8 0 at offset 669: operation for array attempt WARNING: object-types.pdf, object 8 0 at offset 717: ignoring attempt to erase out of bounds array item WARNING: object-types.pdf, object 8 0 at offset 717: ignoring attempt to erase out of bounds array item WARNING: object-types.pdf, object 8 0 at offset 717: ignoring attempt to insert out of bounds array item +WARNING: object-types.pdf, object 8 0 at offset 717: ignoring attempt to set out of bounds array item WARNING: object-types.pdf, object 8 0 at offset 669: operation for array attempted on object of type integer: ignoring attempt to erase item WARNING: object-types.pdf, object 8 0 at offset 669: operation for array attempted on object of type integer: ignoring attempt to insert item WARNING: object-types.pdf, object 8 0 at offset 669: operation for array attempted on object of type integer: ignoring attempt to replace items diff --git a/qpdf/test_driver.cc b/qpdf/test_driver.cc index 82361ba0..4a5d0ae8 100644 --- a/qpdf/test_driver.cc +++ b/qpdf/test_driver.cc @@ -1507,6 +1507,7 @@ test_42(QPDF& pdf, char const* arg2) array.eraseItem(-1); array.eraseItem(16059); array.insertItem(42, "/Dontpanic"_qpdf); + array.setArrayItem(42, "/Dontpanic"_qpdf); integer.eraseItem(0); integer.insertItem(0, null); integer.setArrayFromVector(std::vector()); -- cgit v1.2.3-54-g00ecf From 73023bcb5d0705a8e914ae7586aa2f417a82d9a7 Mon Sep 17 00:00:00 2001 From: m-holger Date: Tue, 28 Mar 2023 10:25:10 +0100 Subject: Change sparse_array test to test sparse QPDF_Arrays --- libtests/sparse_array.cc | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/libtests/sparse_array.cc b/libtests/sparse_array.cc index 62410399..a970b199 100644 --- a/libtests/sparse_array.cc +++ b/libtests/sparse_array.cc @@ -1,19 +1,22 @@ #include -#include +#include +#include #include int main() { - SparseOHArray a; + auto obj = QPDF_Array::create({}, true); + QPDF_Array& a = *obj->as(); + assert(a.size() == 0); - a.append(QPDFObjectHandle::parse("1")); - a.append(QPDFObjectHandle::parse("(potato)")); - a.append(QPDFObjectHandle::parse("null")); - a.append(QPDFObjectHandle::parse("null")); - a.append(QPDFObjectHandle::parse("/Quack")); + a.push_back(QPDFObjectHandle::parse("1")); + a.push_back(QPDFObjectHandle::parse("(potato)")); + a.push_back(QPDFObjectHandle::parse("null")); + a.push_back(QPDFObjectHandle::parse("null")); + a.push_back(QPDFObjectHandle::parse("/Quack")); assert(a.size() == 5); assert(a.at(0).isInteger() && (a.at(0).getIntValue() == 1)); assert(a.at(1).isString() && (a.at(1).getStringValue() == "potato")); @@ -60,20 +63,20 @@ main() a.setAt(4, QPDFObjectHandle::newNull()); assert(a.at(4).isNull()); - a.remove_last(); + a.erase(a.size() - 1); assert(a.size() == 5); assert(a.at(0).isName() && (a.at(0).getName() == "/First")); assert(a.at(1).isInteger() && (a.at(1).getIntValue() == 1)); assert(a.at(3).isName() && (a.at(3).getName() == "/Third")); assert(a.at(4).isNull()); - a.remove_last(); + a.erase(a.size() - 1); assert(a.size() == 4); assert(a.at(0).isName() && (a.at(0).getName() == "/First")); assert(a.at(1).isInteger() && (a.at(1).getIntValue() == 1)); assert(a.at(3).isName() && (a.at(3).getName() == "/Third")); - a.remove_last(); + a.erase(a.size() - 1); assert(a.size() == 3); assert(a.at(0).isName() && (a.at(0).getName() == "/First")); assert(a.at(1).isInteger() && (a.at(1).getIntValue() == 1)); -- cgit v1.2.3-54-g00ecf From 6295da436f77ef0e1016dafbb4172452db65ac0b Mon Sep 17 00:00:00 2001 From: m-holger Date: Wed, 29 Mar 2023 15:45:12 +0100 Subject: Remove SparseOHArray::insert --- libqpdf/QPDF_Array.cc | 14 +++++++++++++- libqpdf/SparseOHArray.cc | 23 ----------------------- libqpdf/qpdf/SparseOHArray.hh | 1 - 3 files changed, 13 insertions(+), 25 deletions(-) diff --git a/libqpdf/QPDF_Array.cc b/libqpdf/QPDF_Array.cc index d7505468..320a1ab7 100644 --- a/libqpdf/QPDF_Array.cc +++ b/libqpdf/QPDF_Array.cc @@ -246,7 +246,19 @@ QPDF_Array::insert(int at, QPDFObjectHandle const& item) } else { checkOwnership(item); if (sparse) { - sp_elements.insert(at, item); + auto iter = sp_elements.elements.crbegin(); + while (iter != sp_elements.elements.crend()) { + auto key = (iter++)->first; + if (key >= at) { + auto nh = sp_elements.elements.extract(key); + ++nh.key(); + sp_elements.elements.insert(std::move(nh)); + } else { + break; + } + } + sp_elements.elements[at] = item.getObj(); + ++sp_elements.n_elements; } else { elements.insert(elements.cbegin() + at, item.getObj()); } diff --git a/libqpdf/SparseOHArray.cc b/libqpdf/SparseOHArray.cc index 9904af1b..390f942a 100644 --- a/libqpdf/SparseOHArray.cc +++ b/libqpdf/SparseOHArray.cc @@ -49,29 +49,6 @@ SparseOHArray::erase(int at) --n_elements; } -void -SparseOHArray::insert(int idx, QPDFObjectHandle oh) -{ - if (idx == n_elements) { - // Allow inserting to the last position - append(oh); - } else { - auto iter = elements.crbegin(); - while (iter != elements.crend()) { - auto key = (iter++)->first; - if (key >= idx) { - auto nh = elements.extract(key); - ++nh.key(); - elements.insert(std::move(nh)); - } else { - break; - } - } - elements[idx] = oh.getObj(); - ++n_elements; - } -} - SparseOHArray SparseOHArray::copy() { diff --git a/libqpdf/qpdf/SparseOHArray.hh b/libqpdf/qpdf/SparseOHArray.hh index 19aa49f0..8adffb30 100644 --- a/libqpdf/qpdf/SparseOHArray.hh +++ b/libqpdf/qpdf/SparseOHArray.hh @@ -34,7 +34,6 @@ class SparseOHArray elements[idx] = oh.getObj(); } void erase(int idx); - void insert(int idx, QPDFObjectHandle oh); SparseOHArray copy(); void disconnect(); -- cgit v1.2.3-54-g00ecf From 1c85e7ece4d832b114f91c1858ba2b24964d5d9e Mon Sep 17 00:00:00 2001 From: m-holger Date: Wed, 29 Mar 2023 15:53:34 +0100 Subject: Remove SparseOHArray::erase --- libqpdf/QPDF_Array.cc | 15 ++++++++++++++- libqpdf/SparseOHArray.cc | 19 ------------------- libqpdf/qpdf/SparseOHArray.hh | 1 - 3 files changed, 14 insertions(+), 21 deletions(-) diff --git a/libqpdf/QPDF_Array.cc b/libqpdf/QPDF_Array.cc index 320a1ab7..fbaf776f 100644 --- a/libqpdf/QPDF_Array.cc +++ b/libqpdf/QPDF_Array.cc @@ -284,7 +284,20 @@ QPDF_Array::erase(int at) return false; } if (sparse) { - sp_elements.erase(at); + auto end = sp_elements.elements.end(); + if (auto iter = sp_elements.elements.lower_bound(at); iter != end) { + if (iter->first == at) { + iter++; + sp_elements.elements.erase(at); + } + + while (iter != end) { + auto nh = sp_elements.elements.extract(iter++); + --nh.key(); + sp_elements.elements.insert(std::move(nh)); + } + } + --sp_elements.n_elements; } else { elements.erase(elements.cbegin() + at); } diff --git a/libqpdf/SparseOHArray.cc b/libqpdf/SparseOHArray.cc index 390f942a..e0c2d41d 100644 --- a/libqpdf/SparseOHArray.cc +++ b/libqpdf/SparseOHArray.cc @@ -30,25 +30,6 @@ SparseOHArray::disconnect() } } -void -SparseOHArray::erase(int at) -{ - auto end = elements.end(); - if (auto iter = elements.lower_bound(at); iter != end) { - if (iter->first == at) { - iter++; - elements.erase(at); - } - - while (iter != end) { - auto nh = elements.extract(iter++); - --nh.key(); - elements.insert(std::move(nh)); - } - } - --n_elements; -} - SparseOHArray SparseOHArray::copy() { diff --git a/libqpdf/qpdf/SparseOHArray.hh b/libqpdf/qpdf/SparseOHArray.hh index 8adffb30..f1a1011e 100644 --- a/libqpdf/qpdf/SparseOHArray.hh +++ b/libqpdf/qpdf/SparseOHArray.hh @@ -33,7 +33,6 @@ class SparseOHArray { elements[idx] = oh.getObj(); } - void erase(int idx); SparseOHArray copy(); void disconnect(); -- cgit v1.2.3-54-g00ecf From e186da17213826ae22b73584641d2e8cd10b24d5 Mon Sep 17 00:00:00 2001 From: m-holger Date: Wed, 29 Mar 2023 16:39:34 +0100 Subject: Remove SparseOHArray::at, setAt and append --- libqpdf/QPDF_Array.cc | 9 ++++----- libqpdf/SparseOHArray.cc | 34 ---------------------------------- libqpdf/qpdf/SparseOHArray.hh | 22 ---------------------- 3 files changed, 4 insertions(+), 61 deletions(-) diff --git a/libqpdf/QPDF_Array.cc b/libqpdf/QPDF_Array.cc index fbaf776f..3ed18bf4 100644 --- a/libqpdf/QPDF_Array.cc +++ b/libqpdf/QPDF_Array.cc @@ -193,7 +193,7 @@ QPDF_Array::setAt(int at, QPDFObjectHandle const& oh) } checkOwnership(oh); if (sparse) { - sp_elements.setAt(at, oh); + sp_elements.elements[at] = oh.getObj(); } else { elements[size_t(at)] = oh.getObj(); } @@ -206,7 +206,7 @@ QPDF_Array::setFromVector(std::vector const& v) if (sparse) { sp_elements = SparseOHArray(); for (auto const& iter: v) { - sp_elements.append(iter); + sp_elements.elements[sp_elements.n_elements++] = iter.getObj(); } } else { elements.resize(0); @@ -224,10 +224,9 @@ QPDF_Array::setFromVector(std::vector>&& v) for (auto&& item: v) { if (item->getTypeCode() != ::ot_null || item->getObjGen().isIndirect()) { - sp_elements.append(std::move(item)); - } else { - ++sp_elements.n_elements; + sp_elements.elements[sp_elements.n_elements] = std::move(item); } + ++sp_elements.n_elements; } } else { elements = std::move(v); diff --git a/libqpdf/SparseOHArray.cc b/libqpdf/SparseOHArray.cc index e0c2d41d..6a1c976a 100644 --- a/libqpdf/SparseOHArray.cc +++ b/libqpdf/SparseOHArray.cc @@ -1,27 +1,5 @@ #include -#include - -static const QPDFObjectHandle null_oh = QPDFObjectHandle::newNull(); - -QPDFObjectHandle -SparseOHArray::at(int idx) const -{ - auto const& iter = elements.find(idx); - return iter == elements.end() ? null_oh : (*iter).second; -} - -void -SparseOHArray::remove_last() -{ - if (this->n_elements == 0) { - throw std::logic_error("INTERNAL ERROR: attempt to remove" - " last item from empty SparseOHArray"); - } - --this->n_elements; - this->elements.erase(this->n_elements); -} - void SparseOHArray::disconnect() { @@ -42,15 +20,3 @@ SparseOHArray::copy() } return result; } - -SparseOHArray::const_iterator -SparseOHArray::begin() const -{ - return this->elements.begin(); -} - -SparseOHArray::const_iterator -SparseOHArray::end() const -{ - return this->elements.end(); -} diff --git a/libqpdf/qpdf/SparseOHArray.hh b/libqpdf/qpdf/SparseOHArray.hh index f1a1011e..1b31f266 100644 --- a/libqpdf/qpdf/SparseOHArray.hh +++ b/libqpdf/qpdf/SparseOHArray.hh @@ -16,31 +16,9 @@ class SparseOHArray { return n_elements; } - void - append(QPDFObjectHandle oh) - { - elements[n_elements++] = oh.getObj(); - } - void - append(std::shared_ptr&& obj) - { - elements[n_elements++] = std::move(obj); - } - QPDFObjectHandle at(int idx) const; - void remove_last(); - void - setAt(int idx, QPDFObjectHandle oh) - { - elements[idx] = oh.getObj(); - } SparseOHArray copy(); void disconnect(); - typedef std::map>::const_iterator - const_iterator; - const_iterator begin() const; - const_iterator end() const; - private: friend class QPDF_Array; std::map> elements; -- cgit v1.2.3-54-g00ecf From 5072238867f37f6c6ecd53dab06e42ea2763cf56 Mon Sep 17 00:00:00 2001 From: m-holger Date: Mon, 2 Jan 2023 19:49:42 +0000 Subject: Refactor QPDF_Array::getAsVector --- libqpdf/QPDFObjectHandle.cc | 5 ++--- libqpdf/QPDF_Array.cc | 16 ++++++++++------ libqpdf/qpdf/QPDF_Array.hh | 2 +- 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/libqpdf/QPDFObjectHandle.cc b/libqpdf/QPDFObjectHandle.cc index 396e2063..20be3f9d 100644 --- a/libqpdf/QPDFObjectHandle.cc +++ b/libqpdf/QPDFObjectHandle.cc @@ -888,15 +888,14 @@ QPDFObjectHandle::getArrayAsMatrix() std::vector QPDFObjectHandle::getArrayAsVector() { - std::vector result; auto array = asArray(); if (array) { - array->getAsVector(result); + return array->getAsVector(); } else { typeWarning("array", "treating as empty"); QTC::TC("qpdf", "QPDFObjectHandle array treating as empty vector"); } - return result; + return {}; } // Array mutators diff --git a/libqpdf/QPDF_Array.cc b/libqpdf/QPDF_Array.cc index 3ed18bf4..9593c31f 100644 --- a/libqpdf/QPDF_Array.cc +++ b/libqpdf/QPDF_Array.cc @@ -172,16 +172,20 @@ QPDF_Array::at(int n) const noexcept } } -void -QPDF_Array::getAsVector(std::vector& v) const +std::vector +QPDF_Array::getAsVector() const { if (sparse) { - int size = sp_elements.size(); - for (int i = 0; i < size; ++i) { - v.push_back(at(i)); + std::vector v; + v.reserve(size_t(size())); + for (auto const& item: sp_elements.elements) { + v.resize(size_t(item.first), null_oh); + v.push_back(item.second); } + v.resize(size_t(size()), null_oh); + return v; } else { - v = std::vector(elements.cbegin(), elements.cend()); + return {elements.cbegin(), elements.cend()}; } } diff --git a/libqpdf/qpdf/QPDF_Array.hh b/libqpdf/qpdf/QPDF_Array.hh index f4d1fc3d..df814d74 100644 --- a/libqpdf/qpdf/QPDF_Array.hh +++ b/libqpdf/qpdf/QPDF_Array.hh @@ -29,7 +29,7 @@ class QPDF_Array: public QPDFValue } QPDFObjectHandle at(int n) const noexcept; bool setAt(int n, QPDFObjectHandle const& oh); - void getAsVector(std::vector&) const; + std::vector getAsVector() const; void setFromVector(std::vector const& items); void setFromVector(std::vector>&& items); bool insert(int at, QPDFObjectHandle const& item); -- cgit v1.2.3-54-g00ecf From 0aae54d3836107fdb9dc54faf0778bf262dd7e0a Mon Sep 17 00:00:00 2001 From: m-holger Date: Thu, 30 Mar 2023 14:16:07 +0100 Subject: Refactor QPDF_Array::setFromVector --- libqpdf/QPDFObjectHandle.cc | 6 +----- libqpdf/QPDF_Array.cc | 45 +++++++++++++++++---------------------------- libqpdf/qpdf/QPDF_Array.hh | 1 - 3 files changed, 18 insertions(+), 34 deletions(-) diff --git a/libqpdf/QPDFObjectHandle.cc b/libqpdf/QPDFObjectHandle.cc index 20be3f9d..c2bc30f0 100644 --- a/libqpdf/QPDFObjectHandle.cc +++ b/libqpdf/QPDFObjectHandle.cc @@ -916,11 +916,7 @@ QPDFObjectHandle::setArrayItem(int n, QPDFObjectHandle const& item) void QPDFObjectHandle::setArrayFromVector(std::vector const& items) { - auto array = asArray(); - if (array) { - for (auto const& item: items) { - checkOwnership(item); - } + if (auto array = asArray()) { array->setFromVector(items); } else { typeWarning("array", "ignoring attempt to replace items"); diff --git a/libqpdf/QPDF_Array.cc b/libqpdf/QPDF_Array.cc index 9593c31f..afec05af 100644 --- a/libqpdf/QPDF_Array.cc +++ b/libqpdf/QPDF_Array.cc @@ -35,7 +35,18 @@ QPDF_Array::QPDF_Array( QPDFValue(::ot_array, "array"), sparse(sparse) { - setFromVector(std::move(v)); + if (sparse) { + sp_elements = SparseOHArray(); + for (auto&& item: v) { + if (item->getTypeCode() != ::ot_null || + item->getObjGen().isIndirect()) { + sp_elements.elements[sp_elements.n_elements] = std::move(item); + } + ++sp_elements.n_elements; + } + } else { + elements = std::move(v); + } } QPDF_Array::QPDF_Array(SparseOHArray const& items) : @@ -207,33 +218,11 @@ QPDF_Array::setAt(int at, QPDFObjectHandle const& oh) void QPDF_Array::setFromVector(std::vector const& v) { - if (sparse) { - sp_elements = SparseOHArray(); - for (auto const& iter: v) { - sp_elements.elements[sp_elements.n_elements++] = iter.getObj(); - } - } else { - elements.resize(0); - for (auto const& iter: v) { - elements.push_back(iter.getObj()); - } - } -} - -void -QPDF_Array::setFromVector(std::vector>&& v) -{ - if (sparse) { - sp_elements = SparseOHArray(); - for (auto&& item: v) { - if (item->getTypeCode() != ::ot_null || - item->getObjGen().isIndirect()) { - sp_elements.elements[sp_elements.n_elements] = std::move(item); - } - ++sp_elements.n_elements; - } - } else { - elements = std::move(v); + elements.resize(0); + elements.reserve(v.size()); + for (auto const& item: v) { + checkOwnership(item); + elements.push_back(item.getObj()); } } diff --git a/libqpdf/qpdf/QPDF_Array.hh b/libqpdf/qpdf/QPDF_Array.hh index df814d74..d0138e5c 100644 --- a/libqpdf/qpdf/QPDF_Array.hh +++ b/libqpdf/qpdf/QPDF_Array.hh @@ -31,7 +31,6 @@ class QPDF_Array: public QPDFValue bool setAt(int n, QPDFObjectHandle const& oh); std::vector getAsVector() const; void setFromVector(std::vector const& items); - void setFromVector(std::vector>&& items); bool insert(int at, QPDFObjectHandle const& item); void push_back(QPDFObjectHandle const& item); bool erase(int at); -- cgit v1.2.3-54-g00ecf From a171ebb9427e41559efbeb1f144a19b73bb3eca6 Mon Sep 17 00:00:00 2001 From: m-holger Date: Sun, 11 Dec 2022 16:13:19 +0000 Subject: Refactor QPDF_Array::disconnect --- include/qpdf/QPDFObjectHandle.hh | 2 -- libqpdf/QPDF_Array.cc | 13 +++++++++---- libqpdf/SparseOHArray.cc | 8 -------- libqpdf/qpdf/SparseOHArray.hh | 1 - 4 files changed, 9 insertions(+), 15 deletions(-) diff --git a/include/qpdf/QPDFObjectHandle.hh b/include/qpdf/QPDFObjectHandle.hh index ecb50ba0..ee424e39 100644 --- a/include/qpdf/QPDFObjectHandle.hh +++ b/include/qpdf/QPDFObjectHandle.hh @@ -1494,10 +1494,8 @@ class QPDFObjectHandle // disconnected(). class DisconnectAccess { - friend class QPDF_Array; friend class QPDF_Dictionary; friend class QPDF_Stream; - friend class SparseOHArray; private: static void diff --git a/libqpdf/QPDF_Array.cc b/libqpdf/QPDF_Array.cc index afec05af..1f1ce55a 100644 --- a/libqpdf/QPDF_Array.cc +++ b/libqpdf/QPDF_Array.cc @@ -116,11 +116,16 @@ void QPDF_Array::disconnect() { if (sparse) { - sp_elements.disconnect(); + for (auto& item: sp_elements.elements) { + auto& obj = item.second; + if (!obj->getObjGen().isIndirect()) { + obj->disconnect(); + } + } } else { - for (auto const& iter: elements) { - if (iter) { - QPDFObjectHandle::DisconnectAccess::disconnect(iter); + for (auto& obj: elements) { + if (!obj->getObjGen().isIndirect()) { + obj->disconnect(); } } } diff --git a/libqpdf/SparseOHArray.cc b/libqpdf/SparseOHArray.cc index 6a1c976a..c830d035 100644 --- a/libqpdf/SparseOHArray.cc +++ b/libqpdf/SparseOHArray.cc @@ -1,13 +1,5 @@ #include -void -SparseOHArray::disconnect() -{ - for (auto& iter: this->elements) { - QPDFObjectHandle::DisconnectAccess::disconnect(iter.second); - } -} - SparseOHArray SparseOHArray::copy() { diff --git a/libqpdf/qpdf/SparseOHArray.hh b/libqpdf/qpdf/SparseOHArray.hh index 1b31f266..e7733472 100644 --- a/libqpdf/qpdf/SparseOHArray.hh +++ b/libqpdf/qpdf/SparseOHArray.hh @@ -17,7 +17,6 @@ class SparseOHArray return n_elements; } SparseOHArray copy(); - void disconnect(); private: friend class QPDF_Array; -- cgit v1.2.3-54-g00ecf From d3f2dc322b8e6341d1c16b03c8d6f894c363ed8b Mon Sep 17 00:00:00 2001 From: m-holger Date: Sun, 11 Dec 2022 16:49:41 +0000 Subject: Refactor QPDF_Array::copy --- libqpdf/QPDF_Array.cc | 21 ++++++++++++++++----- libqpdf/SparseOHArray.cc | 13 ------------- libqpdf/qpdf/SparseOHArray.hh | 1 - 3 files changed, 16 insertions(+), 19 deletions(-) diff --git a/libqpdf/QPDF_Array.cc b/libqpdf/QPDF_Array.cc index 1f1ce55a..d5cef946 100644 --- a/libqpdf/QPDF_Array.cc +++ b/libqpdf/QPDF_Array.cc @@ -92,11 +92,22 @@ QPDF_Array::create(std::vector> const& items) std::shared_ptr QPDF_Array::copy(bool shallow) { - if (sparse) { - return create(shallow ? sp_elements : sp_elements.copy()); - } else { - if (shallow) { + if (shallow) { + if (sparse) { + return create(sp_elements); + } else { return create(elements); + } + } else { + if (sparse) { + SparseOHArray result; + result.n_elements = sp_elements.n_elements; + for (auto const& element: sp_elements.elements) { + auto const& obj = element.second; + result.elements[element.first] = + obj->getObjGen().isIndirect() ? obj : obj->copy(); + } + return create(std::move(result)); } else { std::vector> result; result.reserve(elements.size()); @@ -107,7 +118,7 @@ QPDF_Array::copy(bool shallow) : element->copy()) : element); } - return create(result); + return create(std::move(result)); } } } diff --git a/libqpdf/SparseOHArray.cc b/libqpdf/SparseOHArray.cc index c830d035..8b137891 100644 --- a/libqpdf/SparseOHArray.cc +++ b/libqpdf/SparseOHArray.cc @@ -1,14 +1 @@ -#include -SparseOHArray -SparseOHArray::copy() -{ - SparseOHArray result; - result.n_elements = this->n_elements; - for (auto const& element: this->elements) { - auto value = element.second; - result.elements[element.first] = - value->getObjGen().isIndirect() ? value : value->copy(); - } - return result; -} diff --git a/libqpdf/qpdf/SparseOHArray.hh b/libqpdf/qpdf/SparseOHArray.hh index e7733472..5760f174 100644 --- a/libqpdf/qpdf/SparseOHArray.hh +++ b/libqpdf/qpdf/SparseOHArray.hh @@ -16,7 +16,6 @@ class SparseOHArray { return n_elements; } - SparseOHArray copy(); private: friend class QPDF_Array; -- cgit v1.2.3-54-g00ecf From a7b6975132eed94905eb784a0e267e575cade2cb Mon Sep 17 00:00:00 2001 From: m-holger Date: Wed, 29 Mar 2023 19:52:03 +0100 Subject: Remove SparseOHArray --- libqpdf/CMakeLists.txt | 1 - libqpdf/QPDFObjectHandle.cc | 1 - libqpdf/QPDF_Array.cc | 107 +++++++++++++++++------------------------- libqpdf/SparseOHArray.cc | 1 - libqpdf/qpdf/QPDF_Array.hh | 14 +++--- libqpdf/qpdf/SparseOHArray.hh | 26 ---------- libtests/sparse_array.cc | 1 + 7 files changed, 51 insertions(+), 100 deletions(-) delete mode 100644 libqpdf/SparseOHArray.cc delete mode 100644 libqpdf/qpdf/SparseOHArray.hh diff --git a/libqpdf/CMakeLists.txt b/libqpdf/CMakeLists.txt index 5e3a628e..623e05d3 100644 --- a/libqpdf/CMakeLists.txt +++ b/libqpdf/CMakeLists.txt @@ -115,7 +115,6 @@ set(libqpdf_SOURCES ResourceFinder.cc SecureRandomDataProvider.cc SF_FlateLzwDecode.cc - SparseOHArray.cc qpdf-c.cc qpdfjob-c.cc qpdflogger-c.cc) diff --git a/libqpdf/QPDFObjectHandle.cc b/libqpdf/QPDFObjectHandle.cc index c2bc30f0..b3f208a5 100644 --- a/libqpdf/QPDFObjectHandle.cc +++ b/libqpdf/QPDFObjectHandle.cc @@ -23,7 +23,6 @@ #include #include #include -#include #include #include diff --git a/libqpdf/QPDF_Array.cc b/libqpdf/QPDF_Array.cc index d5cef946..36c5aaea 100644 --- a/libqpdf/QPDF_Array.cc +++ b/libqpdf/QPDF_Array.cc @@ -1,5 +1,6 @@ #include +#include #include static const QPDFObjectHandle null_oh = QPDFObjectHandle::newNull(); @@ -24,6 +25,20 @@ QPDF_Array::checkOwnership(QPDFObjectHandle const& item) const } } +QPDF_Array::QPDF_Array() : + QPDFValue(::ot_array, "array") +{ +} + +QPDF_Array::QPDF_Array(QPDF_Array const& other) : + QPDFValue(::ot_array, "array"), + sparse(other.sparse), + sp_size(other.sp_size), + sp_elements(other.sp_elements), + elements(other.elements) +{ +} + QPDF_Array::QPDF_Array(std::vector const& v) : QPDFValue(::ot_array, "array") { @@ -36,34 +51,18 @@ QPDF_Array::QPDF_Array( sparse(sparse) { if (sparse) { - sp_elements = SparseOHArray(); for (auto&& item: v) { if (item->getTypeCode() != ::ot_null || item->getObjGen().isIndirect()) { - sp_elements.elements[sp_elements.n_elements] = std::move(item); + sp_elements[sp_size] = std::move(item); } - ++sp_elements.n_elements; + ++sp_size; } } else { elements = std::move(v); } } -QPDF_Array::QPDF_Array(SparseOHArray const& items) : - QPDFValue(::ot_array, "array"), - sparse(true), - sp_elements(items) - -{ -} - -QPDF_Array::QPDF_Array(std::vector> const& items) : - QPDFValue(::ot_array, "array"), - sparse(false), - elements(items) -{ -} - std::shared_ptr QPDF_Array::create(std::vector const& items) { @@ -77,37 +76,21 @@ QPDF_Array::create( return do_create(new QPDF_Array(std::move(items), sparse)); } -std::shared_ptr -QPDF_Array::create(SparseOHArray const& items) -{ - return do_create(new QPDF_Array(items)); -} - -std::shared_ptr -QPDF_Array::create(std::vector> const& items) -{ - return do_create(new QPDF_Array(items)); -} - std::shared_ptr QPDF_Array::copy(bool shallow) { if (shallow) { - if (sparse) { - return create(sp_elements); - } else { - return create(elements); - } + return do_create(new QPDF_Array(*this)); } else { if (sparse) { - SparseOHArray result; - result.n_elements = sp_elements.n_elements; - for (auto const& element: sp_elements.elements) { + QPDF_Array* result = new QPDF_Array(); + result->sp_size = sp_size; + for (auto const& element: sp_elements) { auto const& obj = element.second; - result.elements[element.first] = + result->sp_elements[element.first] = obj->getObjGen().isIndirect() ? obj : obj->copy(); } - return create(std::move(result)); + return do_create(result); } else { std::vector> result; result.reserve(elements.size()); @@ -118,7 +101,7 @@ QPDF_Array::copy(bool shallow) : element->copy()) : element); } - return create(std::move(result)); + return create(std::move(result), false); } } } @@ -127,7 +110,7 @@ void QPDF_Array::disconnect() { if (sparse) { - for (auto& item: sp_elements.elements) { + for (auto& item: sp_elements) { auto& obj = item.second; if (!obj->getObjGen().isIndirect()) { obj->disconnect(); @@ -147,8 +130,7 @@ QPDF_Array::unparse() { if (sparse) { std::string result = "[ "; - int size = sp_elements.size(); - for (int i = 0; i < size; ++i) { + for (int i = 0; i < sp_size; ++i) { result += at(i).unparse(); result += " "; } @@ -171,8 +153,7 @@ QPDF_Array::getJSON(int json_version) { if (sparse) { JSON j = JSON::makeArray(); - int size = sp_elements.size(); - for (int i = 0; i < size; ++i) { + for (int i = 0; i < sp_size; ++i) { j.addArrayElement(at(i).getJSON(json_version)); } return j; @@ -192,8 +173,8 @@ QPDF_Array::at(int n) const noexcept if (n < 0 || n >= size()) { return {}; } else if (sparse) { - auto const& iter = sp_elements.elements.find(n); - return iter == sp_elements.elements.end() ? null_oh : (*iter).second; + auto const& iter = sp_elements.find(n); + return iter == sp_elements.end() ? null_oh : (*iter).second; } else { return elements[size_t(n)]; } @@ -205,7 +186,7 @@ QPDF_Array::getAsVector() const if (sparse) { std::vector v; v.reserve(size_t(size())); - for (auto const& item: sp_elements.elements) { + for (auto const& item: sp_elements) { v.resize(size_t(item.first), null_oh); v.push_back(item.second); } @@ -224,7 +205,7 @@ QPDF_Array::setAt(int at, QPDFObjectHandle const& oh) } checkOwnership(oh); if (sparse) { - sp_elements.elements[at] = oh.getObj(); + sp_elements[at] = oh.getObj(); } else { elements[size_t(at)] = oh.getObj(); } @@ -254,19 +235,19 @@ QPDF_Array::insert(int at, QPDFObjectHandle const& item) } else { checkOwnership(item); if (sparse) { - auto iter = sp_elements.elements.crbegin(); - while (iter != sp_elements.elements.crend()) { + auto iter = sp_elements.crbegin(); + while (iter != sp_elements.crend()) { auto key = (iter++)->first; if (key >= at) { - auto nh = sp_elements.elements.extract(key); + auto nh = sp_elements.extract(key); ++nh.key(); - sp_elements.elements.insert(std::move(nh)); + sp_elements.insert(std::move(nh)); } else { break; } } - sp_elements.elements[at] = item.getObj(); - ++sp_elements.n_elements; + sp_elements[at] = item.getObj(); + ++sp_size; } else { elements.insert(elements.cbegin() + at, item.getObj()); } @@ -279,7 +260,7 @@ QPDF_Array::push_back(QPDFObjectHandle const& item) { checkOwnership(item); if (sparse) { - sp_elements.elements[sp_elements.n_elements++] = item.getObj(); + sp_elements[sp_size++] = item.getObj(); } else { elements.push_back(item.getObj()); } @@ -292,20 +273,20 @@ QPDF_Array::erase(int at) return false; } if (sparse) { - auto end = sp_elements.elements.end(); - if (auto iter = sp_elements.elements.lower_bound(at); iter != end) { + auto end = sp_elements.end(); + if (auto iter = sp_elements.lower_bound(at); iter != end) { if (iter->first == at) { iter++; - sp_elements.elements.erase(at); + sp_elements.erase(at); } while (iter != end) { - auto nh = sp_elements.elements.extract(iter++); + auto nh = sp_elements.extract(iter++); --nh.key(); - sp_elements.elements.insert(std::move(nh)); + sp_elements.insert(std::move(nh)); } } - --sp_elements.n_elements; + --sp_size; } else { elements.erase(elements.cbegin() + at); } diff --git a/libqpdf/SparseOHArray.cc b/libqpdf/SparseOHArray.cc deleted file mode 100644 index 8b137891..00000000 --- a/libqpdf/SparseOHArray.cc +++ /dev/null @@ -1 +0,0 @@ - diff --git a/libqpdf/qpdf/QPDF_Array.hh b/libqpdf/qpdf/QPDF_Array.hh index d0138e5c..4762bb6e 100644 --- a/libqpdf/qpdf/QPDF_Array.hh +++ b/libqpdf/qpdf/QPDF_Array.hh @@ -3,7 +3,7 @@ #include -#include +#include #include class QPDF_Array: public QPDFValue @@ -14,9 +14,6 @@ class QPDF_Array: public QPDFValue create(std::vector const& items); static std::shared_ptr create(std::vector>&& items, bool sparse); - static std::shared_ptr create(SparseOHArray const& items); - static std::shared_ptr - create(std::vector> const& items); virtual std::shared_ptr copy(bool shallow = false); virtual std::string unparse(); virtual JSON getJSON(int json_version); @@ -25,7 +22,7 @@ class QPDF_Array: public QPDFValue int size() const noexcept { - return sparse ? sp_elements.size() : int(elements.size()); + return sparse ? sp_size : int(elements.size()); } QPDFObjectHandle at(int n) const noexcept; bool setAt(int n, QPDFObjectHandle const& oh); @@ -36,15 +33,16 @@ class QPDF_Array: public QPDFValue bool erase(int at); private: + QPDF_Array(); + QPDF_Array(QPDF_Array const&); QPDF_Array(std::vector const& items); QPDF_Array(std::vector>&& items, bool sparse); - QPDF_Array(SparseOHArray const& items); - QPDF_Array(std::vector> const& items); void checkOwnership(QPDFObjectHandle const& item) const; bool sparse{false}; - SparseOHArray sp_elements; + int sp_size{0}; + std::map> sp_elements; std::vector> elements; }; diff --git a/libqpdf/qpdf/SparseOHArray.hh b/libqpdf/qpdf/SparseOHArray.hh deleted file mode 100644 index 5760f174..00000000 --- a/libqpdf/qpdf/SparseOHArray.hh +++ /dev/null @@ -1,26 +0,0 @@ -#ifndef QPDF_SPARSEOHARRAY_HH -#define QPDF_SPARSEOHARRAY_HH - -#include -#include -#include - -class QPDF_Array; - -class SparseOHArray -{ - public: - SparseOHArray() = default; - int - size() const noexcept - { - return n_elements; - } - - private: - friend class QPDF_Array; - std::map> elements; - int n_elements{0}; -}; - -#endif // QPDF_SPARSEOHARRAY_HH diff --git a/libtests/sparse_array.cc b/libtests/sparse_array.cc index a970b199..9f31c96f 100644 --- a/libtests/sparse_array.cc +++ b/libtests/sparse_array.cc @@ -1,6 +1,7 @@ #include #include +#include #include #include -- cgit v1.2.3-54-g00ecf From c2ab0441b5d2c03a732d68e2ed36a19343ccbb5a Mon Sep 17 00:00:00 2001 From: m-holger Date: Sun, 25 Dec 2022 09:52:43 +0000 Subject: Refactor QPDF_Array::getJSON --- libqpdf/QPDF_Array.cc | 31 ++++++++++++++++++++++--------- libqpdf/QPDF_Null.cc | 1 + 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/libqpdf/QPDF_Array.cc b/libqpdf/QPDF_Array.cc index 36c5aaea..2cb4bce5 100644 --- a/libqpdf/QPDF_Array.cc +++ b/libqpdf/QPDF_Array.cc @@ -151,20 +151,33 @@ QPDF_Array::unparse() JSON QPDF_Array::getJSON(int json_version) { + static const JSON j_null = JSON::makeNull(); + JSON j_array = JSON::makeArray(); if (sparse) { - JSON j = JSON::makeArray(); - for (int i = 0; i < sp_size; ++i) { - j.addArrayElement(at(i).getJSON(json_version)); + int next = 0; + for (auto& item: sp_elements) { + int key = item.first; + for (int j = next; j < key; ++j) { + j_array.addArrayElement(j_null); + } + auto og = item.second->getObjGen(); + j_array.addArrayElement( + og.isIndirect() ? JSON::makeString(og.unparse(' ') + " R") + : item.second->getJSON(json_version)); + next = ++key; + } + for (int j = next; j < sp_size; ++j) { + j_array.addArrayElement(j_null); } - return j; } else { - JSON j = JSON::makeArray(); - size_t size = elements.size(); - for (int i = 0; i < int(size); ++i) { - j.addArrayElement(at(i).getJSON(json_version)); + for (auto const& item: elements) { + auto og = item->getObjGen(); + j_array.addArrayElement( + og.isIndirect() ? JSON::makeString(og.unparse(' ') + " R") + : item->getJSON(json_version)); } - return j; } + return j_array; } QPDFObjectHandle diff --git a/libqpdf/QPDF_Null.cc b/libqpdf/QPDF_Null.cc index a82f23c0..0b59f5c9 100644 --- a/libqpdf/QPDF_Null.cc +++ b/libqpdf/QPDF_Null.cc @@ -50,5 +50,6 @@ QPDF_Null::unparse() JSON QPDF_Null::getJSON(int json_version) { + // If this is updated, QPDF_Array::getJSON must also be updated. return JSON::makeNull(); } -- cgit v1.2.3-54-g00ecf From 0b53b648ab79a35b9ec0da56c0b936955a51d135 Mon Sep 17 00:00:00 2001 From: m-holger Date: Sun, 25 Dec 2022 11:03:54 +0000 Subject: Refactor QPDF_Array::unparse --- libqpdf/QPDF_Array.cc | 35 ++++++++++++++++++++++------------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/libqpdf/QPDF_Array.cc b/libqpdf/QPDF_Array.cc index 2cb4bce5..3bd139b1 100644 --- a/libqpdf/QPDF_Array.cc +++ b/libqpdf/QPDF_Array.cc @@ -128,24 +128,33 @@ QPDF_Array::disconnect() std::string QPDF_Array::unparse() { + std::string result = "[ "; if (sparse) { - std::string result = "[ "; - for (int i = 0; i < sp_size; ++i) { - result += at(i).unparse(); - result += " "; + int next = 0; + for (auto& item: sp_elements) { + int key = item.first; + for (int j = next; j < key; ++j) { + result += "null "; + } + item.second->resolve(); + auto og = item.second->getObjGen(); + result += og.isIndirect() ? og.unparse(' ') + " R " + : item.second->unparse() + " "; + next = ++key; + } + for (int j = next; j < sp_size; ++j) { + result += "null "; } - result += "]"; - return result; } else { - std::string result = "[ "; - auto size = elements.size(); - for (int i = 0; i < int(size); ++i) { - result += at(i).unparse(); - result += " "; + for (auto const& item: elements) { + item->resolve(); + auto og = item->getObjGen(); + result += og.isIndirect() ? og.unparse(' ') + " R " + : item->unparse() + " "; } - result += "]"; - return result; } + result += "]"; + return result; } JSON -- cgit v1.2.3-54-g00ecf