From 0356bcecc5f58b64d2cf02c68ca1934446cea9cb Mon Sep 17 00:00:00 2001
From: m-holger <m-holger@kubitscheck.org>
Date: Tue, 12 Jul 2022 18:40:30 +0100
Subject: [PATCH] Tidy QPDF::pushInheritedAttributesToPageInternal

Remove unnecessary parameters.
Remove code that is unnecessary as result of a prior call to QPDF::getAllPages.
Avoid clearing and rebuilding of m->all_pages.
---
 include/qpdf/QPDF.hh         |  4 +-
 libqpdf/QPDF_optimization.cc | 86 +++++++++---------------------------
 2 files changed, 22 insertions(+), 68 deletions(-)

diff --git a/include/qpdf/QPDF.hh b/include/qpdf/QPDF.hh
index d563fe8f..64f31edd 100644
--- a/include/qpdf/QPDF.hh
+++ b/include/qpdf/QPDF.hh
@@ -1616,10 +1616,8 @@ class QPDF
     void pushInheritedAttributesToPageInternal(
         QPDFObjectHandle,
         std::map<std::string, std::vector<QPDFObjectHandle>>&,
-        std::vector<QPDFObjectHandle>& all_pages,
         bool allow_changes,
-        bool warn_skipped_keys,
-        std::set<QPDFObjGen>& visited);
+        bool warn_skipped_keys);
     void updateObjectMaps(
         ObjUser const& ou,
         QPDFObjectHandle oh,
diff --git a/libqpdf/QPDF_optimization.cc b/libqpdf/QPDF_optimization.cc
index cafdbe64..4797be5e 100644
--- a/libqpdf/QPDF_optimization.cc
+++ b/libqpdf/QPDF_optimization.cc
@@ -148,15 +148,11 @@ QPDF::pushInheritedAttributesToPage(bool allow_changes, bool warn_skipped_keys)
     // key_ancestors is a mapping of page attribute keys to a stack of
     // Pages nodes that contain values for them.
     std::map<std::string, std::vector<QPDFObjectHandle>> key_ancestors;
-    this->m->all_pages.clear();
-    std::set<QPDFObjGen> visited;
     pushInheritedAttributesToPageInternal(
         this->m->trailer.getKey("/Root").getKey("/Pages"),
         key_ancestors,
-        this->m->all_pages,
         allow_changes,
-        warn_skipped_keys,
-        visited);
+        warn_skipped_keys);
     if (!key_ancestors.empty()) {
         throw std::logic_error("key_ancestors not empty after"
                                " pushing inherited attributes to pages");
@@ -169,35 +165,9 @@ void
 QPDF::pushInheritedAttributesToPageInternal(
     QPDFObjectHandle cur_pages,
     std::map<std::string, std::vector<QPDFObjectHandle>>& key_ancestors,
-    std::vector<QPDFObjectHandle>& pages,
     bool allow_changes,
-    bool warn_skipped_keys,
-    std::set<QPDFObjGen>& visited)
+    bool warn_skipped_keys)
 {
-    QPDFObjGen this_og = cur_pages.getObjGen();
-    if (visited.count(this_og) > 0) {
-        throw QPDFExc(
-            qpdf_e_pages,
-            this->m->file->getName(),
-            this->m->last_object_description,
-            0,
-            "Loop detected in /Pages structure (inherited attributes)");
-    }
-    visited.insert(this_og);
-
-    if (!cur_pages.isDictionary()) {
-        throw QPDFExc(
-            qpdf_e_damaged_pdf,
-            this->m->file->getName(),
-            this->m->last_object_description,
-            this->m->file->getLastOffset(),
-            "invalid object in page tree");
-    }
-
-    // Extract the underlying dictionary object
-    std::string type = cur_pages.getKey("/Type").getName();
-
-    if (type == "/Pages") {
         // Make a list of inheritable keys. Only the keys /MediaBox,
         // /CropBox, /Resources, and /Rotate are inheritable
         // attributes. Push this object onto the stack of pages nodes
@@ -265,17 +235,25 @@ QPDF::pushInheritedAttributesToPageInternal(
             }
         }
 
-        // Visit descendant nodes.
-        QPDFObjectHandle kids = cur_pages.getKey("/Kids");
-        int n = kids.getArrayNItems();
-        for (int i = 0; i < n; ++i) {
-            pushInheritedAttributesToPageInternal(
-                kids.getArrayItem(i),
-                key_ancestors,
-                pages,
-                allow_changes,
-                warn_skipped_keys,
-                visited);
+        // Process descendant nodes.
+        for (auto& kid: cur_pages.getKey("/Kids").aitems()) {
+            if (kid.isDictionaryOfType("/Pages")) {
+                pushInheritedAttributesToPageInternal(
+                    kid, key_ancestors, allow_changes, warn_skipped_keys);
+            } else {
+                // Add all available inheritable attributes not present in
+                // this object to this object.
+                for (auto const& iter: key_ancestors) {
+                    std::string const& key = iter.first;
+                    if (!kid.hasKey(key)) {
+                        QTC::TC("qpdf", "QPDF opt resource inherited");
+                        kid.replaceKey(key, iter.second.back());
+                    } else {
+                        QTC::TC(
+                            "qpdf", "QPDF opt page resource hides ancestor");
+                    }
+                }
+            }
         }
 
         // For each inheritable key, pop the stack.  If the stack
@@ -295,28 +273,6 @@ QPDF::pushInheritedAttributesToPageInternal(
         } else {
             QTC::TC("qpdf", "QPDF opt no inheritable keys");
         }
-    } else if (type == "/Page") {
-        // Add all available inheritable attributes not present in
-        // this object to this object.
-        for (auto const& iter: key_ancestors) {
-            std::string const& key = iter.first;
-            if (!cur_pages.hasKey(key)) {
-                QTC::TC("qpdf", "QPDF opt resource inherited");
-                cur_pages.replaceKey(key, iter.second.back());
-            } else {
-                QTC::TC("qpdf", "QPDF opt page resource hides ancestor");
-            }
-        }
-        pages.push_back(cur_pages);
-    } else {
-        throw QPDFExc(
-            qpdf_e_damaged_pdf,
-            this->m->file->getName(),
-            this->m->last_object_description,
-            this->m->file->getLastOffset(),
-            "invalid Type " + type + " in page tree");
-    }
-    visited.erase(this_og);
 }
 
 void