From 1a8c2eb93b3116a3057e8009b8cbd7510abaf138 Mon Sep 17 00:00:00 2001
From: Jay Berkenbilt <ejb@ql.org>
Date: Sat, 22 Jan 2022 17:37:51 -0500
Subject: [PATCH] QPDFJob: use std::shared_ptr over PointerHolder where
 possible

Also fix QPDFArgParser
---
 include/qpdf/QPDFArgParser.hh | 12 ++---
 include/qpdf/QPDFJob.hh       | 18 ++++----
 libqpdf/QPDFArgParser.cc      | 39 +++++++---------
 libqpdf/QPDFJob.cc            | 83 +++++++++++++++++------------------
 libqpdf/QPDFJob_argv.cc       |  6 +--
 5 files changed, 75 insertions(+), 83 deletions(-)

diff --git a/include/qpdf/QPDFArgParser.hh b/include/qpdf/QPDFArgParser.hh
index 12ade54b..108fc019 100644
--- a/include/qpdf/QPDFArgParser.hh
+++ b/include/qpdf/QPDFArgParser.hh
@@ -23,7 +23,7 @@
 #define QPDFARGPARSER_HH
 
 #include <qpdf/DLL.h>
-#include <qpdf/PointerHolder.hh>
+#include <memory>
 #include <string>
 #include <set>
 #include <map>
@@ -325,15 +325,15 @@ class QPDFArgParser
         option_table_t* option_table;
         std::string option_table_name;
         bare_arg_handler_t final_check_handler;
-        std::vector<PointerHolder<char>> new_argv;
-        std::vector<PointerHolder<char>> bash_argv;
-        PointerHolder<char*> argv_ph;
-        PointerHolder<char*> bash_argv_ph;
+        std::vector<std::shared_ptr<char>> new_argv;
+        std::vector<std::shared_ptr<char>> bash_argv;
+        std::shared_ptr<char*> argv_ph;
+        std::shared_ptr<char*> bash_argv_ph;
         std::map<std::string, HelpTopic> help_topics;
         std::map<std::string, HelpTopic> option_help;
         std::string help_footer;
     };
-    PointerHolder<Members> m;
+    std::shared_ptr<Members> m;
 };
 
 #endif // QPDFARGPARSER_HH
diff --git a/include/qpdf/QPDFJob.hh b/include/qpdf/QPDFJob.hh
index f2e3a019..03035957 100644
--- a/include/qpdf/QPDFJob.hh
+++ b/include/qpdf/QPDFJob.hh
@@ -24,11 +24,11 @@
 
 #include <qpdf/DLL.h>
 #include <qpdf/Constants.h>
-#include <qpdf/PointerHolder.hh>
 #include <qpdf/QPDF.hh>
 #include <qpdf/QPDFPageObjectHelper.hh>
 #include <qpdf/QPDFArgParser.hh>
 
+#include <memory>
 #include <string>
 #include <list>
 #include <vector>
@@ -167,7 +167,7 @@ class QPDFJob
         char const* to_nr;
         char const* from_nr;
         char const* repeat_nr;
-        PointerHolder<QPDF> pdf;
+        std::shared_ptr<QPDF> pdf;
         std::vector<int> to_pagenos;
         std::vector<int> from_pagenos;
         std::vector<int> repeat_pagenos;
@@ -316,14 +316,14 @@ class QPDFJob
 
   private:
     // Basic file processing
-    PointerHolder<QPDF> processFile(
+    std::shared_ptr<QPDF> processFile(
         char const* filename, char const* password);
-    PointerHolder<QPDF> processInputSource(
+    std::shared_ptr<QPDF> processInputSource(
         PointerHolder<InputSource> is, char const* password);
-    PointerHolder<QPDF> doProcess(
+    std::shared_ptr<QPDF> doProcess(
         std::function<void(QPDF*, char const*)> fn,
         char const* password, bool empty);
-    PointerHolder<QPDF> doProcessOnce(
+    std::shared_ptr<QPDF> doProcessOnce(
         std::function<void(QPDF*, char const*)> fn,
         char const* password, bool empty);
 
@@ -331,7 +331,7 @@ class QPDFJob
     void setQPDFOptions(QPDF& pdf);
     void handlePageSpecs(
         QPDF& pdf, bool& warnings,
-        std::vector<PointerHolder<QPDF>>& page_heap);
+        std::vector<std::shared_ptr<QPDF>>& page_heap);
     bool shouldRemoveUnreferencedResources(QPDF& pdf);
     void handleRotations(QPDF& pdf);
     void handleUnderOverlay(QPDF& pdf);
@@ -395,9 +395,9 @@ class QPDFJob
         std::ostream* cout;
         std::ostream* cerr;
         unsigned long encryption_status;
-        PointerHolder<QPDFArgParser> ap;
+        std::shared_ptr<QPDFArgParser> ap;
     };
-    PointerHolder<Members> m;
+    std::shared_ptr<Members> m;
 };
 
 #endif // QPDFOBJECT_HH
diff --git a/libqpdf/QPDFArgParser.cc b/libqpdf/QPDFArgParser.cc
index 8d7c978c..33d50a10 100644
--- a/libqpdf/QPDFArgParser.cc
+++ b/libqpdf/QPDFArgParser.cc
@@ -295,10 +295,8 @@ QPDFArgParser::handleArgFileArguments()
 {
     // Support reading arguments from files. Create a new argv. Ensure
     // that argv itself as well as all its contents are automatically
-    // deleted by using PointerHolder objects to back the pointers in
-    // argv.
-    this->m->new_argv.push_back(
-        PointerHolder<char>(true, QUtil::copy_string(this->m->argv[0])));
+    // deleted by using shared pointers to back the pointers in argv.
+    this->m->new_argv.push_back(QUtil::make_shared_cstr(this->m->argv[0]));
     for (int i = 1; i < this->m->argc; ++i)
     {
         char* argfile = 0;
@@ -321,16 +319,16 @@ QPDFArgParser::handleArgFileArguments()
         else
         {
             this->m->new_argv.push_back(
-                PointerHolder<char>(
-                    true, QUtil::copy_string(this->m->argv[i])));
+                QUtil::make_shared_cstr(this->m->argv[i]));
         }
     }
-    this->m->argv_ph =
-        PointerHolder<char*>(true, new char*[1 + this->m->new_argv.size()]);
-    this->m->argv = this->m->argv_ph.getPointer();
+    this->m->argv_ph = std::shared_ptr<char*>(
+        new char*[1 + this->m->new_argv.size()],
+        std::default_delete<char*[]>());
+    this->m->argv = this->m->argv_ph.get();
     for (size_t i = 0; i < this->m->new_argv.size(); ++i)
     {
-        this->m->argv[i] = this->m->new_argv.at(i).getPointer();
+        this->m->argv[i] = this->m->new_argv.at(i).get();
     }
     this->m->argc = QIntC::to_int(this->m->new_argv.size());
     this->m->argv[this->m->argc] = 0;
@@ -374,8 +372,7 @@ QPDFArgParser::handleBashArguments()
                     if (! arg.empty())
                     {
                         this->m->bash_argv.push_back(
-                            PointerHolder<char>(
-                                true, QUtil::copy_string(arg.c_str())));
+                            QUtil::make_shared_cstr(arg));
                         arg.clear();
                     }
                 }
@@ -426,17 +423,17 @@ QPDFArgParser::handleBashArguments()
         // This can't happen if properly invoked by bash, but ensure
         // we have a valid argv[0] regardless.
         this->m->bash_argv.push_back(
-            PointerHolder<char>(
-                true, QUtil::copy_string(this->m->argv[0])));
+            QUtil::make_shared_cstr(this->m->argv[0]));
     }
     // Explicitly discard any non-space-terminated word. The "current
     // word" is handled specially.
-    this->m->bash_argv_ph =
-        PointerHolder<char*>(true, new char*[1 + this->m->bash_argv.size()]);
-    this->m->argv = this->m->bash_argv_ph.getPointer();
+    this->m->bash_argv_ph = std::shared_ptr<char*>(
+        new char*[1 + this->m->bash_argv.size()],
+        std::default_delete<char*[]>());
+    this->m->argv = this->m->bash_argv_ph.get();
     for (size_t i = 0; i < this->m->bash_argv.size(); ++i)
     {
-        this->m->argv[i] = this->m->bash_argv.at(i).getPointer();
+        this->m->argv[i] = this->m->bash_argv.at(i).get();
     }
     this->m->argc = QIntC::to_int(this->m->bash_argv.size());
     this->m->argv[this->m->argc] = 0;
@@ -467,11 +464,9 @@ QPDFArgParser::readArgsFromFile(char const* filename)
         QTC::TC("libtests", "QPDFArgParser read args from file");
         lines = QUtil::read_lines_from_file(filename);
     }
-    for (std::list<std::string>::iterator iter = lines.begin();
-         iter != lines.end(); ++iter)
+    for (auto const& line: lines)
     {
-        this->m->new_argv.push_back(
-            PointerHolder<char>(true, QUtil::copy_string((*iter).c_str())));
+        this->m->new_argv.push_back(QUtil::make_shared_cstr(line));
     }
 }
 
diff --git a/libqpdf/QPDFJob.cc b/libqpdf/QPDFJob.cc
index 4676d872..c6eb128e 100644
--- a/libqpdf/QPDFJob.cc
+++ b/libqpdf/QPDFJob.cc
@@ -17,7 +17,6 @@
 #include <qpdf/Pl_DCT.hh>
 #include <qpdf/Pl_Count.hh>
 #include <qpdf/Pl_Flate.hh>
-#include <qpdf/PointerHolder.hh>
 
 #include <qpdf/QPDF.hh>
 #include <qpdf/QPDFPageDocumentHelper.hh>
@@ -45,7 +44,7 @@ namespace
         }
         virtual void provideStreamData(int objid, int generation,
                                        Pipeline* pipeline);
-        PointerHolder<Pipeline> makePipeline(
+        std::shared_ptr<Pipeline> makePipeline(
             std::string const& description, Pipeline* next);
         bool evaluate(std::string const& description);
 
@@ -103,10 +102,10 @@ ImageOptimizer::ImageOptimizer(QPDFJob& o, QPDFObjectHandle& image) :
 {
 }
 
-PointerHolder<Pipeline>
+std::shared_ptr<Pipeline>
 ImageOptimizer::makePipeline(std::string const& description, Pipeline* next)
 {
-    PointerHolder<Pipeline> result;
+    std::shared_ptr<Pipeline> result;
     QPDFObjectHandle dict = image.getDict();
     QPDFObjectHandle w_obj = dict.getKey("/Width");
     QPDFObjectHandle h_obj = dict.getKey("/Height");
@@ -207,7 +206,7 @@ ImageOptimizer::makePipeline(std::string const& description, Pipeline* next)
         return result;
     }
 
-    result = new Pl_DCT("jpg", next, w, h, components, cs);
+    result = std::make_shared<Pl_DCT>("jpg", next, w, h, components, cs);
     return result;
 }
 
@@ -227,13 +226,13 @@ ImageOptimizer::evaluate(std::string const& description)
     }
     Pl_Discard d;
     Pl_Count c("count", &d);
-    PointerHolder<Pipeline> p = makePipeline(description, &c);
-    if (p.getPointer() == 0)
+    std::shared_ptr<Pipeline> p = makePipeline(description, &c);
+    if (p.get() == nullptr)
     {
         // message issued by makePipeline
         return false;
     }
-    if (! image.pipeStreamData(p.getPointer(), 0, qpdf_dl_specialized))
+    if (! image.pipeStreamData(p.get(), 0, qpdf_dl_specialized))
     {
         return false;
     }
@@ -260,8 +259,8 @@ ImageOptimizer::evaluate(std::string const& description)
 void
 ImageOptimizer::provideStreamData(int, int, Pipeline* pipeline)
 {
-    PointerHolder<Pipeline> p = makePipeline("", pipeline);
-    if (p.getPointer() == 0)
+    std::shared_ptr<Pipeline> p = makePipeline("", pipeline);
+    if (p.get() == nullptr)
     {
         // Should not be possible
         image.warnIfPossible("unable to create pipeline after previous"
@@ -269,7 +268,7 @@ ImageOptimizer::provideStreamData(int, int, Pipeline* pipeline)
         pipeline->finish();
         return;
     }
-    image.pipeStreamData(p.getPointer(), 0, qpdf_dl_specialized,
+    image.pipeStreamData(p.get(), 0, qpdf_dl_specialized,
                          false, false);
 }
 
@@ -450,7 +449,7 @@ void
 QPDFJob::run()
 {
     QPDFJob& o = *this; // QXXXQ
-    PointerHolder<QPDF> pdf_ph;
+    std::shared_ptr<QPDF> pdf_ph;
     try
     {
         pdf_ph = processFile(o.infilename, o.password);
@@ -480,7 +479,7 @@ QPDFJob::run()
         return;
     }
     bool other_warnings = false;
-    std::vector<PointerHolder<QPDF>> page_heap;
+    std::vector<std::shared_ptr<QPDF>> page_heap;
     if (! o.page_specs.empty())
     {
         handlePageSpecs(pdf, other_warnings, page_heap);
@@ -1793,12 +1792,12 @@ QPDFJob::doInspection(QPDF& pdf)
     }
 }
 
-PointerHolder<QPDF>
+std::shared_ptr<QPDF>
 QPDFJob::doProcessOnce(
     std::function<void(QPDF*, char const*)> fn,
     char const* password, bool empty)
 {
-    PointerHolder<QPDF> pdf = new QPDF;
+    auto pdf = std::make_shared<QPDF>();
     setQPDFOptions(*pdf);
     if (empty)
     {
@@ -1806,12 +1805,12 @@ QPDFJob::doProcessOnce(
     }
     else
     {
-        fn(pdf.getPointer(), password);
+        fn(pdf.get(), password);
     }
     return pdf;
 }
 
-PointerHolder<QPDF>
+std::shared_ptr<QPDF>
 QPDFJob::doProcess(
     std::function<void(QPDF*, char const*)> fn,
     char const* password, bool empty)
@@ -1904,7 +1903,7 @@ QPDFJob::doProcess(
     throw std::logic_error("do_process returned");
 }
 
-PointerHolder<QPDF>
+std::shared_ptr<QPDF>
 QPDFJob::processFile(char const* filename, char const* password)
 {
     auto f1 = std::mem_fn<void(char const*, char const*)>(&QPDF::processFile);
@@ -1913,7 +1912,7 @@ QPDFJob::processFile(char const* filename, char const* password)
     return doProcess(fn, password, strcmp(filename, "") == 0);
 }
 
-PointerHolder<QPDF>
+std::shared_ptr<QPDF>
 QPDFJob::processInputSource(
     PointerHolder<InputSource> is, char const* password)
 {
@@ -1969,15 +1968,15 @@ QPDFJob::validateUnderOverlay(QPDF& pdf, QPDFJob::UnderOverlay* uo)
 
 static QPDFAcroFormDocumentHelper* get_afdh_for_qpdf(
     std::map<unsigned long long,
-             PointerHolder<QPDFAcroFormDocumentHelper>>& afdh_map,
+             std::shared_ptr<QPDFAcroFormDocumentHelper>>& afdh_map,
     QPDF* q)
 {
     auto uid = q->getUniqueId();
     if (! afdh_map.count(uid))
     {
-        afdh_map[uid] = new QPDFAcroFormDocumentHelper(*q);
+        afdh_map[uid] = std::make_shared<QPDFAcroFormDocumentHelper>(*q);
     }
-    return afdh_map[uid].getPointer();
+    return afdh_map[uid].get();
 }
 
 void
@@ -1999,7 +1998,7 @@ QPDFJob::doUnderOverlayForPage(
     }
 
     std::map<unsigned long long,
-             PointerHolder<QPDFAcroFormDocumentHelper>> afdh;
+             std::shared_ptr<QPDFAcroFormDocumentHelper>> afdh;
     auto make_afdh = [&](QPDFPageObjectHelper& ph) {
         QPDF* q = ph.getObjectHandle().getOwningQPDF();
         return get_afdh_for_qpdf(afdh, q);
@@ -2098,8 +2097,8 @@ QPDFJob::handleUnderOverlay(QPDF& pdf)
     QPDFJob& o = *this; // QXXXQ
     validateUnderOverlay(pdf, &o.underlay);
     validateUnderOverlay(pdf, &o.overlay);
-    if ((0 == o.underlay.pdf.getPointer()) &&
-        (0 == o.overlay.pdf.getPointer()))
+    if ((nullptr == o.underlay.pdf.get()) &&
+        (nullptr == o.overlay.pdf.get()))
     {
         return;
     }
@@ -2110,12 +2109,12 @@ QPDFJob::handleUnderOverlay(QPDF& pdf)
     std::map<int, QPDFObjectHandle> underlay_fo;
     std::map<int, QPDFObjectHandle> overlay_fo;
     std::vector<QPDFPageObjectHelper> upages;
-    if (o.underlay.pdf.getPointer())
+    if (o.underlay.pdf.get())
     {
         upages = QPDFPageDocumentHelper(*(o.underlay.pdf)).getAllPages();
     }
     std::vector<QPDFPageObjectHelper> opages;
-    if (o.overlay.pdf.getPointer())
+    if (o.overlay.pdf.get())
     {
         opages = QPDFPageDocumentHelper(*(o.overlay.pdf)).getAllPages();
     }
@@ -2275,11 +2274,11 @@ QPDFJob::handleTransformations(QPDF& pdf)
 {
     QPDFJob& o = *this; // QXXXQ
     QPDFPageDocumentHelper dh(pdf);
-    PointerHolder<QPDFAcroFormDocumentHelper> afdh;
+    std::shared_ptr<QPDFAcroFormDocumentHelper> afdh;
     auto make_afdh = [&]() {
-        if (! afdh.getPointer())
+        if (! afdh.get())
         {
-            afdh = new QPDFAcroFormDocumentHelper(pdf);
+            afdh = std::make_shared<QPDFAcroFormDocumentHelper>(pdf);
         }
     };
     if (o.externalize_inline_images ||
@@ -2351,7 +2350,7 @@ QPDFJob::handleTransformations(QPDF& pdf)
         make_afdh();
         for (auto& page: dh.getAllPages())
         {
-            page.flattenRotation(afdh.getPointer());
+            page.flattenRotation(afdh.get());
         }
     }
     if (o.remove_page_labels)
@@ -2543,7 +2542,7 @@ static QPDFObjectHandle added_page(QPDF& pdf, QPDFPageObjectHelper page)
 void
 QPDFJob::handlePageSpecs(
     QPDF& pdf, bool& warnings,
-    std::vector<PointerHolder<QPDF>>& page_heap)
+    std::vector<std::shared_ptr<QPDF>>& page_heap)
 {
     QPDFJob& o = *this; // QXXXQ
     // Parse all page specifications and translate them into lists of
@@ -2594,7 +2593,7 @@ QPDFJob::handlePageSpecs(
         if (page_spec_qpdfs.count(page_spec.filename) == 0)
         {
             // Open the PDF file and store the QPDF object. Throw a
-            // PointerHolder to the qpdf into a heap so that it
+            // std::shared_ptr to the qpdf into a heap so that it
             // survives through copying to the output but gets cleaned up
             // automatically at the end. Do not canonicalize the file
             // name. Using two different paths to refer to the same
@@ -2630,9 +2629,9 @@ QPDFJob::handlePageSpecs(
                 is = fis;
                 fis->setFilename(page_spec.filename.c_str());
             }
-            PointerHolder<QPDF> qpdf_ph = processInputSource(is, password);
+            std::shared_ptr<QPDF> qpdf_ph = processInputSource(is, password);
             page_heap.push_back(qpdf_ph);
-            page_spec_qpdfs[page_spec.filename] = qpdf_ph.getPointer();
+            page_spec_qpdfs[page_spec.filename] = qpdf_ph.get();
             if (cis)
             {
                 cis->stayOpen(false);
@@ -2735,7 +2734,7 @@ QPDFJob::handlePageSpecs(
     bool any_page_labels = false;
     int out_pageno = 0;
     std::map<unsigned long long,
-             PointerHolder<QPDFAcroFormDocumentHelper>> afdh_map;
+             std::shared_ptr<QPDFAcroFormDocumentHelper>> afdh_map;
     auto this_afdh = get_afdh_for_qpdf(afdh_map, &pdf);
     std::set<QPDFObjGen> referenced_fields;
     for (std::vector<QPDFPageData>::iterator iter =
@@ -3147,8 +3146,8 @@ QPDFJob::setEncryptionOptions(QPDF& pdf, QPDFWriter& w)
 static void parse_version(std::string const& full_version_string,
                           std::string& version, int& extension_level)
 {
-    PointerHolder<char> vp(true, QUtil::copy_string(full_version_string));
-    char* v = vp.getPointer();
+    auto vp = QUtil::make_shared_cstr(full_version_string);
+    char* v = vp.get();
     char* p1 = strchr(v, '.');
     char* p2 = (p1 ? strchr(1 + p1, '.') : 0);
     if (p2 && *(p2 + 1))
@@ -3221,7 +3220,7 @@ QPDFJob::setWriterOptions(QPDF& pdf, QPDFWriter& w)
     }
     if (o.copy_encryption)
     {
-        PointerHolder<QPDF> encryption_pdf =
+        std::shared_ptr<QPDF> encryption_pdf =
             processFile(o.encryption_file, o.encryption_file_password);
         w.copyEncryptionParameters(*encryption_pdf);
     }
@@ -3313,10 +3312,10 @@ QPDFJob::doSplitPages(QPDF& pdf, bool& warnings)
         }
         QPDF outpdf;
         outpdf.emptyPDF();
-        PointerHolder<QPDFAcroFormDocumentHelper> out_afdh;
+        std::shared_ptr<QPDFAcroFormDocumentHelper> out_afdh;
         if (afdh.hasAcroForm())
         {
-            out_afdh = new QPDFAcroFormDocumentHelper(outpdf);
+            out_afdh = std::make_shared<QPDFAcroFormDocumentHelper>(outpdf);
         }
         if (o.suppress_warnings)
         {
@@ -3327,7 +3326,7 @@ QPDFJob::doSplitPages(QPDF& pdf, bool& warnings)
             QPDFObjectHandle page = pages.at(pageno - 1);
             outpdf.addPage(page, false);
             auto new_page = added_page(outpdf, page);
-            if (out_afdh.getPointer())
+            if (out_afdh.get())
             {
                 QTC::TC("qpdf", "qpdf copy form fields in split_pages");
                 try
diff --git a/libqpdf/QPDFJob_argv.cc b/libqpdf/QPDFJob_argv.cc
index 647311bf..715446d3 100644
--- a/libqpdf/QPDFJob_argv.cc
+++ b/libqpdf/QPDFJob_argv.cc
@@ -198,9 +198,7 @@ ArgParser::argPasswordFile(char* parameter)
     if (lines.size() >= 1)
     {
         // Make sure the memory for this stays in scope.
-        o.password_alloc = std::shared_ptr<char>(
-            QUtil::copy_string(lines.front().c_str()),
-            std::default_delete<char[]>());
+        o.password_alloc = QUtil::make_shared_cstr(lines.front());
         o.password = o.password_alloc.get();
 
         if (lines.size() > 1)
@@ -1535,7 +1533,7 @@ QPDFJob::initializeFromArgv(int argc, char* argv[], char const* progname_env)
     // QPDFArgParser must stay in scope for the life of the QPDFJob
     // object since it holds dynamic memory used for argv, which is
     // pointed to by other member variables.
-    this->m->ap = new QPDFArgParser(argc, argv, progname_env);
+    this->m->ap = std::make_shared<QPDFArgParser>(argc, argv, progname_env);
     setMessagePrefix(this->m->ap->getProgname());
     ArgParser ap(*this->m->ap, *this);
     ap.parseOptions();