Refactor `QPDFJob`: replace `infilename` with `infile_name()`, centralize input …

…file handling in `Inputs`, and simplify related logic.

Refactor `QPDFJob`: replace `infilename` with `infile_name()`, centralize input …
…file handling in `Inputs`, and simplify related logic.
m-holger
1 parent 0fae76cf
Showing 4 changed files with 52 additions and 29 deletions
libqpdf/QPDFJob.cc
libqpdf/QPDFJob_config.cc
libqpdf/qpdf/QPDFJob_private.hh
qpdf/qpdf.testcov
@@ -394,7 +394,7 @@ QPDFJob::createQPDF()
     checkConfiguration();
     std::unique_ptr<QPDF> pdf_sp;
     try {
-        processFile(pdf_sp, m->infilename.data(), m->password.data(), true, true);
+        processFile(pdf_sp, m->infile_nm(), m->password.data(), true, true);
     } catch (QPDFExc& e) {
         if (e.getErrorCode() == qpdf_e_password) {
             // Allow certain operations to work when an incorrect password is supplied.
@@ -567,10 +567,10 @@ QPDFJob::checkConfiguration()
         // standard output.
         m->outfilename = "-";
     }
-    if (m->infilename.empty() && !m->empty_input) {
+    if (m->infile_name().empty() && !m->empty_input) {
         usage("an input file name is required");
     }
-    if (m->replace_input && m->infilename.empty()) {
+    if (m->replace_input && m->infile_name().empty()) {
         usage("--replace-input may not be used with --empty");
     }
     if (m->require_outfile && m->outfilename.empty() && !m->replace_input) {
@@ -609,8 +609,7 @@ QPDFJob::checkConfiguration()
     if (save_to_stdout) {
         m->log->saveToStandardOutput(true);
     }
-    if (!m->split_pages && QUtil::same_file(m->infilename.data(), m->outfilename.data())) {
-        QTC::TC("qpdf", "QPDFJob same file error");
+    if (!m->split_pages && QUtil::same_file(m->infile_nm(), m->outfilename.data())) {
         usage(
             "input file and output file are the same; use --replace-input to intentionally "
             "overwrite the input file");
@@ -734,7 +733,7 @@ QPDFJob::doCheck(QPDF&amp; pdf)
     // may continue to perform additional checks after finding errors.
     bool okay = true;
     auto& cout = *m->log->getInfo();
-    cout << "checking " << m->infilename << "\n";
+    cout << "checking " << m->infile_name() << "\n";
     QPDF::JobSetter::setCheckMode(pdf, true);
     try {
         int extension_level = pdf.getExtensionLevel();
@@ -905,7 +904,7 @@ QPDFJob::doListAttachments(QPDF&amp; pdf)
             });
         }
     } else {
-        *m->log->getInfo() << m->infilename << " has no embedded files\n";
+        *m->log->getInfo() << m->infile_name() << " has no embedded files\n";
     }
 }
@@ -1671,7 +1670,6 @@ QPDFJob::doInspection(QPDF&amp; pdf)
         doCheck(pdf);
     }
     if (m->show_npages) {
-        QTC::TC("qpdf", "QPDFJob npages");
         cout << pdf.getRoot().getKey("/Pages").getKey("/Count").getIntValue() << "\n";
     }
     if (m->show_encryption) {
@@ -1679,9 +1677,9 @@ QPDFJob::doInspection(QPDF&amp; pdf)
     }
     if (m->check_linearization) {
         if (!pdf.isLinearized()) {
-            cout << m->infilename << " is not linearized\n";
+            cout << m->infile_name() << " is not linearized\n";
         } else if (pdf.checkLinearization()) {
-            cout << m->infilename << ": no linearization errors\n";
+            cout << m->infile_name() << ": no linearization errors\n";
         } else {
             m->warnings = true;
         }
@@ -1690,7 +1688,7 @@ QPDFJob::doInspection(QPDF&amp; pdf)
         if (pdf.isLinearized()) {
             pdf.showLinearizationData();
         } else {
-            cout << m->infilename << " is not linearized\n";
+            cout << m->infile_name() << " is not linearized\n";
         }
     }
     if (m->show_xref) {
@@ -1727,7 +1725,7 @@ QPDFJob::doProcessOnce(
     if (empty) {
         pdf->emptyPDF();
     } else if (main_input && m->json_input) {
-        pdf->createFromJSON(m->infilename);
+        pdf->createFromJSON(m->infile_name());
     } else {
         fn(pdf.get(), password);
     }
@@ -2431,13 +2429,13 @@ QPDFJob::handlePageSpecs(QPDF&amp; pdf)
     if (m->inputs.selections.empty()) {
         return;
     }
-    auto& main_input = m->inputs.files[m->infilename];
+    auto& main_input = m->inputs.files[m->infile_name()];
     main_input.initialize(m->inputs, &pdf);
     // Handle "." as a shortcut for the input file.
     for (auto& selection: m->inputs.selections) {
         if (selection.filename == ".") {
-            selection.filename = m->infilename;
+            selection.filename = m->infile_name();
         } else {
             // Force insertion
             (void)m->inputs.files[selection.filename];
@@ -3012,7 +3010,7 @@ QPDFJob::doSplitPages(QPDF&amp; pdf)
             page_range += "-" + QUtil::uint_to_string(last, QIntC::to_int(pageno_len));
         }
         std::string outfile = before + page_range + after;
-        if (QUtil::same_file(m->infilename.data(), outfile.data())) {
+        if (QUtil::same_file(m->infile_nm(), outfile.data())) {
             throw std::runtime_error("split pages would overwrite input file with " + outfile);
         }
         QPDFWriter w(outpdf, outfile.c_str());
@@ -3031,7 +3029,7 @@ QPDFJob::writeOutfile(QPDF&amp; pdf)
     if (m->replace_input) {
         // Append but don't prepend to the path to generate a temporary name. This saves us from
         // having to split the path by directory and non-directory.
-        temp_out = m->infilename + ".~qpdf-temp#";
+        temp_out = m->infile_name() + ".~qpdf-temp#";
         // m->outfilename will be restored to 0 before temp_out goes out of scope.
         m->outfilename = temp_out;
     } else if (m->outfilename == "-") {
@@ -3065,13 +3063,13 @@ QPDFJob::writeOutfile(QPDF&amp; pdf)
     if (m->replace_input) {
         // We must close the input before we can rename files
         pdf.closeInputSource();
-        std::string backup = m->infilename + ".~qpdf-orig";
+        std::string backup = m->infile_name() + ".~qpdf-orig";
         bool warnings = pdf.anyWarnings();
         if (!warnings) {
             backup.append(1, '#');
         }
-        QUtil::rename_file(m->infilename.data(), backup.data());
-        QUtil::rename_file(temp_out.data(), m->infilename.data());
+        QUtil::rename_file(m->infile_nm(), backup.data());
+        QUtil::rename_file(temp_out.data(), m->infile_nm());
         if (warnings) {
             *m->log->getError() << m->message_prefix
                                 << ": there are warnings; original file kept in " << backup << "\n";
@@ -15,18 +15,14 @@ QPDFJob::Config::checkConfiguration()
 QPDFJob::Config*
 QPDFJob::Config::inputFile(std::string const& filename)
 {
-    if (o.m->infilename.empty()) {
-        o.m->infilename = filename;
-    } else {
-        usage("input file has already been given");
-    }
+    o.m->inputs.infile_name(filename);
     return this;
 }
 QPDFJob::Config*
 QPDFJob::Config::emptyInput()
 {
-    if (o.m->infilename.empty()) {
+    if (o.m->infile_name().empty()) {
         // Various places in QPDFJob.cc used to know that the empty string for infile means empty.
         // This approach meant that passing "" as the argument to inputFile in job JSON, or
         // equivalently using "" as a positional command-line argument would be the same as
@@ -69,6 +69,22 @@ struct QPDFJob::Inputs
     void new_selection(
         std::string const& filename, std::string const& password, std::string const& range);
+    std::string const&
+    infile_name() const
+    {
+        return infile_name_;
+    }
+
+    void
+    infile_name(std::string const& name)
+    {
+        if (!infile_name_.empty()) {
+            usage("input file has already been given");
+        }
+        infile_name_ = name;
+    }
+
+    std::string infile_name_;
     std::string encryption_file;
     std::string encryption_file_password;
     bool keep_files_open{true};
@@ -145,6 +161,10 @@ class QPDFJob::Members
     Members(Members const&) = delete;
     ~Members() = default;
+    inline const char* infile_nm() const;
+
+    inline std::string const& infile_name() const;
+
   private:
     // These default values are duplicated in help and docs.
     static int constexpr DEFAULT_OI_MIN_WIDTH = 128;
@@ -272,7 +292,6 @@ class QPDFJob::Members
     bool replace_input{false};
     bool check_is_encrypted{false};
     bool check_requires_password{false};
-    std::string infilename;
     bool empty_input{false};
     std::string outfilename;
     bool json_input{false};
@@ -282,4 +301,16 @@ class QPDFJob::Members
     std::vector<PageLabelSpec> page_label_specs;
 };
-#endif // QPDFOBJECT_PRIVATE_HH
+inline const char*
+QPDFJob::Members::infile_nm() const
+{
+    return inputs.infile_name().data();
+}
+
+inline std::string const&
+QPDFJob::Members::infile_name() const
+{
+    return inputs.infile_name();
+}
+
+#endif // QPDFJOB_PRIVATE_HH
@@ -208,7 +208,6 @@ QPDF not caching overridden objstm object 0
 QPDF_optimization indirect outlines 0
 QPDF xref space 2
 QPDFJob pages range omitted in middle 0
-QPDFJob npages 0
 QPDFWriter standard deterministic ID 1
 QPDFWriter linearized deterministic ID 1
 qpdf-c called qpdf_set_deterministic_ID 0
@@ -221,7 +220,6 @@ QPDFParser found fake 1
 QPDFParser no val for last key 0
 QPDF resolve failure to null 0
 QPDFObjectHandle errors in parsecontent 0
-QPDFJob same file error 0
 QPDFJob split-pages %d 0
 QPDFJob split-pages .pdf 0
 QPDFJob split-pages other 0