Commit ce3617a30a35af601a97f74cb9745648c9e679d8
Committed by
GitHub
Merge pull request #1607 from m-holger/cover
Refactor error handling and simplify logic across multiple pipelines
Showing
7 changed files
with
76 additions
and
82 deletions
libqpdf/InputSource.cc
| ... | ... | @@ -2,10 +2,13 @@ |
| 2 | 2 | |
| 3 | 3 | #include <qpdf/QIntC.hh> |
| 4 | 4 | #include <qpdf/QTC.hh> |
| 5 | +#include <qpdf/Util.hh> | |
| 6 | + | |
| 5 | 7 | #include <cstring> |
| 6 | 8 | #include <stdexcept> |
| 7 | 9 | |
| 8 | 10 | using namespace std::literals; |
| 11 | +using namespace qpdf; | |
| 9 | 12 | |
| 10 | 13 | void |
| 11 | 14 | InputSource::setLastOffset(qpdf_offset_t offset) |
| ... | ... | @@ -73,10 +76,10 @@ InputSource::findFirst(char const* start_chars, qpdf_offset_t offset, size_t len |
| 73 | 76 | // To enable us to guarantee null-termination, save an extra byte so that buf[size] is valid |
| 74 | 77 | // memory. |
| 75 | 78 | size_t size = sizeof(buf) - 1; |
| 76 | - if ((strlen(start_chars) < 1) || (strlen(start_chars) > size)) { | |
| 77 | - throw std::logic_error( | |
| 78 | - "InputSource::findSource called with too small or too large of a character sequence"); | |
| 79 | - } | |
| 79 | + util::assertion( | |
| 80 | + !(strlen(start_chars) < 1 || strlen(start_chars) > size), | |
| 81 | + "InputSource::findSource called with too small or too large of a character sequence" // | |
| 82 | + ); | |
| 80 | 83 | |
| 81 | 84 | char* p = nullptr; |
| 82 | 85 | qpdf_offset_t buf_offset = offset; |
| ... | ... | @@ -155,7 +158,6 @@ InputSource::findFirst(char const* start_chars, qpdf_offset_t offset, size_t len |
| 155 | 158 | p = buf + bytes_read; |
| 156 | 159 | } |
| 157 | 160 | } |
| 158 | - throw std::logic_error("InputSource after while (true)"); | |
| 159 | 161 | } |
| 160 | 162 | |
| 161 | 163 | bool | ... | ... |
libqpdf/OffsetInputSource.cc
| 1 | 1 | #include <qpdf/OffsetInputSource.hh> |
| 2 | 2 | |
| 3 | +#include <qpdf/Util.hh> | |
| 4 | + | |
| 3 | 5 | #include <limits> |
| 4 | 6 | #include <sstream> |
| 5 | 7 | #include <stdexcept> |
| 6 | 8 | |
| 9 | +using namespace qpdf; | |
| 10 | + | |
| 7 | 11 | OffsetInputSource::OffsetInputSource( |
| 8 | 12 | std::shared_ptr<InputSource> proxied, qpdf_offset_t global_offset) : |
| 9 | 13 | proxied(proxied), |
| 10 | 14 | global_offset(global_offset) |
| 11 | 15 | { |
| 12 | - if (global_offset < 0) { | |
| 13 | - throw std::logic_error("OffsetInputSource constructed with negative offset"); | |
| 14 | - } | |
| 15 | - this->max_safe_offset = std::numeric_limits<qpdf_offset_t>::max() - global_offset; | |
| 16 | + util::assertion(global_offset >= 0, "OffsetInputSource constructed with negative offset"); | |
| 17 | + max_safe_offset = std::numeric_limits<qpdf_offset_t>::max() - global_offset; | |
| 16 | 18 | } |
| 17 | 19 | |
| 18 | 20 | qpdf_offset_t |
| 19 | 21 | OffsetInputSource::findAndSkipNextEOL() |
| 20 | 22 | { |
| 21 | - return this->proxied->findAndSkipNextEOL() - this->global_offset; | |
| 23 | + return proxied->findAndSkipNextEOL() - global_offset; | |
| 22 | 24 | } |
| 23 | 25 | |
| 24 | 26 | std::string const& |
| 25 | 27 | OffsetInputSource::getName() const |
| 26 | 28 | { |
| 27 | - return this->proxied->getName(); | |
| 29 | + return proxied->getName(); | |
| 28 | 30 | } |
| 29 | 31 | |
| 30 | 32 | qpdf_offset_t |
| 31 | 33 | OffsetInputSource::tell() |
| 32 | 34 | { |
| 33 | - return this->proxied->tell() - this->global_offset; | |
| 35 | + return proxied->tell() - global_offset; | |
| 34 | 36 | } |
| 35 | 37 | |
| 36 | 38 | void |
| 37 | 39 | OffsetInputSource::seek(qpdf_offset_t offset, int whence) |
| 38 | 40 | { |
| 39 | 41 | if (whence == SEEK_SET) { |
| 40 | - if (offset > this->max_safe_offset) { | |
| 42 | + if (offset > max_safe_offset) { | |
| 41 | 43 | std::ostringstream msg; |
| 42 | 44 | msg.imbue(std::locale::classic()); |
| 43 | 45 | msg << "seeking to " << offset << " offset by " << global_offset |
| 44 | 46 | << " would cause an overflow of the offset type"; |
| 45 | 47 | throw std::range_error(msg.str()); |
| 46 | 48 | } |
| 47 | - this->proxied->seek(offset + global_offset, whence); | |
| 49 | + proxied->seek(offset + global_offset, whence); | |
| 48 | 50 | } else { |
| 49 | - this->proxied->seek(offset, whence); | |
| 50 | - } | |
| 51 | - if (tell() < 0) { | |
| 52 | - throw std::runtime_error("offset input source: seek before beginning of file"); | |
| 51 | + proxied->seek(offset, whence); | |
| 53 | 52 | } |
| 53 | + util::no_ci_rt_error_if(tell() < 0, "offset input source: seek before beginning of file"); | |
| 54 | 54 | } |
| 55 | 55 | |
| 56 | 56 | void |
| ... | ... | @@ -62,13 +62,13 @@ OffsetInputSource::rewind() |
| 62 | 62 | size_t |
| 63 | 63 | OffsetInputSource::read(char* buffer, size_t length) |
| 64 | 64 | { |
| 65 | - size_t result = this->proxied->read(buffer, length); | |
| 66 | - this->setLastOffset(this->proxied->getLastOffset() - global_offset); | |
| 65 | + size_t result = proxied->read(buffer, length); | |
| 66 | + setLastOffset(proxied->getLastOffset() - global_offset); | |
| 67 | 67 | return result; |
| 68 | 68 | } |
| 69 | 69 | |
| 70 | 70 | void |
| 71 | 71 | OffsetInputSource::unreadCh(char ch) |
| 72 | 72 | { |
| 73 | - this->proxied->unreadCh(ch); | |
| 73 | + proxied->unreadCh(ch); | |
| 74 | 74 | } | ... | ... |
libqpdf/Pl_PNGFilter.cc
| ... | ... | @@ -2,11 +2,14 @@ |
| 2 | 2 | |
| 3 | 3 | #include <qpdf/QTC.hh> |
| 4 | 4 | #include <qpdf/QUtil.hh> |
| 5 | +#include <qpdf/Util.hh> | |
| 5 | 6 | |
| 6 | 7 | #include <climits> |
| 7 | 8 | #include <cstring> |
| 8 | 9 | #include <stdexcept> |
| 9 | 10 | |
| 11 | +using namespace qpdf; | |
| 12 | + | |
| 10 | 13 | namespace |
| 11 | 14 | { |
| 12 | 15 | unsigned long long memory_limit{0}; |
| ... | ... | @@ -28,25 +31,19 @@ Pl_PNGFilter::Pl_PNGFilter( |
| 28 | 31 | Pipeline(identifier, next), |
| 29 | 32 | action(action) |
| 30 | 33 | { |
| 31 | - if (!next) { | |
| 32 | - throw std::logic_error("Attempt to create Pl_PNGFilter with nullptr as next"); | |
| 33 | - } | |
| 34 | - if (samples_per_pixel < 1) { | |
| 35 | - throw std::runtime_error("PNGFilter created with invalid samples_per_pixel"); | |
| 36 | - } | |
| 37 | - if (!((bits_per_sample == 1) || (bits_per_sample == 2) || (bits_per_sample == 4) || | |
| 38 | - (bits_per_sample == 8) || (bits_per_sample == 16))) { | |
| 39 | - throw std::runtime_error( | |
| 40 | - "PNGFilter created with invalid bits_per_sample not 1, 2, 4, 8, or 16"); | |
| 41 | - } | |
| 34 | + util::assertion(next, "Attempt to create Pl_PNGFilter with nullptr as next"); | |
| 35 | + util::no_ci_rt_error_if( | |
| 36 | + samples_per_pixel < 1, "PNGFilter created with invalid samples_per_pixel"); | |
| 37 | + util::no_ci_rt_error_if( | |
| 38 | + !(bits_per_sample == 1 || bits_per_sample == 2 || bits_per_sample == 4 || | |
| 39 | + bits_per_sample == 8 || bits_per_sample == 16), | |
| 40 | + "PNGFilter created with invalid bits_per_sample not 1, 2, 4, 8, or 16"); | |
| 42 | 41 | bytes_per_pixel = ((bits_per_sample * samples_per_pixel) + 7) / 8; |
| 43 | 42 | unsigned long long bpr = ((columns * bits_per_sample * samples_per_pixel) + 7) / 8; |
| 44 | - if ((bpr == 0) || (bpr > (UINT_MAX - 1))) { | |
| 45 | - throw std::runtime_error("PNGFilter created with invalid columns value"); | |
| 46 | - } | |
| 47 | - if (memory_limit > 0 && bpr > (memory_limit / 2U)) { | |
| 48 | - throw std::runtime_error("PNGFilter memory limit exceeded"); | |
| 49 | - } | |
| 43 | + util::no_ci_rt_error_if( | |
| 44 | + bpr == 0 || bpr > (UINT_MAX - 1), "PNGFilter created with invalid columns value"); | |
| 45 | + util::no_ci_rt_error_if( | |
| 46 | + memory_limit > 0 && bpr > (memory_limit / 2U), "PNGFilter memory limit exceeded"); | |
| 50 | 47 | bytes_per_row = bpr & UINT_MAX; |
| 51 | 48 | buf1 = QUtil::make_shared_array<unsigned char>(bytes_per_row + 1); |
| 52 | 49 | buf2 = QUtil::make_shared_array<unsigned char>(bytes_per_row + 1); | ... | ... |
libqpdf/Pl_RunLength.cc
| 1 | 1 | #include <qpdf/Pl_RunLength.hh> |
| 2 | 2 | |
| 3 | 3 | #include <qpdf/QTC.hh> |
| 4 | -#include <qpdf/QUtil.hh> | |
| 4 | +#include <qpdf/Util.hh> | |
| 5 | + | |
| 6 | +using namespace qpdf; | |
| 5 | 7 | |
| 6 | 8 | namespace |
| 7 | 9 | { |
| ... | ... | @@ -29,9 +31,7 @@ Pl_RunLength::Pl_RunLength(char const* identifier, Pipeline* next, action_e acti |
| 29 | 31 | Pipeline(identifier, next), |
| 30 | 32 | m(std::make_unique<Members>(action)) |
| 31 | 33 | { |
| 32 | - if (!next) { | |
| 33 | - throw std::logic_error("Attempt to create Pl_RunLength with nullptr as next"); | |
| 34 | - } | |
| 34 | + util::assertion(next, "Attempt to create Pl_RunLength with nullptr as next"); | |
| 35 | 35 | } |
| 36 | 36 | |
| 37 | 37 | void |
| ... | ... | @@ -40,10 +40,7 @@ Pl_RunLength::setMemoryLimit(unsigned long long limit) |
| 40 | 40 | memory_limit = limit; |
| 41 | 41 | } |
| 42 | 42 | |
| 43 | -Pl_RunLength::~Pl_RunLength() // NOLINT (modernize-use-equals-default) | |
| 44 | -{ | |
| 45 | - // Must be explicit and not inline -- see QPDF_DLL_CLASS in README-maintainer | |
| 46 | -} | |
| 43 | +Pl_RunLength::~Pl_RunLength() = default; | |
| 47 | 44 | |
| 48 | 45 | void |
| 49 | 46 | Pl_RunLength::write(unsigned char const* data, size_t len) |
| ... | ... | @@ -59,12 +56,12 @@ void |
| 59 | 56 | Pl_RunLength::encode(unsigned char const* data, size_t len) |
| 60 | 57 | { |
| 61 | 58 | for (size_t i = 0; i < len; ++i) { |
| 62 | - if ((m->state == st_top) != (m->length <= 1)) { | |
| 63 | - throw std::logic_error("Pl_RunLength::encode: state/length inconsistency"); | |
| 64 | - } | |
| 59 | + util::assertion( | |
| 60 | + (m->state == st_top) == (m->length <= 1), | |
| 61 | + "Pl_RunLength::encode: state/length inconsistency"); | |
| 65 | 62 | unsigned char ch = data[i]; |
| 66 | - if ((m->length > 0) && ((m->state == st_copying) || (m->length < 128)) && | |
| 67 | - (ch == m->buf[m->length - 1])) { | |
| 63 | + if (m->length > 0 && (m->state == st_copying || m->length < 128) && | |
| 64 | + ch == m->buf[m->length - 1]) { | |
| 68 | 65 | QTC::TC("libtests", "Pl_RunLength: switch to run", (m->length == 128) ? 0 : 1); |
| 69 | 66 | if (m->state == st_copying) { |
| 70 | 67 | --m->length; |
| ... | ... | @@ -76,7 +73,7 @@ Pl_RunLength::encode(unsigned char const* data, size_t len) |
| 76 | 73 | m->buf[m->length] = ch; |
| 77 | 74 | ++m->length; |
| 78 | 75 | } else { |
| 79 | - if ((m->length == 128) || (m->state == st_run)) { | |
| 76 | + if (m->length == 128 || m->state == st_run) { | |
| 80 | 77 | flush_encode(); |
| 81 | 78 | } else if (m->length > 0) { |
| 82 | 79 | m->state = st_copying; |
| ... | ... | @@ -90,9 +87,8 @@ Pl_RunLength::encode(unsigned char const* data, size_t len) |
| 90 | 87 | void |
| 91 | 88 | Pl_RunLength::decode(unsigned char const* data, size_t len) |
| 92 | 89 | { |
| 93 | - if (memory_limit && (len + m->out.size()) > memory_limit) { | |
| 94 | - throw std::runtime_error("Pl_RunLength memory limit exceeded"); | |
| 95 | - } | |
| 90 | + util::no_ci_rt_error_if( | |
| 91 | + memory_limit && (len + m->out.size()) > memory_limit, "Pl_RunLength memory limit exceeded"); | |
| 96 | 92 | m->out.reserve(len); |
| 97 | 93 | for (size_t i = 0; i < len; ++i) { |
| 98 | 94 | unsigned char const& ch = data[i]; |
| ... | ... | @@ -142,9 +138,9 @@ Pl_RunLength::flush_encode() |
| 142 | 138 | QTC::TC("libtests", "Pl_RunLength flush empty buffer"); |
| 143 | 139 | } |
| 144 | 140 | if (m->state == st_run) { |
| 145 | - if ((m->length < 2) || (m->length > 128)) { | |
| 146 | - throw std::logic_error("Pl_RunLength: invalid length in flush_encode for run"); | |
| 147 | - } | |
| 141 | + util::assertion( | |
| 142 | + !(m->length < 2 || m->length > 128), | |
| 143 | + "Pl_RunLength: invalid length in flush_encode for run"); | |
| 148 | 144 | auto ch = static_cast<unsigned char>(257 - m->length); |
| 149 | 145 | next()->write(&ch, 1); |
| 150 | 146 | next()->write(&m->buf[0], 1); | ... | ... |
libqpdf/Pl_StdioFile.cc
| ... | ... | @@ -3,9 +3,13 @@ |
| 3 | 3 | #include <qpdf/Pl_StdioFile.hh> |
| 4 | 4 | |
| 5 | 5 | #include <qpdf/QUtil.hh> |
| 6 | +#include <qpdf/Util.hh> | |
| 7 | + | |
| 6 | 8 | #include <cerrno> |
| 7 | 9 | #include <stdexcept> |
| 8 | 10 | |
| 11 | +using namespace qpdf; | |
| 12 | + | |
| 9 | 13 | class Pl_StdioFile::Members |
| 10 | 14 | { |
| 11 | 15 | public: |
| ... | ... | @@ -46,7 +50,7 @@ Pl_StdioFile::write(unsigned char const* buf, size_t len) |
| 46 | 50 | void |
| 47 | 51 | Pl_StdioFile::finish() |
| 48 | 52 | { |
| 49 | - if ((fflush(m->file) == -1) && (errno == EBADF)) { | |
| 50 | - throw std::logic_error(this->identifier + ": Pl_StdioFile::finish: stream already closed"); | |
| 51 | - } | |
| 53 | + util::assertion( | |
| 54 | + !(fflush(m->file) == -1 && errno == EBADF), | |
| 55 | + identifier + ": Pl_StdioFile::finish: stream already closed"); | |
| 52 | 56 | } | ... | ... |
libqpdf/Pl_TIFFPredictor.cc
| ... | ... | @@ -3,10 +3,13 @@ |
| 3 | 3 | #include <qpdf/BitStream.hh> |
| 4 | 4 | #include <qpdf/BitWriter.hh> |
| 5 | 5 | #include <qpdf/QTC.hh> |
| 6 | +#include <qpdf/Util.hh> | |
| 6 | 7 | |
| 7 | 8 | #include <climits> |
| 8 | 9 | #include <stdexcept> |
| 9 | 10 | |
| 11 | +using namespace qpdf; | |
| 12 | + | |
| 10 | 13 | namespace |
| 11 | 14 | { |
| 12 | 15 | unsigned long long memory_limit{0}; |
| ... | ... | @@ -25,22 +28,17 @@ Pl_TIFFPredictor::Pl_TIFFPredictor( |
| 25 | 28 | samples_per_pixel(samples_per_pixel), |
| 26 | 29 | bits_per_sample(bits_per_sample) |
| 27 | 30 | { |
| 28 | - if (!next) { | |
| 29 | - throw std::logic_error("Attempt to create Pl_TIFFPredictor with nullptr as next"); | |
| 30 | - } | |
| 31 | - if (samples_per_pixel < 1) { | |
| 32 | - throw std::runtime_error("TIFFPredictor created with invalid samples_per_pixel"); | |
| 33 | - } | |
| 34 | - if ((bits_per_sample < 1) || (bits_per_sample > (8 * (sizeof(unsigned long long))))) { | |
| 35 | - throw std::runtime_error("TIFFPredictor created with invalid bits_per_sample"); | |
| 36 | - } | |
| 31 | + util::assertion(next, "Attempt to create Pl_TIFFPredictor with nullptr as next"); | |
| 32 | + util::no_ci_rt_error_if( | |
| 33 | + samples_per_pixel < 1, "TIFFPredictor created with invalid samples_per_pixel"); | |
| 34 | + util::no_ci_rt_error_if( | |
| 35 | + bits_per_sample < 1 || bits_per_sample > (8 * (sizeof(unsigned long long))), | |
| 36 | + "TIFFPredictor created with invalid bits_per_sample"); | |
| 37 | 37 | unsigned long long bpr = ((columns * bits_per_sample * samples_per_pixel) + 7) / 8; |
| 38 | - if ((bpr == 0) || (bpr > (UINT_MAX - 1))) { | |
| 39 | - throw std::runtime_error("TIFFPredictor created with invalid columns value"); | |
| 40 | - } | |
| 41 | - if (memory_limit > 0 && bpr > (memory_limit / 2U)) { | |
| 42 | - throw std::runtime_error("TIFFPredictor memory limit exceeded"); | |
| 43 | - } | |
| 38 | + util::no_ci_rt_error_if( | |
| 39 | + bpr == 0 || bpr > (UINT_MAX - 1), "TIFFPredictor created with invalid columns value"); | |
| 40 | + util::no_ci_rt_error_if( | |
| 41 | + memory_limit > 0 && bpr > (memory_limit / 2U), "TIFFPredictor memory limit exceeded"); | |
| 44 | 42 | this->bytes_per_row = bpr & UINT_MAX; |
| 45 | 43 | } |
| 46 | 44 | ... | ... |
libqpdf/qpdf/Pl_MD5.hh
| ... | ... | @@ -3,12 +3,11 @@ |
| 3 | 3 | |
| 4 | 4 | #include <qpdf/MD5.hh> |
| 5 | 5 | #include <qpdf/Pipeline.hh> |
| 6 | - | |
| 7 | -#include <stdexcept> | |
| 6 | +#include <qpdf/Util.hh> | |
| 8 | 7 | |
| 9 | 8 | // This pipeline sends its output to its successor unmodified. After calling finish, the MD5 |
| 10 | 9 | // checksum of the data that passed through the pipeline is available. |
| 11 | - | |
| 10 | +// | |
| 12 | 11 | // This pipeline is reusable; i.e., it is safe to call write() after calling finish(). The first |
| 13 | 12 | // call to write() after a call to finish() initializes a new MD5 object. |
| 14 | 13 | class Pl_MD5 final: public Pipeline |
| ... | ... | @@ -34,9 +33,7 @@ class Pl_MD5 final: public Pipeline |
| 34 | 33 | std::string |
| 35 | 34 | getHexDigest() |
| 36 | 35 | { |
| 37 | - if (!enabled) { | |
| 38 | - throw std::logic_error("digest requested for a disabled MD5 Pipeline"); | |
| 39 | - } | |
| 36 | + qpdf::util::assertion(enabled, "digest requested for a disabled MD5 Pipeline"); | |
| 40 | 37 | in_progress = false; |
| 41 | 38 | return md5.unparse(); |
| 42 | 39 | } | ... | ... |