From e1e62e610ff8d09e4a7e14caa4fd87fb6b2e2ba9 Mon Sep 17 00:00:00 2001 From: m-holger Date: Sun, 30 Nov 2025 09:01:19 +0000 Subject: [PATCH] Refactor error handling and simplify logic across multiple pipelines --- libqpdf/InputSource.cc | 12 +++++++----- libqpdf/OffsetInputSource.cc | 32 ++++++++++++++++---------------- libqpdf/Pl_PNGFilter.cc | 31 ++++++++++++++----------------- libqpdf/Pl_RunLength.cc | 36 ++++++++++++++++-------------------- libqpdf/Pl_StdioFile.cc | 10 +++++++--- libqpdf/Pl_TIFFPredictor.cc | 28 +++++++++++++--------------- libqpdf/qpdf/Pl_MD5.hh | 9 +++------ 7 files changed, 76 insertions(+), 82 deletions(-) diff --git a/libqpdf/InputSource.cc b/libqpdf/InputSource.cc index 1530ed4..08b6160 100644 --- a/libqpdf/InputSource.cc +++ b/libqpdf/InputSource.cc @@ -2,10 +2,13 @@ #include #include +#include + #include #include using namespace std::literals; +using namespace qpdf; void InputSource::setLastOffset(qpdf_offset_t offset) @@ -73,10 +76,10 @@ InputSource::findFirst(char const* start_chars, qpdf_offset_t offset, size_t len // To enable us to guarantee null-termination, save an extra byte so that buf[size] is valid // memory. size_t size = sizeof(buf) - 1; - if ((strlen(start_chars) < 1) || (strlen(start_chars) > size)) { - throw std::logic_error( - "InputSource::findSource called with too small or too large of a character sequence"); - } + util::assertion( + !(strlen(start_chars) < 1 || strlen(start_chars) > size), + "InputSource::findSource called with too small or too large of a character sequence" // + ); char* p = nullptr; qpdf_offset_t buf_offset = offset; @@ -155,7 +158,6 @@ InputSource::findFirst(char const* start_chars, qpdf_offset_t offset, size_t len p = buf + bytes_read; } } - throw std::logic_error("InputSource after while (true)"); } bool diff --git a/libqpdf/OffsetInputSource.cc b/libqpdf/OffsetInputSource.cc index d0028f0..7e22f2f 100644 --- a/libqpdf/OffsetInputSource.cc +++ b/libqpdf/OffsetInputSource.cc @@ -1,56 +1,56 @@ #include +#include + #include #include #include +using namespace qpdf; + OffsetInputSource::OffsetInputSource( std::shared_ptr proxied, qpdf_offset_t global_offset) : proxied(proxied), global_offset(global_offset) { - if (global_offset < 0) { - throw std::logic_error("OffsetInputSource constructed with negative offset"); - } - this->max_safe_offset = std::numeric_limits::max() - global_offset; + util::assertion(global_offset >= 0, "OffsetInputSource constructed with negative offset"); + max_safe_offset = std::numeric_limits::max() - global_offset; } qpdf_offset_t OffsetInputSource::findAndSkipNextEOL() { - return this->proxied->findAndSkipNextEOL() - this->global_offset; + return proxied->findAndSkipNextEOL() - global_offset; } std::string const& OffsetInputSource::getName() const { - return this->proxied->getName(); + return proxied->getName(); } qpdf_offset_t OffsetInputSource::tell() { - return this->proxied->tell() - this->global_offset; + return proxied->tell() - global_offset; } void OffsetInputSource::seek(qpdf_offset_t offset, int whence) { if (whence == SEEK_SET) { - if (offset > this->max_safe_offset) { + if (offset > max_safe_offset) { std::ostringstream msg; msg.imbue(std::locale::classic()); msg << "seeking to " << offset << " offset by " << global_offset << " would cause an overflow of the offset type"; throw std::range_error(msg.str()); } - this->proxied->seek(offset + global_offset, whence); + proxied->seek(offset + global_offset, whence); } else { - this->proxied->seek(offset, whence); - } - if (tell() < 0) { - throw std::runtime_error("offset input source: seek before beginning of file"); + proxied->seek(offset, whence); } + util::no_ci_rt_error_if(tell() < 0, "offset input source: seek before beginning of file"); } void @@ -62,13 +62,13 @@ OffsetInputSource::rewind() size_t OffsetInputSource::read(char* buffer, size_t length) { - size_t result = this->proxied->read(buffer, length); - this->setLastOffset(this->proxied->getLastOffset() - global_offset); + size_t result = proxied->read(buffer, length); + setLastOffset(proxied->getLastOffset() - global_offset); return result; } void OffsetInputSource::unreadCh(char ch) { - this->proxied->unreadCh(ch); + proxied->unreadCh(ch); } diff --git a/libqpdf/Pl_PNGFilter.cc b/libqpdf/Pl_PNGFilter.cc index e3a3528..fb82b1c 100644 --- a/libqpdf/Pl_PNGFilter.cc +++ b/libqpdf/Pl_PNGFilter.cc @@ -2,11 +2,14 @@ #include #include +#include #include #include #include +using namespace qpdf; + namespace { unsigned long long memory_limit{0}; @@ -28,25 +31,19 @@ Pl_PNGFilter::Pl_PNGFilter( Pipeline(identifier, next), action(action) { - if (!next) { - throw std::logic_error("Attempt to create Pl_PNGFilter with nullptr as next"); - } - if (samples_per_pixel < 1) { - throw std::runtime_error("PNGFilter created with invalid samples_per_pixel"); - } - if (!((bits_per_sample == 1) || (bits_per_sample == 2) || (bits_per_sample == 4) || - (bits_per_sample == 8) || (bits_per_sample == 16))) { - throw std::runtime_error( - "PNGFilter created with invalid bits_per_sample not 1, 2, 4, 8, or 16"); - } + util::assertion(next, "Attempt to create Pl_PNGFilter with nullptr as next"); + util::no_ci_rt_error_if( + samples_per_pixel < 1, "PNGFilter created with invalid samples_per_pixel"); + util::no_ci_rt_error_if( + !(bits_per_sample == 1 || bits_per_sample == 2 || bits_per_sample == 4 || + bits_per_sample == 8 || bits_per_sample == 16), + "PNGFilter created with invalid bits_per_sample not 1, 2, 4, 8, or 16"); bytes_per_pixel = ((bits_per_sample * samples_per_pixel) + 7) / 8; unsigned long long bpr = ((columns * bits_per_sample * samples_per_pixel) + 7) / 8; - if ((bpr == 0) || (bpr > (UINT_MAX - 1))) { - throw std::runtime_error("PNGFilter created with invalid columns value"); - } - if (memory_limit > 0 && bpr > (memory_limit / 2U)) { - throw std::runtime_error("PNGFilter memory limit exceeded"); - } + util::no_ci_rt_error_if( + bpr == 0 || bpr > (UINT_MAX - 1), "PNGFilter created with invalid columns value"); + util::no_ci_rt_error_if( + memory_limit > 0 && bpr > (memory_limit / 2U), "PNGFilter memory limit exceeded"); bytes_per_row = bpr & UINT_MAX; buf1 = QUtil::make_shared_array(bytes_per_row + 1); buf2 = QUtil::make_shared_array(bytes_per_row + 1); diff --git a/libqpdf/Pl_RunLength.cc b/libqpdf/Pl_RunLength.cc index cd269ab..c89c0c6 100644 --- a/libqpdf/Pl_RunLength.cc +++ b/libqpdf/Pl_RunLength.cc @@ -1,7 +1,9 @@ #include #include -#include +#include + +using namespace qpdf; namespace { @@ -29,9 +31,7 @@ Pl_RunLength::Pl_RunLength(char const* identifier, Pipeline* next, action_e acti Pipeline(identifier, next), m(std::make_unique(action)) { - if (!next) { - throw std::logic_error("Attempt to create Pl_RunLength with nullptr as next"); - } + util::assertion(next, "Attempt to create Pl_RunLength with nullptr as next"); } void @@ -40,10 +40,7 @@ Pl_RunLength::setMemoryLimit(unsigned long long limit) memory_limit = limit; } -Pl_RunLength::~Pl_RunLength() // NOLINT (modernize-use-equals-default) -{ - // Must be explicit and not inline -- see QPDF_DLL_CLASS in README-maintainer -} +Pl_RunLength::~Pl_RunLength() = default; void Pl_RunLength::write(unsigned char const* data, size_t len) @@ -59,12 +56,12 @@ void Pl_RunLength::encode(unsigned char const* data, size_t len) { for (size_t i = 0; i < len; ++i) { - if ((m->state == st_top) != (m->length <= 1)) { - throw std::logic_error("Pl_RunLength::encode: state/length inconsistency"); - } + util::assertion( + (m->state == st_top) == (m->length <= 1), + "Pl_RunLength::encode: state/length inconsistency"); unsigned char ch = data[i]; - if ((m->length > 0) && ((m->state == st_copying) || (m->length < 128)) && - (ch == m->buf[m->length - 1])) { + if (m->length > 0 && (m->state == st_copying || m->length < 128) && + ch == m->buf[m->length - 1]) { QTC::TC("libtests", "Pl_RunLength: switch to run", (m->length == 128) ? 0 : 1); if (m->state == st_copying) { --m->length; @@ -76,7 +73,7 @@ Pl_RunLength::encode(unsigned char const* data, size_t len) m->buf[m->length] = ch; ++m->length; } else { - if ((m->length == 128) || (m->state == st_run)) { + if (m->length == 128 || m->state == st_run) { flush_encode(); } else if (m->length > 0) { m->state = st_copying; @@ -90,9 +87,8 @@ Pl_RunLength::encode(unsigned char const* data, size_t len) void Pl_RunLength::decode(unsigned char const* data, size_t len) { - if (memory_limit && (len + m->out.size()) > memory_limit) { - throw std::runtime_error("Pl_RunLength memory limit exceeded"); - } + util::no_ci_rt_error_if( + memory_limit && (len + m->out.size()) > memory_limit, "Pl_RunLength memory limit exceeded"); m->out.reserve(len); for (size_t i = 0; i < len; ++i) { unsigned char const& ch = data[i]; @@ -142,9 +138,9 @@ Pl_RunLength::flush_encode() QTC::TC("libtests", "Pl_RunLength flush empty buffer"); } if (m->state == st_run) { - if ((m->length < 2) || (m->length > 128)) { - throw std::logic_error("Pl_RunLength: invalid length in flush_encode for run"); - } + util::assertion( + !(m->length < 2 || m->length > 128), + "Pl_RunLength: invalid length in flush_encode for run"); auto ch = static_cast(257 - m->length); next()->write(&ch, 1); next()->write(&m->buf[0], 1); diff --git a/libqpdf/Pl_StdioFile.cc b/libqpdf/Pl_StdioFile.cc index a2f500f..e2103ea 100644 --- a/libqpdf/Pl_StdioFile.cc +++ b/libqpdf/Pl_StdioFile.cc @@ -3,9 +3,13 @@ #include #include +#include + #include #include +using namespace qpdf; + class Pl_StdioFile::Members { public: @@ -46,7 +50,7 @@ Pl_StdioFile::write(unsigned char const* buf, size_t len) void Pl_StdioFile::finish() { - if ((fflush(m->file) == -1) && (errno == EBADF)) { - throw std::logic_error(this->identifier + ": Pl_StdioFile::finish: stream already closed"); - } + util::assertion( + !(fflush(m->file) == -1 && errno == EBADF), + identifier + ": Pl_StdioFile::finish: stream already closed"); } diff --git a/libqpdf/Pl_TIFFPredictor.cc b/libqpdf/Pl_TIFFPredictor.cc index ee46796..3df5115 100644 --- a/libqpdf/Pl_TIFFPredictor.cc +++ b/libqpdf/Pl_TIFFPredictor.cc @@ -3,10 +3,13 @@ #include #include #include +#include #include #include +using namespace qpdf; + namespace { unsigned long long memory_limit{0}; @@ -25,22 +28,17 @@ Pl_TIFFPredictor::Pl_TIFFPredictor( samples_per_pixel(samples_per_pixel), bits_per_sample(bits_per_sample) { - if (!next) { - throw std::logic_error("Attempt to create Pl_TIFFPredictor with nullptr as next"); - } - if (samples_per_pixel < 1) { - throw std::runtime_error("TIFFPredictor created with invalid samples_per_pixel"); - } - if ((bits_per_sample < 1) || (bits_per_sample > (8 * (sizeof(unsigned long long))))) { - throw std::runtime_error("TIFFPredictor created with invalid bits_per_sample"); - } + util::assertion(next, "Attempt to create Pl_TIFFPredictor with nullptr as next"); + util::no_ci_rt_error_if( + samples_per_pixel < 1, "TIFFPredictor created with invalid samples_per_pixel"); + util::no_ci_rt_error_if( + bits_per_sample < 1 || bits_per_sample > (8 * (sizeof(unsigned long long))), + "TIFFPredictor created with invalid bits_per_sample"); unsigned long long bpr = ((columns * bits_per_sample * samples_per_pixel) + 7) / 8; - if ((bpr == 0) || (bpr > (UINT_MAX - 1))) { - throw std::runtime_error("TIFFPredictor created with invalid columns value"); - } - if (memory_limit > 0 && bpr > (memory_limit / 2U)) { - throw std::runtime_error("TIFFPredictor memory limit exceeded"); - } + util::no_ci_rt_error_if( + bpr == 0 || bpr > (UINT_MAX - 1), "TIFFPredictor created with invalid columns value"); + util::no_ci_rt_error_if( + memory_limit > 0 && bpr > (memory_limit / 2U), "TIFFPredictor memory limit exceeded"); this->bytes_per_row = bpr & UINT_MAX; } diff --git a/libqpdf/qpdf/Pl_MD5.hh b/libqpdf/qpdf/Pl_MD5.hh index 37c9380..3a8a2d4 100644 --- a/libqpdf/qpdf/Pl_MD5.hh +++ b/libqpdf/qpdf/Pl_MD5.hh @@ -3,12 +3,11 @@ #include #include - -#include +#include // This pipeline sends its output to its successor unmodified. After calling finish, the MD5 // checksum of the data that passed through the pipeline is available. - +// // This pipeline is reusable; i.e., it is safe to call write() after calling finish(). The first // call to write() after a call to finish() initializes a new MD5 object. class Pl_MD5 final: public Pipeline @@ -34,9 +33,7 @@ class Pl_MD5 final: public Pipeline std::string getHexDigest() { - if (!enabled) { - throw std::logic_error("digest requested for a disabled MD5 Pipeline"); - } + qpdf::util::assertion(enabled, "digest requested for a disabled MD5 Pipeline"); in_progress = false; return md5.unparse(); } -- libgit2 0.21.4