From b53923d1a55d63bdff82d000bdb4dba9c54ddb58 Mon Sep 17 00:00:00 2001 From: m-holger Date: Tue, 13 May 2025 15:22:11 +0100 Subject: [PATCH] Refactor input file handling to use std::string and flag --- include/qpdf/QPDFJob.hh | 3 ++- libqpdf/QPDFJob.cc | 34 +++++++++++++++++----------------- libqpdf/QPDFJob_config.cc | 18 +++++++++--------- 3 files changed, 28 insertions(+), 27 deletions(-) diff --git a/include/qpdf/QPDFJob.hh b/include/qpdf/QPDFJob.hh index 68a141c..0b85bd6 100644 --- a/include/qpdf/QPDFJob.hh +++ b/include/qpdf/QPDFJob.hh @@ -711,7 +711,8 @@ class QPDFJob bool replace_input{false}; bool check_is_encrypted{false}; bool check_requires_password{false}; - std::shared_ptr infilename; + std::string infilename; + bool empty_input{false}; std::string outfilename; bool json_input{false}; bool json_output{false}; diff --git a/libqpdf/QPDFJob.cc b/libqpdf/QPDFJob.cc index ff0afce..0bd4090 100644 --- a/libqpdf/QPDFJob.cc +++ b/libqpdf/QPDFJob.cc @@ -440,7 +440,7 @@ QPDFJob::createQPDF() checkConfiguration(); std::unique_ptr pdf_sp; try { - processFile(pdf_sp, m->infilename.get(), m->password.get(), true, true); + processFile(pdf_sp, m->infilename.data(), m->password.get(), true, true); } catch (QPDFExc& e) { if (e.getErrorCode() == qpdf_e_password) { // Allow certain operations to work when an incorrect password is supplied. @@ -619,10 +619,10 @@ QPDFJob::checkConfiguration() // standard output. m->outfilename = "-"; } - if (!m->infilename) { + if (m->infilename.empty() && !m->empty_input) { usage("an input file name is required"); } - if (m->replace_input && (strlen(m->infilename.get()) == 0)) { + if (m->replace_input && m->infilename.empty()) { usage("--replace-input may not be used with --empty"); } if (m->require_outfile && m->outfilename.empty() && !m->replace_input) { @@ -661,7 +661,7 @@ QPDFJob::checkConfiguration() if (save_to_stdout) { m->log->saveToStandardOutput(true); } - if (!m->split_pages && QUtil::same_file(m->infilename.get(), m->outfilename.data())) { + if (!m->split_pages && QUtil::same_file(m->infilename.data(), m->outfilename.data())) { QTC::TC("qpdf", "QPDFJob same file error"); usage( "input file and output file are the same; use --replace-input to intentionally " @@ -786,7 +786,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.get() << "\n"; + cout << "checking " << m->infilename << "\n"; QPDF::JobSetter::setCheckMode(pdf, true); try { int extension_level = pdf.getExtensionLevel(); @@ -947,7 +947,7 @@ QPDFJob::doListAttachments(QPDF& pdf) }); } } else { - *m->log->getInfo() << m->infilename.get() << " has no embedded files\n"; + *m->log->getInfo() << m->infilename << " has no embedded files\n"; } } @@ -1722,9 +1722,9 @@ QPDFJob::doInspection(QPDF& pdf) } if (m->check_linearization) { if (!pdf.isLinearized()) { - cout << m->infilename.get() << " is not linearized\n"; + cout << m->infilename << " is not linearized\n"; } else if (pdf.checkLinearization()) { - cout << m->infilename.get() << ": no linearization errors\n"; + cout << m->infilename << ": no linearization errors\n"; } else { m->warnings = true; } @@ -1733,7 +1733,7 @@ QPDFJob::doInspection(QPDF& pdf) if (pdf.isLinearized()) { pdf.showLinearizationData(); } else { - cout << m->infilename.get() << " is not linearized\n"; + cout << m->infilename << " is not linearized\n"; } } if (m->show_xref) { @@ -1770,7 +1770,7 @@ QPDFJob::doProcessOnce( if (empty) { pdf->emptyPDF(); } else if (main_input && m->json_input) { - pdf->createFromJSON(m->infilename.get()); + pdf->createFromJSON(m->infilename); } else { fn(pdf.get(), password); } @@ -2432,7 +2432,7 @@ QPDFJob::handlePageSpecs(QPDF& pdf, std::vector>& page_hea // Handle "." as a shortcut for the input file for (auto& page_spec: m->page_specs) { if (page_spec.filename == ".") { - page_spec.filename = m->infilename.get(); + page_spec.filename = m->infilename; } if (page_spec.range.empty()) { page_spec.range = "1-z"; @@ -2458,7 +2458,7 @@ QPDFJob::handlePageSpecs(QPDF& pdf, std::vector>& page_hea // Create a QPDF object for each file that we may take pages from. std::map page_spec_qpdfs; std::map page_spec_cfis; - page_spec_qpdfs[m->infilename.get()] = &pdf; + page_spec_qpdfs[m->infilename] = &pdf; std::vector parsed_specs; std::map> copied_pages; for (auto& page_spec: m->page_specs) { @@ -3087,7 +3087,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.get(), outfile.c_str())) { + if (QUtil::same_file(m->infilename.data(), outfile.data())) { throw std::runtime_error("split pages would overwrite input file with " + outfile); } QPDFWriter w(outpdf, outfile.c_str()); @@ -3106,7 +3106,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 = std::string(m->infilename.get()) + ".~qpdf-temp#"; + temp_out = m->infilename + ".~qpdf-temp#"; // m->outfilename will be restored to 0 before temp_out goes out of scope. m->outfilename = temp_out; } else if (m->outfilename == "-") { @@ -3140,13 +3140,13 @@ QPDFJob::writeOutfile(QPDF& pdf) if (m->replace_input) { // We must close the input before we can rename files pdf.closeInputSource(); - std::string backup = std::string(m->infilename.get()) + ".~qpdf-orig"; + std::string backup = m->infilename + ".~qpdf-orig"; bool warnings = pdf.anyWarnings(); if (!warnings) { backup.append(1, '#'); } - QUtil::rename_file(m->infilename.get(), backup.c_str()); - QUtil::rename_file(temp_out.data(), m->infilename.get()); + QUtil::rename_file(m->infilename.data(), backup.data()); + QUtil::rename_file(temp_out.data(), m->infilename.data()); 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 3f56337..cfa878a 100644 --- a/libqpdf/QPDFJob_config.cc +++ b/libqpdf/QPDFJob_config.cc @@ -15,8 +15,8 @@ QPDFJob::Config::checkConfiguration() QPDFJob::Config* QPDFJob::Config::inputFile(std::string const& filename) { - if (o.m->infilename == nullptr) { - o.m->infilename = QUtil::make_shared_cstr(filename); + if (o.m->infilename.empty()) { + o.m->infilename = filename; } else { usage("input file has already been given"); } @@ -26,13 +26,13 @@ QPDFJob::Config::inputFile(std::string const& filename) QPDFJob::Config* QPDFJob::Config::emptyInput() { - if (o.m->infilename == nullptr) { - // Various places in QPDFJob.cc know that the empty string for infile means empty. We set it - // to something other than a null pointer as an indication that some input source has been - // specified. This approach means that passing "" as the argument to inputFile in job JSON, - // or equivalently using "" as a positional command-line argument would be the same as - // --empty. This probably isn't worth blocking or coding around. - o.m->infilename = QUtil::make_shared_cstr(""); + if (o.m->infilename.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 + // --empty. This was deemed to be not worth blocking or coding around. This no longer holds + // from 12.3. + o.m->empty_input = true; } else { usage("empty input can't be used since input file has already been given"); } -- libgit2 0.21.4