From decc7d3c12ab8ce4a6c77d48cfc57d721913b887 Mon Sep 17 00:00:00 2001 From: m-holger Date: Fri, 8 Aug 2025 09:54:48 +0100 Subject: [PATCH] Refactor `QPDFObjectHandle` and `BaseHandle`: streamline type handling, simplify warnings, and centralize utility methods. --- include/qpdf/ObjectHandle.hh | 11 +++++++++++ include/qpdf/QPDFObjectHandle.hh | 1 - libqpdf/QPDFObjectHandle.cc | 127 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------------------------------------- libqpdf/qpdf/QPDFObject_private.hh | 7 ------- 4 files changed, 83 insertions(+), 63 deletions(-) diff --git a/include/qpdf/ObjectHandle.hh b/include/qpdf/ObjectHandle.hh index b8978e2..600cec2 100644 --- a/include/qpdf/ObjectHandle.hh +++ b/include/qpdf/ObjectHandle.hh @@ -22,7 +22,10 @@ #include #include +#include + #include +#include #include #include @@ -71,6 +74,9 @@ namespace qpdf inline qpdf_object_type_e type_code() const; std::string unparse() const; void write_json(int json_version, JSON::Writer& p) const; + static void warn(QPDF*, QPDFExc&&); + void warn(QPDFExc&&) const; + void warn(std::string const& warning) const; protected: BaseHandle() = default; @@ -87,6 +93,11 @@ namespace qpdf template T* as() const; + std::string description() const; + std::runtime_error type_error(char const* expected_type) const; + QPDFExc type_error(char const* expected_type, std::string const& message) const; + char const* type_name() const; + std::shared_ptr obj; }; diff --git a/include/qpdf/QPDFObjectHandle.hh b/include/qpdf/QPDFObjectHandle.hh index 6eaeedb..afa3204 100644 --- a/include/qpdf/QPDFObjectHandle.hh +++ b/include/qpdf/QPDFObjectHandle.hh @@ -1357,7 +1357,6 @@ class QPDFObjectHandle: public qpdf::BaseHandle QPDF* context); std::vector arrayOrStreamToStreamArray(std::string const& description, std::string& all_description); - static void warn(QPDF*, QPDFExc const&); void checkOwnership(QPDFObjectHandle const&) const; }; diff --git a/libqpdf/QPDFObjectHandle.cc b/libqpdf/QPDFObjectHandle.cc index 5127ebe..7253ac7 100644 --- a/libqpdf/QPDFObjectHandle.cc +++ b/libqpdf/QPDFObjectHandle.cc @@ -611,11 +611,11 @@ QPDFObjectHandle::isSameObjectAs(QPDFObjectHandle const& rhs) const qpdf_object_type_e QPDFObjectHandle::getTypeCode() const { - return obj ? obj->getResolvedTypeCode() : ::ot_uninitialized; + return type_code(); } char const* -QPDFObjectHandle::getTypeName() const +BaseHandle::type_name() const { static constexpr std::array tn{ "uninitialized", @@ -634,7 +634,13 @@ QPDFObjectHandle::getTypeName() const "unresolved", "destroyed", "reference"}; - return obj ? tn[getTypeCode()] : "uninitialized"; + return tn[type_code()]; +} + +char const* +QPDFObjectHandle::getTypeName() const +{ + return type_name(); } bool @@ -1242,27 +1248,23 @@ QPDFObjectHandle::arrayOrStreamToStreamArray( result.emplace_back(item); } else { QTC::TC("qpdf", "QPDFObjectHandle non-stream in stream array"); - warn( - item.getOwningQPDF(), - QPDFExc( - qpdf_e_damaged_pdf, - "", - description + ": item index " + std::to_string(i) + " (from 0)", - 0, - "ignoring non-stream in an array of streams")); + item.warn( + {qpdf_e_damaged_pdf, + "", + description + ": item index " + std::to_string(i) + " (from 0)", + 0, + "ignoring non-stream in an array of streams"}); } } } else if (isStream()) { result.emplace_back(*this); - } else if (!isNull()) { + } else if (!null()) { warn( - getOwningQPDF(), - QPDFExc( - qpdf_e_damaged_pdf, - "", - description, - 0, - " object is supposed to be a stream or an array of streams but is neither")); + {qpdf_e_damaged_pdf, + "", + description, + 0, + " object is supposed to be a stream or an array of streams but is neither"}); } bool first = true; @@ -1824,6 +1826,12 @@ QPDFObjectHandle::newReserved(QPDF* qpdf) return qpdf->newReserved(); } +std::string +BaseHandle::description() const +{ + return obj ? obj->getDescription() : ""s; +} + void QPDFObjectHandle::setObjectDescription(QPDF* owning_qpdf, std::string const& object_description) { @@ -1934,50 +1942,44 @@ QPDFObjectHandle::assertInitialized() const } } +std::runtime_error +BaseHandle::type_error(char const* expected_type) const +{ + return std::runtime_error( + "operation for "s + expected_type + " attempted on object of type " + type_name()); +} + +QPDFExc +BaseHandle::type_error(char const* expected_type, std::string const& message) const +{ + return { + qpdf_e_object, + "", + description(), + 0, + "operation for "s + expected_type + " attempted on object of type " + type_name() + ": " + + message}; +} + void -QPDFObjectHandle::typeWarning(char const* expected_type, std::string const& warning) const +QPDFObjectHandle::typeWarning(char const* expected_type, std::string const& message) const { - QPDF* context = nullptr; - std::string description; - // Type checks above guarantee that the object has been dereferenced. Nevertheless, dereference - // throws exceptions in the test suite if (!obj) { throw std::logic_error("attempted to dereference an uninitialized QPDFObjectHandle"); } - obj->getDescription(context, description); - // Null context handled by warn - warn( - context, - QPDFExc( - qpdf_e_object, - "", - description, - 0, - std::string("operation for ") + expected_type + " attempted on object of type " + - QPDFObjectHandle(*this).getTypeName() + ": " + warning)); + warn(type_error(expected_type, message)); } void QPDFObjectHandle::warnIfPossible(std::string const& warning) const { - QPDF* context = nullptr; - std::string description; - if (obj && obj->getDescription(context, description)) { - warn(context, QPDFExc(qpdf_e_damaged_pdf, "", description, 0, warning)); - } else { - *QPDFLogger::defaultLogger()->getError() << warning << "\n"; - } + warn(warning); } void QPDFObjectHandle::objectWarning(std::string const& warning) const { - QPDF* context = nullptr; - std::string description; - // Type checks above guarantee that the object is initialized. - obj->getDescription(context, description); - // Null context handled by warn - warn(context, QPDFExc(qpdf_e_object, "", description, 0, warning)); + warn({qpdf_e_object, "", description(), 0, warning}); } void @@ -2130,15 +2132,30 @@ QPDFObjectHandle::assertPageObject() const } void -QPDFObjectHandle::warn(QPDF* qpdf, QPDFExc const& e) +BaseHandle::warn(QPDF* qpdf, QPDFExc&& e) { - // If parsing on behalf of a QPDF object and want to give a warning, we can warn through the - // object. If parsing for some other reason, such as an explicit creation of an object from a - // string, then just throw the exception. - if (qpdf) { - qpdf->warn(e); + if (!qpdf) { + throw std::move(e); + } + qpdf->warn(std::move(e)); +} + +void +BaseHandle::warn(QPDFExc&& e) const +{ + if (!qpdf()) { + throw std::move(e); + } + qpdf()->warn(std::move(e)); +} + +void +BaseHandle::warn(std::string const& warning) const +{ + if (qpdf()) { + warn({qpdf_e_damaged_pdf, "", description(), 0, warning}); } else { - throw e; + *QPDFLogger::defaultLogger()->getError() << warning << "\n"; } } diff --git a/libqpdf/qpdf/QPDFObject_private.hh b/libqpdf/qpdf/QPDFObject_private.hh index 8518665..dde27e4 100644 --- a/libqpdf/qpdf/QPDFObject_private.hh +++ b/libqpdf/qpdf/QPDFObject_private.hh @@ -434,13 +434,6 @@ class QPDFObject parsed_offset = offset; } } - bool - getDescription(QPDF*& a_qpdf, std::string& description) - { - a_qpdf = qpdf; - description = getDescription(); - return qpdf != nullptr; - } qpdf_offset_t getParsedOffset() { -- libgit2 0.21.4