From dfb2718c31f97d90c8c890586a0bb2380d8f28fc Mon Sep 17 00:00:00 2001
From: m-holger <m.holger@qpdf.org>
Date: Wed, 27 Aug 2025 19:02:06 +0100
Subject: [PATCH] Refactor `QPDFOutlineDocumentHelper::Members` to use `std::unique_ptr` for `names_dest`, improve memory management, and streamline initialization logic

---
 include/qpdf/QPDFOutlineDocumentHelper.hh | 24 ++----------------------
 libqpdf/QPDFOutlineDocumentHelper.cc      | 35 ++++++++++++++++++++++++++---------
 qpdf/qpdf.testcov                         |  2 --
 3 files changed, 28 insertions(+), 33 deletions(-)

diff --git a/include/qpdf/QPDFOutlineDocumentHelper.hh b/include/qpdf/QPDFOutlineDocumentHelper.hh
index a36b4d5..5dc639a 100644
--- a/include/qpdf/QPDFOutlineDocumentHelper.hh
+++ b/include/qpdf/QPDFOutlineDocumentHelper.hh
@@ -63,33 +63,13 @@ class QPDFOutlineDocumentHelper: public QPDFDocumentHelper
     {
         friend class QPDFOutlineObjectHelper;
 
-        static bool
-        checkSeen(QPDFOutlineDocumentHelper& dh, QPDFObjGen og)
-        {
-            return !dh.m->seen.add(og);
-        }
+        static bool checkSeen(QPDFOutlineDocumentHelper& dh, QPDFObjGen og);
     };
 
   private:
     void initializeByPage();
 
-    class Members
-    {
-        friend class QPDFOutlineDocumentHelper;
-
-      public:
-        ~Members() = default;
-
-      private:
-        Members() = default;
-        Members(Members const&) = delete;
-
-        std::vector<QPDFOutlineObjectHelper> outlines;
-        QPDFObjGen::set seen;
-        QPDFObjectHandle dest_dict;
-        std::shared_ptr<QPDFNameTreeObjectHelper> names_dest;
-        std::map<QPDFObjGen, std::vector<QPDFOutlineObjectHelper>> by_page;
-    };
+    class Members;
 
     std::shared_ptr<Members> m;
 };
diff --git a/libqpdf/QPDFOutlineDocumentHelper.cc b/libqpdf/QPDFOutlineDocumentHelper.cc
index 13107c3..e996789 100644
--- a/libqpdf/QPDFOutlineDocumentHelper.cc
+++ b/libqpdf/QPDFOutlineDocumentHelper.cc
@@ -2,22 +2,42 @@
 
 #include <qpdf/QTC.hh>
 
+class QPDFOutlineDocumentHelper::Members
+{
+  public:
+    Members() = default;
+    Members(Members const&) = delete;
+    ~Members() = default;
+
+    std::vector<QPDFOutlineObjectHelper> outlines;
+    QPDFObjGen::set seen;
+    QPDFObjectHandle dest_dict;
+    std::unique_ptr<QPDFNameTreeObjectHelper> names_dest;
+    std::map<QPDFObjGen, std::vector<QPDFOutlineObjectHelper>> by_page;
+};
+
+bool
+QPDFOutlineDocumentHelper::Accessor::checkSeen(QPDFOutlineDocumentHelper& dh, QPDFObjGen og)
+{
+    return !dh.m->seen.add(og);
+}
+
 QPDFOutlineDocumentHelper::QPDFOutlineDocumentHelper(QPDF& qpdf) :
     QPDFDocumentHelper(qpdf),
-    m(new Members())
+    m(std::make_shared<Members>())
 {
     QPDFObjectHandle root = qpdf.getRoot();
     if (!root.hasKey("/Outlines")) {
         return;
     }
-    QPDFObjectHandle outlines = root.getKey("/Outlines");
+    auto outlines = root.getKey("/Outlines");
     if (!(outlines.isDictionary() && outlines.hasKey("/First"))) {
         return;
     }
     QPDFObjectHandle cur = outlines.getKey("/First");
     QPDFObjGen::set seen;
     while (!cur.isNull() && seen.add(cur)) {
-        m->outlines.push_back(QPDFOutlineObjectHelper::Accessor::create(cur, *this, 1));
+        m->outlines.emplace_back(QPDFOutlineObjectHelper::Accessor::create(cur, *this, 1));
         cur = cur.getKey("/Next");
     }
 }
@@ -55,11 +75,10 @@ QPDFOutlineDocumentHelper::getOutlinesForPage(QPDFObjGen og)
     if (m->by_page.empty()) {
         initializeByPage();
     }
-    std::vector<QPDFOutlineObjectHelper> result;
     if (m->by_page.contains(og)) {
-        result = m->by_page[og];
+        return m->by_page[og];
     }
-    return result;
+    return {};
 }
 
 QPDFObjectHandle
@@ -70,13 +89,12 @@ QPDFOutlineDocumentHelper::resolveNamedDest(QPDFObjectHandle name)
         if (!m->dest_dict) {
             m->dest_dict = qpdf.getRoot().getKey("/Dests");
         }
-        QTC::TC("qpdf", "QPDFOutlineDocumentHelper name named dest");
         result = m->dest_dict.getKeyIfDict(name.getName());
     } else if (name.isString()) {
         if (!m->names_dest) {
             auto dests = qpdf.getRoot().getKey("/Names").getKeyIfDict("/Dests");
             if (dests.isDictionary()) {
-                m->names_dest = std::make_shared<QPDFNameTreeObjectHelper>(dests, qpdf);
+                m->names_dest = std::make_unique<QPDFNameTreeObjectHelper>(dests, qpdf);
             }
         }
         if (m->names_dest) {
@@ -89,7 +107,6 @@ QPDFOutlineDocumentHelper::resolveNamedDest(QPDFObjectHandle name)
         return QPDFObjectHandle::newNull();
     }
     if (result.isDictionary()) {
-        QTC::TC("qpdf", "QPDFOutlineDocumentHelper named dest dictionary");
         return result.getKey("/D");
     }
     return result;
diff --git a/qpdf/qpdf.testcov b/qpdf/qpdf.testcov
index 5292fd7..1ea1757 100644
--- a/qpdf/qpdf.testcov
+++ b/qpdf/qpdf.testcov
@@ -341,9 +341,7 @@ QPDFJob automatically set keep files open 1
 QPDFOutlineObjectHelper direct dest 0
 QPDFOutlineObjectHelper action dest 0
 QPDFOutlineObjectHelper named dest 0
-QPDFOutlineDocumentHelper name named dest 0
 QPDFOutlineDocumentHelper string named dest 0
-QPDFOutlineDocumentHelper named dest dictionary 0
 QPDFOutlineObjectHelper loop 0
 QPDFObjectHandle merge top type mismatch 0
 QPDFObjectHandle merge shallow copy 0
--
libgit2 0.21.4