From ac039c8525407361477f041550db6fb0efbc497a Mon Sep 17 00:00:00 2001 From: m-holger Date: Wed, 17 Sep 2025 14:31:17 +0100 Subject: [PATCH] Refactor `QPDFJob`: replace `infilename` with `infile_name()`, centralize input file handling in `Inputs`, and simplify related logic. --- libqpdf/QPDFJob.cc | 36 +++++++++++++++++------------------- libqpdf/QPDFJob_config.cc | 8 ++------ libqpdf/qpdf/QPDFJob_private.hh | 35 +++++++++++++++++++++++++++++++++-- qpdf/qpdf.testcov | 2 -- 4 files changed, 52 insertions(+), 29 deletions(-) diff --git a/libqpdf/QPDFJob.cc b/libqpdf/QPDFJob.cc index 5731e4a..5f01dbb 100644 --- a/libqpdf/QPDFJob.cc +++ b/libqpdf/QPDFJob.cc @@ -394,7 +394,7 @@ QPDFJob::createQPDF() checkConfiguration(); std::unique_ptr 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& 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& 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& 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& 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& 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& 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& 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& 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& 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"; diff --git a/libqpdf/QPDFJob_config.cc b/libqpdf/QPDFJob_config.cc index fb58d52..e1c9ea2 100644 --- a/libqpdf/QPDFJob_config.cc +++ b/libqpdf/QPDFJob_config.cc @@ -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 diff --git a/libqpdf/qpdf/QPDFJob_private.hh b/libqpdf/qpdf/QPDFJob_private.hh index 28bdced..18668a5 100644 --- a/libqpdf/qpdf/QPDFJob_private.hh +++ b/libqpdf/qpdf/QPDFJob_private.hh @@ -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 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 diff --git a/qpdf/qpdf.testcov b/qpdf/qpdf.testcov index 24dea97..31f6b21 100644 --- a/qpdf/qpdf.testcov +++ b/qpdf/qpdf.testcov @@ -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 -- libgit2 0.21.4