Commit b53923d1a55d63bdff82d000bdb4dba9c54ddb58
1 parent
3a42f63a
Refactor input file handling to use std::string and flag
Replaced raw shared pointers with std::string for `infilename` to improve simplicity and readability. Added a boolean `empty_input` flag to explicitly track empty input cases. Adjusted related logic and function calls accordingly for consistency.
Showing
3 changed files
with
28 additions
and
27 deletions
include/qpdf/QPDFJob.hh
| @@ -711,7 +711,8 @@ class QPDFJob | @@ -711,7 +711,8 @@ class QPDFJob | ||
| 711 | bool replace_input{false}; | 711 | bool replace_input{false}; |
| 712 | bool check_is_encrypted{false}; | 712 | bool check_is_encrypted{false}; |
| 713 | bool check_requires_password{false}; | 713 | bool check_requires_password{false}; |
| 714 | - std::shared_ptr<char> infilename; | 714 | + std::string infilename; |
| 715 | + bool empty_input{false}; | ||
| 715 | std::string outfilename; | 716 | std::string outfilename; |
| 716 | bool json_input{false}; | 717 | bool json_input{false}; |
| 717 | bool json_output{false}; | 718 | bool json_output{false}; |
libqpdf/QPDFJob.cc
| @@ -440,7 +440,7 @@ QPDFJob::createQPDF() | @@ -440,7 +440,7 @@ QPDFJob::createQPDF() | ||
| 440 | checkConfiguration(); | 440 | checkConfiguration(); |
| 441 | std::unique_ptr<QPDF> pdf_sp; | 441 | std::unique_ptr<QPDF> pdf_sp; |
| 442 | try { | 442 | try { |
| 443 | - processFile(pdf_sp, m->infilename.get(), m->password.get(), true, true); | 443 | + processFile(pdf_sp, m->infilename.data(), m->password.get(), true, true); |
| 444 | } catch (QPDFExc& e) { | 444 | } catch (QPDFExc& e) { |
| 445 | if (e.getErrorCode() == qpdf_e_password) { | 445 | if (e.getErrorCode() == qpdf_e_password) { |
| 446 | // Allow certain operations to work when an incorrect password is supplied. | 446 | // Allow certain operations to work when an incorrect password is supplied. |
| @@ -619,10 +619,10 @@ QPDFJob::checkConfiguration() | @@ -619,10 +619,10 @@ QPDFJob::checkConfiguration() | ||
| 619 | // standard output. | 619 | // standard output. |
| 620 | m->outfilename = "-"; | 620 | m->outfilename = "-"; |
| 621 | } | 621 | } |
| 622 | - if (!m->infilename) { | 622 | + if (m->infilename.empty() && !m->empty_input) { |
| 623 | usage("an input file name is required"); | 623 | usage("an input file name is required"); |
| 624 | } | 624 | } |
| 625 | - if (m->replace_input && (strlen(m->infilename.get()) == 0)) { | 625 | + if (m->replace_input && m->infilename.empty()) { |
| 626 | usage("--replace-input may not be used with --empty"); | 626 | usage("--replace-input may not be used with --empty"); |
| 627 | } | 627 | } |
| 628 | if (m->require_outfile && m->outfilename.empty() && !m->replace_input) { | 628 | if (m->require_outfile && m->outfilename.empty() && !m->replace_input) { |
| @@ -661,7 +661,7 @@ QPDFJob::checkConfiguration() | @@ -661,7 +661,7 @@ QPDFJob::checkConfiguration() | ||
| 661 | if (save_to_stdout) { | 661 | if (save_to_stdout) { |
| 662 | m->log->saveToStandardOutput(true); | 662 | m->log->saveToStandardOutput(true); |
| 663 | } | 663 | } |
| 664 | - if (!m->split_pages && QUtil::same_file(m->infilename.get(), m->outfilename.data())) { | 664 | + if (!m->split_pages && QUtil::same_file(m->infilename.data(), m->outfilename.data())) { |
| 665 | QTC::TC("qpdf", "QPDFJob same file error"); | 665 | QTC::TC("qpdf", "QPDFJob same file error"); |
| 666 | usage( | 666 | usage( |
| 667 | "input file and output file are the same; use --replace-input to intentionally " | 667 | "input file and output file are the same; use --replace-input to intentionally " |
| @@ -786,7 +786,7 @@ QPDFJob::doCheck(QPDF& pdf) | @@ -786,7 +786,7 @@ QPDFJob::doCheck(QPDF& pdf) | ||
| 786 | // may continue to perform additional checks after finding errors. | 786 | // may continue to perform additional checks after finding errors. |
| 787 | bool okay = true; | 787 | bool okay = true; |
| 788 | auto& cout = *m->log->getInfo(); | 788 | auto& cout = *m->log->getInfo(); |
| 789 | - cout << "checking " << m->infilename.get() << "\n"; | 789 | + cout << "checking " << m->infilename << "\n"; |
| 790 | QPDF::JobSetter::setCheckMode(pdf, true); | 790 | QPDF::JobSetter::setCheckMode(pdf, true); |
| 791 | try { | 791 | try { |
| 792 | int extension_level = pdf.getExtensionLevel(); | 792 | int extension_level = pdf.getExtensionLevel(); |
| @@ -947,7 +947,7 @@ QPDFJob::doListAttachments(QPDF& pdf) | @@ -947,7 +947,7 @@ QPDFJob::doListAttachments(QPDF& pdf) | ||
| 947 | }); | 947 | }); |
| 948 | } | 948 | } |
| 949 | } else { | 949 | } else { |
| 950 | - *m->log->getInfo() << m->infilename.get() << " has no embedded files\n"; | 950 | + *m->log->getInfo() << m->infilename << " has no embedded files\n"; |
| 951 | } | 951 | } |
| 952 | } | 952 | } |
| 953 | 953 | ||
| @@ -1722,9 +1722,9 @@ QPDFJob::doInspection(QPDF& pdf) | @@ -1722,9 +1722,9 @@ QPDFJob::doInspection(QPDF& pdf) | ||
| 1722 | } | 1722 | } |
| 1723 | if (m->check_linearization) { | 1723 | if (m->check_linearization) { |
| 1724 | if (!pdf.isLinearized()) { | 1724 | if (!pdf.isLinearized()) { |
| 1725 | - cout << m->infilename.get() << " is not linearized\n"; | 1725 | + cout << m->infilename << " is not linearized\n"; |
| 1726 | } else if (pdf.checkLinearization()) { | 1726 | } else if (pdf.checkLinearization()) { |
| 1727 | - cout << m->infilename.get() << ": no linearization errors\n"; | 1727 | + cout << m->infilename << ": no linearization errors\n"; |
| 1728 | } else { | 1728 | } else { |
| 1729 | m->warnings = true; | 1729 | m->warnings = true; |
| 1730 | } | 1730 | } |
| @@ -1733,7 +1733,7 @@ QPDFJob::doInspection(QPDF& pdf) | @@ -1733,7 +1733,7 @@ QPDFJob::doInspection(QPDF& pdf) | ||
| 1733 | if (pdf.isLinearized()) { | 1733 | if (pdf.isLinearized()) { |
| 1734 | pdf.showLinearizationData(); | 1734 | pdf.showLinearizationData(); |
| 1735 | } else { | 1735 | } else { |
| 1736 | - cout << m->infilename.get() << " is not linearized\n"; | 1736 | + cout << m->infilename << " is not linearized\n"; |
| 1737 | } | 1737 | } |
| 1738 | } | 1738 | } |
| 1739 | if (m->show_xref) { | 1739 | if (m->show_xref) { |
| @@ -1770,7 +1770,7 @@ QPDFJob::doProcessOnce( | @@ -1770,7 +1770,7 @@ QPDFJob::doProcessOnce( | ||
| 1770 | if (empty) { | 1770 | if (empty) { |
| 1771 | pdf->emptyPDF(); | 1771 | pdf->emptyPDF(); |
| 1772 | } else if (main_input && m->json_input) { | 1772 | } else if (main_input && m->json_input) { |
| 1773 | - pdf->createFromJSON(m->infilename.get()); | 1773 | + pdf->createFromJSON(m->infilename); |
| 1774 | } else { | 1774 | } else { |
| 1775 | fn(pdf.get(), password); | 1775 | fn(pdf.get(), password); |
| 1776 | } | 1776 | } |
| @@ -2432,7 +2432,7 @@ QPDFJob::handlePageSpecs(QPDF& pdf, std::vector<std::unique_ptr<QPDF>>& page_hea | @@ -2432,7 +2432,7 @@ QPDFJob::handlePageSpecs(QPDF& pdf, std::vector<std::unique_ptr<QPDF>>& page_hea | ||
| 2432 | // Handle "." as a shortcut for the input file | 2432 | // Handle "." as a shortcut for the input file |
| 2433 | for (auto& page_spec: m->page_specs) { | 2433 | for (auto& page_spec: m->page_specs) { |
| 2434 | if (page_spec.filename == ".") { | 2434 | if (page_spec.filename == ".") { |
| 2435 | - page_spec.filename = m->infilename.get(); | 2435 | + page_spec.filename = m->infilename; |
| 2436 | } | 2436 | } |
| 2437 | if (page_spec.range.empty()) { | 2437 | if (page_spec.range.empty()) { |
| 2438 | page_spec.range = "1-z"; | 2438 | page_spec.range = "1-z"; |
| @@ -2458,7 +2458,7 @@ QPDFJob::handlePageSpecs(QPDF& pdf, std::vector<std::unique_ptr<QPDF>>& page_hea | @@ -2458,7 +2458,7 @@ QPDFJob::handlePageSpecs(QPDF& pdf, std::vector<std::unique_ptr<QPDF>>& page_hea | ||
| 2458 | // Create a QPDF object for each file that we may take pages from. | 2458 | // Create a QPDF object for each file that we may take pages from. |
| 2459 | std::map<std::string, QPDF*> page_spec_qpdfs; | 2459 | std::map<std::string, QPDF*> page_spec_qpdfs; |
| 2460 | std::map<std::string, ClosedFileInputSource*> page_spec_cfis; | 2460 | std::map<std::string, ClosedFileInputSource*> page_spec_cfis; |
| 2461 | - page_spec_qpdfs[m->infilename.get()] = &pdf; | 2461 | + page_spec_qpdfs[m->infilename] = &pdf; |
| 2462 | std::vector<QPDFPageData> parsed_specs; | 2462 | std::vector<QPDFPageData> parsed_specs; |
| 2463 | std::map<unsigned long long, std::set<QPDFObjGen>> copied_pages; | 2463 | std::map<unsigned long long, std::set<QPDFObjGen>> copied_pages; |
| 2464 | for (auto& page_spec: m->page_specs) { | 2464 | for (auto& page_spec: m->page_specs) { |
| @@ -3087,7 +3087,7 @@ QPDFJob::doSplitPages(QPDF& pdf) | @@ -3087,7 +3087,7 @@ QPDFJob::doSplitPages(QPDF& pdf) | ||
| 3087 | page_range += "-" + QUtil::uint_to_string(last, QIntC::to_int(pageno_len)); | 3087 | page_range += "-" + QUtil::uint_to_string(last, QIntC::to_int(pageno_len)); |
| 3088 | } | 3088 | } |
| 3089 | std::string outfile = before + page_range + after; | 3089 | std::string outfile = before + page_range + after; |
| 3090 | - if (QUtil::same_file(m->infilename.get(), outfile.c_str())) { | 3090 | + if (QUtil::same_file(m->infilename.data(), outfile.data())) { |
| 3091 | throw std::runtime_error("split pages would overwrite input file with " + outfile); | 3091 | throw std::runtime_error("split pages would overwrite input file with " + outfile); |
| 3092 | } | 3092 | } |
| 3093 | QPDFWriter w(outpdf, outfile.c_str()); | 3093 | QPDFWriter w(outpdf, outfile.c_str()); |
| @@ -3106,7 +3106,7 @@ QPDFJob::writeOutfile(QPDF& pdf) | @@ -3106,7 +3106,7 @@ QPDFJob::writeOutfile(QPDF& pdf) | ||
| 3106 | if (m->replace_input) { | 3106 | if (m->replace_input) { |
| 3107 | // Append but don't prepend to the path to generate a temporary name. This saves us from | 3107 | // Append but don't prepend to the path to generate a temporary name. This saves us from |
| 3108 | // having to split the path by directory and non-directory. | 3108 | // having to split the path by directory and non-directory. |
| 3109 | - temp_out = std::string(m->infilename.get()) + ".~qpdf-temp#"; | 3109 | + temp_out = m->infilename + ".~qpdf-temp#"; |
| 3110 | // m->outfilename will be restored to 0 before temp_out goes out of scope. | 3110 | // m->outfilename will be restored to 0 before temp_out goes out of scope. |
| 3111 | m->outfilename = temp_out; | 3111 | m->outfilename = temp_out; |
| 3112 | } else if (m->outfilename == "-") { | 3112 | } else if (m->outfilename == "-") { |
| @@ -3140,13 +3140,13 @@ QPDFJob::writeOutfile(QPDF& pdf) | @@ -3140,13 +3140,13 @@ QPDFJob::writeOutfile(QPDF& pdf) | ||
| 3140 | if (m->replace_input) { | 3140 | if (m->replace_input) { |
| 3141 | // We must close the input before we can rename files | 3141 | // We must close the input before we can rename files |
| 3142 | pdf.closeInputSource(); | 3142 | pdf.closeInputSource(); |
| 3143 | - std::string backup = std::string(m->infilename.get()) + ".~qpdf-orig"; | 3143 | + std::string backup = m->infilename + ".~qpdf-orig"; |
| 3144 | bool warnings = pdf.anyWarnings(); | 3144 | bool warnings = pdf.anyWarnings(); |
| 3145 | if (!warnings) { | 3145 | if (!warnings) { |
| 3146 | backup.append(1, '#'); | 3146 | backup.append(1, '#'); |
| 3147 | } | 3147 | } |
| 3148 | - QUtil::rename_file(m->infilename.get(), backup.c_str()); | ||
| 3149 | - QUtil::rename_file(temp_out.data(), m->infilename.get()); | 3148 | + QUtil::rename_file(m->infilename.data(), backup.data()); |
| 3149 | + QUtil::rename_file(temp_out.data(), m->infilename.data()); | ||
| 3150 | if (warnings) { | 3150 | if (warnings) { |
| 3151 | *m->log->getError() << m->message_prefix | 3151 | *m->log->getError() << m->message_prefix |
| 3152 | << ": there are warnings; original file kept in " << backup << "\n"; | 3152 | << ": there are warnings; original file kept in " << backup << "\n"; |
libqpdf/QPDFJob_config.cc
| @@ -15,8 +15,8 @@ QPDFJob::Config::checkConfiguration() | @@ -15,8 +15,8 @@ QPDFJob::Config::checkConfiguration() | ||
| 15 | QPDFJob::Config* | 15 | QPDFJob::Config* |
| 16 | QPDFJob::Config::inputFile(std::string const& filename) | 16 | QPDFJob::Config::inputFile(std::string const& filename) |
| 17 | { | 17 | { |
| 18 | - if (o.m->infilename == nullptr) { | ||
| 19 | - o.m->infilename = QUtil::make_shared_cstr(filename); | 18 | + if (o.m->infilename.empty()) { |
| 19 | + o.m->infilename = filename; | ||
| 20 | } else { | 20 | } else { |
| 21 | usage("input file has already been given"); | 21 | usage("input file has already been given"); |
| 22 | } | 22 | } |
| @@ -26,13 +26,13 @@ QPDFJob::Config::inputFile(std::string const& filename) | @@ -26,13 +26,13 @@ QPDFJob::Config::inputFile(std::string const& filename) | ||
| 26 | QPDFJob::Config* | 26 | QPDFJob::Config* |
| 27 | QPDFJob::Config::emptyInput() | 27 | QPDFJob::Config::emptyInput() |
| 28 | { | 28 | { |
| 29 | - if (o.m->infilename == nullptr) { | ||
| 30 | - // Various places in QPDFJob.cc know that the empty string for infile means empty. We set it | ||
| 31 | - // to something other than a null pointer as an indication that some input source has been | ||
| 32 | - // specified. This approach means that passing "" as the argument to inputFile in job JSON, | ||
| 33 | - // or equivalently using "" as a positional command-line argument would be the same as | ||
| 34 | - // --empty. This probably isn't worth blocking or coding around. | ||
| 35 | - o.m->infilename = QUtil::make_shared_cstr(""); | 29 | + if (o.m->infilename.empty()) { |
| 30 | + // Various places in QPDFJob.cc used to know that the empty string for infile means empty. | ||
| 31 | + // This approach meant that passing "" as the argument to inputFile in job JSON, or | ||
| 32 | + // equivalently using "" as a positional command-line argument would be the same as | ||
| 33 | + // --empty. This was deemed to be not worth blocking or coding around. This no longer holds | ||
| 34 | + // from 12.3. | ||
| 35 | + o.m->empty_input = true; | ||
| 36 | } else { | 36 | } else { |
| 37 | usage("empty input can't be used since input file has already been given"); | 37 | usage("empty input can't be used since input file has already been given"); |
| 38 | } | 38 | } |