From 8b05c550b3ee4a61ada4aef6f530c9a41efaf8a5 Mon Sep 17 00:00:00 2001 From: m-holger Date: Mon, 29 May 2023 19:26:18 +0100 Subject: Fix doc typos --- libqpdf/QPDF.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libqpdf/QPDF.cc b/libqpdf/QPDF.cc index 6f718742..1dc5de1e 100644 --- a/libqpdf/QPDF.cc +++ b/libqpdf/QPDF.cc @@ -1487,7 +1487,8 @@ QPDF::readObjectAtOffset( // Special case: if offset is 0, just return null. Some PDF writers, in particular // "Mac OS X 10.7.5 Quartz PDFContext", may store deleted objects in the xref table as - // "0000000000 00000 n", which is not correct, but it won't hurt anything for to ignore these. + // "0000000000 00000 n", which is not correct, but it won't hurt anything for us to ignore + // these. if (offset == 0) { QTC::TC("qpdf", "QPDF bogus 0 offset", 0); warn(damagedPDF(0, "object has offset 0")); -- cgit v1.2.3-54-g00ecf From 62f00b6d9c93e17ff1eb25cb19f2d82c1038f280 Mon Sep 17 00:00:00 2001 From: m-holger Date: Mon, 29 May 2023 14:35:45 +0100 Subject: Change JSONHandler::m to std::unique_ptr and declare Members in implementation file --- libqpdf/JSONHandler.cc | 18 +++++++++++++++ libqpdf/qpdf/JSONHandler.hh | 53 +++++++++++++-------------------------------- 2 files changed, 33 insertions(+), 38 deletions(-) diff --git a/libqpdf/JSONHandler.cc b/libqpdf/JSONHandler.cc index b5c7c35d..d6021935 100644 --- a/libqpdf/JSONHandler.cc +++ b/libqpdf/JSONHandler.cc @@ -4,11 +4,29 @@ #include #include +class JSONHandler::Members +{ + friend class JSONHandler; + + public: + ~Members() = default; + + private: + Members() = default; + Members(Members const&) = delete; + + Handlers h; +}; + JSONHandler::JSONHandler() : m(new Members()) { } +JSONHandler::~JSONHandler() +{ +} + void JSONHandler::usage(std::string const& msg) { diff --git a/libqpdf/qpdf/JSONHandler.hh b/libqpdf/qpdf/JSONHandler.hh index 8fdcaee6..6439ff12 100644 --- a/libqpdf/qpdf/JSONHandler.hh +++ b/libqpdf/qpdf/JSONHandler.hh @@ -16,7 +16,7 @@ class JSONHandler public: // A QPDFUsage exception is thrown if there are any errors validating the JSON object. JSONHandler(); - ~JSONHandler() = default; + ~JSONHandler(); // Based on the type of handler, expect the object to be of a certain type. QPDFUsage is thrown // otherwise. Multiple handlers may be registered, which allows the object to be of various @@ -55,49 +55,26 @@ class JSONHandler struct Handlers { - Handlers() : - any_handler(nullptr), - null_handler(nullptr), - string_handler(nullptr), - number_handler(nullptr), - bool_handler(nullptr), - dict_start_handler(nullptr), - dict_end_handler(nullptr), - array_start_handler(nullptr), - array_end_handler(nullptr), - final_handler(nullptr) - { - } - - json_handler_t any_handler; - void_handler_t null_handler; - string_handler_t string_handler; - string_handler_t number_handler; - bool_handler_t bool_handler; - json_handler_t dict_start_handler; - void_handler_t dict_end_handler; - json_handler_t array_start_handler; - void_handler_t array_end_handler; - void_handler_t final_handler; + Handlers() = default; + + json_handler_t any_handler{nullptr}; + void_handler_t null_handler{nullptr}; + string_handler_t string_handler{nullptr}; + string_handler_t number_handler{nullptr}; + bool_handler_t bool_handler{nullptr}; + json_handler_t dict_start_handler{nullptr}; + void_handler_t dict_end_handler{nullptr}; + json_handler_t array_start_handler{nullptr}; + void_handler_t array_end_handler{nullptr}; + void_handler_t final_handler{nullptr}; std::map> dict_handlers; std::shared_ptr fallback_dict_handler; std::shared_ptr array_item_handler; }; - class Members - { - friend class JSONHandler; - - public: - ~Members() = default; + class Members; - private: - Members() = default; - Members(Members const&) = delete; - - Handlers h; - }; - std::shared_ptr m; + std::unique_ptr m; }; #endif // JSONHANDLER_HH -- cgit v1.2.3-54-g00ecf From 75e74679c54ed6e51217530277b6bf45aa2b2dcd Mon Sep 17 00:00:00 2001 From: m-holger Date: Mon, 12 Jun 2023 09:02:20 +0100 Subject: Move struct JSONHandler::Handlers to implementation file --- libqpdf/JSONHandler.cc | 19 +++++++++++++++++++ libqpdf/qpdf/JSONHandler.hh | 18 ------------------ 2 files changed, 19 insertions(+), 18 deletions(-) diff --git a/libqpdf/JSONHandler.cc b/libqpdf/JSONHandler.cc index d6021935..a3ff4555 100644 --- a/libqpdf/JSONHandler.cc +++ b/libqpdf/JSONHandler.cc @@ -4,6 +4,25 @@ #include #include +struct Handlers +{ + Handlers() = default; + + JSONHandler::json_handler_t any_handler{nullptr}; + JSONHandler::void_handler_t null_handler{nullptr}; + JSONHandler::string_handler_t string_handler{nullptr}; + JSONHandler::string_handler_t number_handler{nullptr}; + JSONHandler::bool_handler_t bool_handler{nullptr}; + JSONHandler::json_handler_t dict_start_handler{nullptr}; + JSONHandler::void_handler_t dict_end_handler{nullptr}; + JSONHandler::json_handler_t array_start_handler{nullptr}; + JSONHandler::void_handler_t array_end_handler{nullptr}; + JSONHandler::void_handler_t final_handler{nullptr}; + std::map> dict_handlers; + std::shared_ptr fallback_dict_handler; + std::shared_ptr array_item_handler; +}; + class JSONHandler::Members { friend class JSONHandler; diff --git a/libqpdf/qpdf/JSONHandler.hh b/libqpdf/qpdf/JSONHandler.hh index 6439ff12..653924f6 100644 --- a/libqpdf/qpdf/JSONHandler.hh +++ b/libqpdf/qpdf/JSONHandler.hh @@ -53,24 +53,6 @@ class JSONHandler static void usage(std::string const& msg); - struct Handlers - { - Handlers() = default; - - json_handler_t any_handler{nullptr}; - void_handler_t null_handler{nullptr}; - string_handler_t string_handler{nullptr}; - string_handler_t number_handler{nullptr}; - bool_handler_t bool_handler{nullptr}; - json_handler_t dict_start_handler{nullptr}; - void_handler_t dict_end_handler{nullptr}; - json_handler_t array_start_handler{nullptr}; - void_handler_t array_end_handler{nullptr}; - void_handler_t final_handler{nullptr}; - std::map> dict_handlers; - std::shared_ptr fallback_dict_handler; - std::shared_ptr array_item_handler; - }; class Members; -- cgit v1.2.3-54-g00ecf From 8cb89529bd52ab40f5cf93024f6fbf6c0ef52f56 Mon Sep 17 00:00:00 2001 From: m-holger Date: Mon, 12 Jun 2023 13:29:58 +0100 Subject: Use early returns in JSONHandler::handle --- libqpdf/JSONHandler.cc | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/libqpdf/JSONHandler.cc b/libqpdf/JSONHandler.cc index a3ff4555..4a69fd60 100644 --- a/libqpdf/JSONHandler.cc +++ b/libqpdf/JSONHandler.cc @@ -117,24 +117,24 @@ JSONHandler::handle(std::string const& path, JSON j) m->h.any_handler(path, j); return; } - bool handled = false; + bool bvalue = false; std::string s_value; if (m->h.null_handler && j.isNull()) { m->h.null_handler(path); - handled = true; + return; } if (m->h.string_handler && j.getString(s_value)) { m->h.string_handler(path, s_value); - handled = true; + return; } if (m->h.number_handler && j.getNumber(s_value)) { m->h.number_handler(path, s_value); - handled = true; + return; } if (m->h.bool_handler && j.getBool(bvalue)) { m->h.bool_handler(path, bvalue); - handled = true; + return; } if (m->h.dict_start_handler && j.isDictionary()) { m->h.dict_start_handler(path, j); @@ -156,7 +156,7 @@ JSONHandler::handle(std::string const& path, JSON j) } }); m->h.dict_end_handler(path); - handled = true; + return; } if (m->h.array_start_handler && j.isArray()) { m->h.array_start_handler(path, j); @@ -166,15 +166,13 @@ JSONHandler::handle(std::string const& path, JSON j) ++i; }); m->h.array_end_handler(path); - handled = true; + return; } - if (!handled) { - // It would be nice to include information about what type the object was and what types - // were allowed, but we're relying on schema validation to make sure input is properly - // structured before calling the handlers. It would be different if this code were trying to - // be part of a general-purpose JSON package. - QTC::TC("libtests", "JSONHandler unhandled value"); - usage("JSON handler: value at " + path + " is not of expected type"); - } + // It would be nice to include information about what type the object was and what types were + // allowed, but we're relying on schema validation to make sure input is properly structured + // before calling the handlers. It would be different if this code were trying to be part of a + // general-purpose JSON package. + QTC::TC("libtests", "JSONHandler unhandled value"); + usage("JSON handler: value at " + path + " is not of expected type"); } -- cgit v1.2.3-54-g00ecf From d8bbe46eaa6386ac3900a8d75ce7621889bca1f6 Mon Sep 17 00:00:00 2001 From: m-holger Date: Tue, 13 Jun 2023 10:53:35 +0100 Subject: Update README-maintainer section on use of Member pattern --- README-maintainer.md | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/README-maintainer.md b/README-maintainer.md index 4f11fdcf..254c248f 100644 --- a/README-maintainer.md +++ b/README-maintainer.md @@ -235,16 +235,26 @@ Building docs from pull requests is also enabled. // README-maintainer ``` -* Put private member variables in std::shared_ptr for all - public classes. Remember to use QPDF_DLL on ~Members(). Exception: - indirection through std::shared_ptr is expensive, so don't - do it for classes that are copied a lot, like QPDFObjectHandle and - QPDFObject. It may be possible to declare - std::shared_ptr m_ph; - Member* m; - with m = m_ph.get(), and then indirect through m in - performance-critical settings, though in 2022, std::shared_ptr is - sufficiently performant that this may not be worth it. +* Put private member variables in std::unique_ptr for all + public classes. Forward declare Members in the header file and define + Members in the implementation file. One of the major benefits of + defining Members in the implementation file is that it makes it easier + to use private classes as data members and simplifies the include order. + Remember that Members must be fully defined before the destructor of the + main class. For an example of this pattern see class JSONHandler. + + Exception: indirection through std::unique_ptr incurs an overhead, + so don't do it for: + * (especially private) classes that are copied a lot, like QPDFObjectHandle + and QPDFObject. + * classes that are a shared pointer to another class, such as QPDFObjectHandle + or JSON. + + For exported classes that do not use the member pattern for performance + reasons it is worth considering adding a std::unique_ptr to an empty Members + class initialized to nullptr to give the flexibility to add data members + without breaking the ABI. + * Traversal of objects is expensive. It's worth adding some complexity to avoid needless traversals of objects. -- cgit v1.2.3-54-g00ecf