Refactor `QPDFJob::Selection` and `Inputs`: replace `Selection::filename` with `…

…input_entry`, centralize input file handling in `infile_name`, update `new_selection` logic, and simplify page range and input processing.

Refactor `QPDFJob::Selection` and `Inputs`: replace `Selection::filename` with `…
…input_entry`, centralize input file handling in `infile_name`, update `new_selection` logic, and simplify page range and input processing.
m-holger
1 parent 2c6a3e97
Showing 4 changed files with 78 additions and 37 deletions
include/qpdf/QPDFJob.hh
libqpdf/QPDFJob.cc
libqpdf/QPDFJob_config.cc
libqpdf/qpdf/QPDFJob_private.hh
@@ -463,8 +463,6 @@ class QPDFJob
     void setQPDFOptions(QPDF& pdf);
     void handlePageSpecs(QPDF& pdf);
     bool shouldRemoveUnreferencedResources(QPDF& pdf);
-    void new_selection(
-        std::string const& filename, std::string const& password, std::string const& range);
     void handleRotations(QPDF& pdf);
     void getUOPagenos(
         std::vector<UnderOverlay>& uo, std::vector<std::map<size_t, std::vector<int>>>& pagenos);
@@ -2347,10 +2347,25 @@ QPDFJob::Input::initialize(Inputs&amp; in, QPDF* a_qpdf)
 }
  
 void
-QPDFJob::Inputs::new_selection(
-    std::string const& filename, std::string const& password, std::string const& range)
+QPDFJob::Inputs::infile_name(std::string const& name)
 {
-    selections.emplace_back(filename, password, range);
+    if (!infile_name_.empty()) {
+        usage("input file has already been given");
+    }
+    infile_name_ = name;
+
+    auto& in_entry = *files.insert({name, Input()}).first;
+    auto it = files.find("");
+    if (it != files.end()) {
+        // We allready have selection entries for the main input file. We need to fix them to point
+        // to the correct files entry.
+        for (auto& selection: selections) {
+            if (selection.in_entry == &*it) {
+                selection.in_entry = &in_entry;
+            }
+        }
+        files.erase(it);
+    }
 }
  
 void
@@ -2394,6 +2409,9 @@ QPDFJob::Inputs::process_all()
         selection.process(*this);
     }
  
+    if (!infile_name().empty()) {
+        files.erase("");
+    }
     if (!keep_files_open_set) {
         // Count the number of distinct files to determine whether we should keep files open or not.
         // Rather than trying to code some portable heuristic based on OS limits, just hard-code
@@ -2415,20 +2433,20 @@ QPDFJob::Inputs::process_all()
     for (auto& selection: selections) {
         // Read original pages from the PDF, and parse the page range associated with this
         // occurrence of the file.
-        auto const& file_spec = files[selection.filename];
+        auto const& input = selection.input();
         if (selection.range.empty()) {
-            selection.selected_pages.reserve(static_cast<size_t>(file_spec.n_pages));
-            for (int i = 1; i <= file_spec.n_pages; ++i) {
+            selection.selected_pages.reserve(static_cast<size_t>(input.n_pages));
+            for (int i = 1; i <= input.n_pages; ++i) {
                 selection.selected_pages.push_back(i);
             }
             continue;
         }
         try {
             selection.selected_pages =
-                QUtil::parse_numrange(selection.range.data(), files[selection.filename].n_pages);
+                QUtil::parse_numrange(selection.range.data(), selection.input().n_pages);
         } catch (std::runtime_error& e) {
             throw std::runtime_error(
-                "parsing numeric range for " + selection.filename + ": " + e.what());
+                "parsing numeric range for " + selection.filename() + ": " + e.what());
         }
     }
 }
@@ -2446,16 +2464,47 @@ QPDFJob::Inputs::clear()
     return any_warnings;
 }
  
+QPDFJob::Selection&
+QPDFJob::Inputs::new_selection(std::string const& filename)
+{
+    // Handle "." as a shortcut for the input file. Note that infile_name may not be known yet, in
+    // which case we are wrongly entering an empty name. This will be corrected in the infile_name
+    // setter.
+    return selections.emplace_back(
+        *files.insert({(filename == "." ? infile_name() : filename), Input()}).first);
+}
+
+void
+QPDFJob::Inputs::new_selection(
+    std::string const& filename, std::string const& password, std::string const& range)
+{
+    auto& selection = new_selection(filename);
+    selection.password = password;
+    selection.range = range;
+}
+
+QPDFJob::Selection::Selection(std::pair<const std::string, QPDFJob::Input>& entry) :
+    in_entry(&entry)
+{
+}
+
+QPDFJob::Input&
+QPDFJob::Selection::input()
+{
+    return in_entry->second;
+}
+
+std::string const&
+QPDFJob::Selection::filename()
+{
+    return in_entry->first;
+}
+
 void
 QPDFJob::Selection::process(QPDFJob::Inputs& in)
 {
-    // Handle "." as a shortcut for the input file.
-    if (filename == ".") {
-        filename = in.infile_name();
-    }
-    auto& input = in.files[filename];
     if (!password.empty()) {
-        input.password = password;
+        input().password = password;
     }
 }
  
@@ -2528,7 +2577,7 @@ QPDFJob::handlePageSpecs(QPDF&amp; pdf)
     auto& this_afdh = pdf.acroform();
     std::set<QPDFObjGen> referenced_fields;
     for (auto& selection: new_specs.empty() ? m->inputs.selections : new_specs) {
-        auto& input = m->inputs.files[selection.filename];
+        auto& input = selection.input();
         if (input.cfis) {
             input.cfis->stayOpen(true);
         }
@@ -2538,7 +2587,7 @@ QPDFJob::handlePageSpecs(QPDF&amp; pdf)
             any_page_labels = true;
         }
         doIfVerbose([&](Pipeline& v, std::string const& prefix) {
-            v << prefix << ": adding pages from " << selection.filename << "\n";
+            v << prefix << ": adding pages from " << selection.filename() << "\n";
         });
         for (auto pageno_iter: selection.selected_pages) {
             // Pages are specified from 1 but numbered from 0 in the vector
@@ -1001,7 +1001,7 @@ QPDFJob::PagesConfig::pageSpec(
 QPDFJob::PagesConfig*
 QPDFJob::PagesConfig::file(std::string const& arg)
 {
-    config->o.m->inputs.new_selection(arg, {}, {});
+    (void)config->o.m->inputs.new_selection(arg);
     return this;
 }
  
@@ -10,25 +10,24 @@
 // single clause in the --pages option.
 struct QPDFJob::Selection
 {
-    Selection(std::string const& filename, std::string const& password, std::string const& range) :
-        filename(filename),
-        password(password),
-        range(range)
-    {
-    }
+    Selection() = delete;
+
+    Selection(std::pair<const std::string, QPDFJob::Input>& entry);
  
     Selection(QPDFJob::Selection const& other, int page) :
-        filename(other.filename),
-        // range and password are no longer required when this constructor is called.
+        in_entry(other.in_entry),
         selected_pages({page})
     {
     }
  
+    QPDFJob::Input& input();
+    std::string const& filename();
+
     void process(Inputs& in);
  
-    std::string filename;
+    std::pair<const std::string, QPDFJob::Input>* in_entry{nullptr};
     std::string password;
-    std::string range;
+    std::string range; // An empty range means all pages.
     std::vector<int> selected_pages;
 };
  
@@ -66,6 +65,8 @@ struct QPDFJob::Inputs
     // Destroy all owned QPDF objects. Return false if any of the QPDF objects recorded warnings.
     bool clear();
  
+    Selection& new_selection(std::string const& filename);
+
     void new_selection(
         std::string const& filename, std::string const& password, std::string const& range);
  
@@ -75,14 +76,7 @@ struct QPDFJob::Inputs
         return infile_name_;
     }
  
-    void
-    infile_name(std::string const& name)
-    {
-        if (!infile_name_.empty()) {
-            usage("input file has already been given");
-        }
-        infile_name_ = name;
-    }
+    void infile_name(std::string const& name);
  
     std::string infile_name_;
     std::string encryption_file;