From 5b233bba3f885cd7f6202a70677d7ae2970bdfeb Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 13 Jul 2016 14:28:45 -0400 Subject: MAINT-5011: Remove alarming ll_kdu_error() function whose body reads: // *FIX: This exception is bad, bad, bad. It gets thrown from a // destructor which can lead to immediate program termination! throw "ll_kdu_error() throwing an exception"; which would be bad indeed... if ll_kdu_error() were ever actually referenced by anything! --- indra/llkdu/llimagej2ckdu.cpp | 7 ------- 1 file changed, 7 deletions(-) (limited to 'indra/llkdu') diff --git a/indra/llkdu/llimagej2ckdu.cpp b/indra/llkdu/llimagej2ckdu.cpp index 282c859e9e..94c7dbdd23 100644 --- a/indra/llkdu/llimagej2ckdu.cpp +++ b/indra/llkdu/llimagej2ckdu.cpp @@ -128,13 +128,6 @@ private: S32 mRowGap; }; -void ll_kdu_error( void ) -{ - // *FIX: This exception is bad, bad, bad. It gets thrown from a - // destructor which can lead to immediate program termination! - throw "ll_kdu_error() throwing an exception"; -} - // Stuff for new kdu error handling class LLKDUMessageWarning : public kdu_message { -- cgit v1.2.3 From 21e8352de6c4d662ffdb44d31294ce9caca86517 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 13 Jul 2016 14:42:44 -0400 Subject: MAINT-5011: Introduce KduError instead of throw/catch const char*. KduError is derived from std::runtime_error, so the message string becomes its what() message. --- indra/llkdu/llimagej2ckdu.cpp | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) (limited to 'indra/llkdu') diff --git a/indra/llkdu/llimagej2ckdu.cpp b/indra/llkdu/llimagej2ckdu.cpp index 94c7dbdd23..50c2e03cd4 100644 --- a/indra/llkdu/llimagej2ckdu.cpp +++ b/indra/llkdu/llimagej2ckdu.cpp @@ -34,6 +34,13 @@ #include "kdu_block_coding.h" +#include + +struct KduError: public std::runtime_error +{ + KduError(const std::string& msg): std::runtime_error(msg) {} +}; + class kdc_flow_control { public: @@ -171,7 +178,7 @@ void LLKDUMessageError::flush(bool end_of_message) { if (end_of_message) { - throw "KDU throwing an exception"; + throw KduError("LLKDUMessageError::flush()"); } } @@ -415,9 +422,9 @@ BOOL LLImageJ2CKDU::initDecode(LLImageJ2C &base, LLImageRaw &raw_image, F32 deco mTPosp->x = 0; } } - catch (const char* msg) + catch (const KduError& msg) { - base.setLastError(ll_safe_string(msg)); + base.setLastError(msg.what()); return FALSE; } catch (...) @@ -505,9 +512,9 @@ BOOL LLImageJ2CKDU::decodeImpl(LLImageJ2C &base, LLImageRaw &raw_image, F32 deco return FALSE; } } - catch (const char* msg) + catch (const KduError& msg) { - base.setLastError(ll_safe_string(msg)); + base.setLastError(msg.what()); base.decodeFailed(); cleanupCodeStream(); return TRUE; // done @@ -698,9 +705,9 @@ BOOL LLImageJ2CKDU::encodeImpl(LLImageJ2C &base, const LLImageRaw &raw_image, co base.updateData(); // set width, height delete[] output_buffer; } - catch(const char* msg) + catch(const KduError& msg) { - base.setLastError(ll_safe_string(msg)); + base.setLastError(msg.what()); return FALSE; } catch( ... ) @@ -722,9 +729,9 @@ BOOL LLImageJ2CKDU::getMetadata(LLImageJ2C &base) setupCodeStream(base, FALSE, MODE_FAST); return TRUE; } - catch (const char* msg) + catch (const KduError& msg) { - base.setLastError(ll_safe_string(msg)); + base.setLastError(msg.what()); return FALSE; } catch (...) -- cgit v1.2.3 From d7c904632b9dac112228092952f993569477366c Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 14 Jul 2016 10:26:57 -0400 Subject: MAINT-5011: On advice from NickyD, say KDUError not KduError. Also place KDUError into anonymous namespace to emphasize that it's entirely local to this .cpp file. --- indra/llkdu/llimagej2ckdu.cpp | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) (limited to 'indra/llkdu') diff --git a/indra/llkdu/llimagej2ckdu.cpp b/indra/llkdu/llimagej2ckdu.cpp index 50c2e03cd4..2a5b24fbc1 100644 --- a/indra/llkdu/llimagej2ckdu.cpp +++ b/indra/llkdu/llimagej2ckdu.cpp @@ -36,10 +36,12 @@ #include -struct KduError: public std::runtime_error +namespace { +struct KDUError: public std::runtime_error { - KduError(const std::string& msg): std::runtime_error(msg) {} + KDUError(const std::string& msg): std::runtime_error(msg) {} }; +} // anonymous namespace class kdc_flow_control { @@ -178,7 +180,7 @@ void LLKDUMessageError::flush(bool end_of_message) { if (end_of_message) { - throw KduError("LLKDUMessageError::flush()"); + throw KDUError("LLKDUMessageError::flush()"); } } @@ -422,7 +424,7 @@ BOOL LLImageJ2CKDU::initDecode(LLImageJ2C &base, LLImageRaw &raw_image, F32 deco mTPosp->x = 0; } } - catch (const KduError& msg) + catch (const KDUError& msg) { base.setLastError(msg.what()); return FALSE; @@ -512,7 +514,7 @@ BOOL LLImageJ2CKDU::decodeImpl(LLImageJ2C &base, LLImageRaw &raw_image, F32 deco return FALSE; } } - catch (const KduError& msg) + catch (const KDUError& msg) { base.setLastError(msg.what()); base.decodeFailed(); @@ -705,7 +707,7 @@ BOOL LLImageJ2CKDU::encodeImpl(LLImageJ2C &base, const LLImageRaw &raw_image, co base.updateData(); // set width, height delete[] output_buffer; } - catch(const KduError& msg) + catch(const KDUError& msg) { base.setLastError(msg.what()); return FALSE; @@ -729,7 +731,7 @@ BOOL LLImageJ2CKDU::getMetadata(LLImageJ2C &base) setupCodeStream(base, FALSE, MODE_FAST); return TRUE; } - catch (const KduError& msg) + catch (const KDUError& msg) { base.setLastError(msg.what()); return FALSE; -- cgit v1.2.3 From 9c49a6c91dd9b5bbe811fcd91d8992ed6bac33e7 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 19 Jul 2016 16:25:25 -0400 Subject: MAINT-5011: Introduce LLException base class for viewer exceptions. This also introduces LLContinueError for exceptions which should interrupt some part of viewer processing (e.g. the current coroutine) but should attempt to let the viewer session proceed. Derive all existing viewer exception classes from LLException rather than from std::runtime_error or std::logic_error. Use BOOST_THROW_EXCEPTION() rather than plain 'throw' to enrich the thrown exception with source file, line number and containing function. --- indra/llkdu/llimagej2ckdu.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'indra/llkdu') diff --git a/indra/llkdu/llimagej2ckdu.cpp b/indra/llkdu/llimagej2ckdu.cpp index 2a5b24fbc1..fa58931407 100644 --- a/indra/llkdu/llimagej2ckdu.cpp +++ b/indra/llkdu/llimagej2ckdu.cpp @@ -34,12 +34,13 @@ #include "kdu_block_coding.h" -#include +#include "llexception.h" +#include namespace { -struct KDUError: public std::runtime_error +struct KDUError: public LLException { - KDUError(const std::string& msg): std::runtime_error(msg) {} + KDUError(const std::string& msg): LLException(msg) {} }; } // anonymous namespace @@ -180,7 +181,7 @@ void LLKDUMessageError::flush(bool end_of_message) { if (end_of_message) { - throw KDUError("LLKDUMessageError::flush()"); + BOOST_THROW_EXCEPTION(KDUError("LLKDUMessageError::flush()")); } } -- cgit v1.2.3 From 5e9d2f57c82a57307a48afea09aa539b9fa80abf Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 17 Aug 2016 11:36:24 -0400 Subject: MAINT-5011: Use LLTHROW() instead of plain BOOST_THROW_EXCEPTION(). A level of preprocessor indirection lets us later change the implementation if desired. --- indra/llkdu/llimagej2ckdu.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'indra/llkdu') diff --git a/indra/llkdu/llimagej2ckdu.cpp b/indra/llkdu/llimagej2ckdu.cpp index fa58931407..8dd0b6d458 100644 --- a/indra/llkdu/llimagej2ckdu.cpp +++ b/indra/llkdu/llimagej2ckdu.cpp @@ -35,7 +35,6 @@ #include "kdu_block_coding.h" #include "llexception.h" -#include namespace { struct KDUError: public LLException @@ -181,7 +180,7 @@ void LLKDUMessageError::flush(bool end_of_message) { if (end_of_message) { - BOOST_THROW_EXCEPTION(KDUError("LLKDUMessageError::flush()")); + LLTHROW(KDUError("LLKDUMessageError::flush()")); } } -- cgit v1.2.3 From db4f13e7bcfc7ef61f750152494f1e52ad5a9080 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 17 Aug 2016 11:48:33 -0400 Subject: MAINT-5011: Clean up indentation from merge of viewer-release. --- indra/llkdu/llimagej2ckdu.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'indra/llkdu') diff --git a/indra/llkdu/llimagej2ckdu.cpp b/indra/llkdu/llimagej2ckdu.cpp index fe79079db3..e6ed6b2202 100644 --- a/indra/llkdu/llimagej2ckdu.cpp +++ b/indra/llkdu/llimagej2ckdu.cpp @@ -176,7 +176,7 @@ struct LLKDUMessageError : public LLKDUMessage // exception if we want to recover from a KDU error. if (end_of_message) { - LLTHROW(KDUError("LLKDUMessageError::flush()")); + LLTHROW(KDUError("LLKDUMessageError::flush()")); } } }; -- cgit v1.2.3 From 993f54f6e91d78a9c2e1389ad878d6bd46e9be5b Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 17 Aug 2016 15:40:03 -0400 Subject: MAINT-5011: Try to enrich catch (...) logging throughout viewer. Turns out we have a surprising number of catch (...) clauses in the viewer code base. If all we currently do is LL_ERRS() << "unknown exception" << LL_ENDL; then call CRASH_ON_UNHANDLED_EXCEPTION() instead. If what we do is LL_WARNS() << "unknown exception" << LL_ENDL; then call LOG_UNHANDLED_EXCEPTION() instead. Since many places need LOG_UNHANDLED_EXCEPTION() and nobody catches LLContinueError yet, eliminate LLContinueError& parameter from LOG_UNHANDLED_EXCEPTION(). This permits us to use the same log message as CRASH_ON_UNHANDLED_EXCEPTION(), just with a different severity level. Where a catch (...) clause actually provides contextual information, or makes an error string, add boost::current_exception_diagnostic_information() to try to figure out actual exception class and message. --- indra/llkdu/llimagej2ckdu.cpp | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) (limited to 'indra/llkdu') diff --git a/indra/llkdu/llimagej2ckdu.cpp b/indra/llkdu/llimagej2ckdu.cpp index e6ed6b2202..341c47a268 100644 --- a/indra/llkdu/llimagej2ckdu.cpp +++ b/indra/llkdu/llimagej2ckdu.cpp @@ -35,6 +35,7 @@ #include "kdu_block_coding.h" #include "llexception.h" +#include namespace { struct KDUError: public LLException @@ -400,7 +401,8 @@ bool LLImageJ2CKDU::initDecode(LLImageJ2C &base, LLImageRaw &raw_image, F32 deco } catch (...) { - base.setLastError("Unknown J2C error"); + base.setLastError("Unknown J2C error: " + + boost::current_exception_diagnostic_information()); return false; } @@ -498,7 +500,8 @@ bool LLImageJ2CKDU::decodeImpl(LLImageJ2C &base, LLImageRaw &raw_image, F32 deco } catch (...) { - base.setLastError( "Unknown J2C error" ); + base.setLastError("Unknown J2C error: " + + boost::current_exception_diagnostic_information()); base.decodeFailed(); cleanupCodeStream(); return true; // done @@ -689,7 +692,8 @@ bool LLImageJ2CKDU::encodeImpl(LLImageJ2C &base, const LLImageRaw &raw_image, co } catch( ... ) { - base.setLastError( "Unknown J2C error" ); + base.setLastError("Unknown J2C error: " + + boost::current_exception_diagnostic_information()); return false; } @@ -713,7 +717,8 @@ bool LLImageJ2CKDU::getMetadata(LLImageJ2C &base) } catch (...) { - base.setLastError( "Unknown J2C error" ); + base.setLastError("Unknown J2C error: " + + boost::current_exception_diagnostic_information()); return false; } } -- cgit v1.2.3 From 83eb9600631fcb98275b8d3db736f692fd5e6e1c Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 17 Aug 2016 15:47:08 -0400 Subject: MAINT-5011: Derive image-load exceptions from LLContinueError. Failure to load an image shouldn't crash the whole viewer. --- indra/llkdu/llimagej2ckdu.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'indra/llkdu') diff --git a/indra/llkdu/llimagej2ckdu.cpp b/indra/llkdu/llimagej2ckdu.cpp index 341c47a268..4c2eac9eea 100644 --- a/indra/llkdu/llimagej2ckdu.cpp +++ b/indra/llkdu/llimagej2ckdu.cpp @@ -38,9 +38,10 @@ #include namespace { -struct KDUError: public LLException +// Failure to load an image shouldn't crash the whole viewer. +struct KDUError: public LLContinueError { - KDUError(const std::string& msg): LLException(msg) {} + KDUError(const std::string& msg): LLContinueError(msg) {} }; } // anonymous namespace -- cgit v1.2.3 From 0eac1f41f68bd2cd3b7e73db9eb96c690507b448 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 17 Aug 2016 16:42:10 -0400 Subject: MAINT-5011: Catch kdu_exception (aka int) in case it leaks out. KDU internally throws kdu_exception, which is a typedef for int. It's possible that such an exception might leak out. Our usual strategy for unknown exceptions is to catch (...) and let boost::current_exception_diagnostic_information() handle them. However, for int (or a class not derived from std::exception), that function will only shrug and report no information available. Besides, we want to format kdu_exception specially anyway. First, the KDU #defines are in hex, so we should report the value in hex. But on inspection, certain of those hex values are actually multibyte ASCII literals in disguise -- so also report the byte string value. --- indra/llkdu/llimagej2ckdu.cpp | 76 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 75 insertions(+), 1 deletion(-) (limited to 'indra/llkdu') diff --git a/indra/llkdu/llimagej2ckdu.cpp b/indra/llkdu/llimagej2ckdu.cpp index 4c2eac9eea..9e1dfc7213 100644 --- a/indra/llkdu/llimagej2ckdu.cpp +++ b/indra/llkdu/llimagej2ckdu.cpp @@ -1,4 +1,4 @@ - /** +/** * @file llimagej2ckdu.cpp * @brief This is an implementation of JPEG2000 encode/decode using Kakadu * @@ -36,6 +36,8 @@ #include "llexception.h" #include +#include +#include namespace { // Failure to load an image shouldn't crash the whole viewer. @@ -43,6 +45,40 @@ struct KDUError: public LLContinueError { KDUError(const std::string& msg): LLContinueError(msg) {} }; + +// KDU defines int error codes as hex values, so we should log them in hex +// so we can grep KDU headers for the hex. However those hex values +// generally "happen" to encode big-endian multibyte character sequences, +// e.g. KDU_ERROR_EXCEPTION is 0x6b647545: 'kduE' +// But beware because KDU_NULL_EXCEPTION is simply 0 -- which doesn't +// preclude somebody from throwing it. +std::string report_kdu_exception(kdu_exception mb) +{ + std::ostringstream out; + // always report mb in hex + out << "kdu_exception " << std::hex << mb; + + // Also display as many chars as are encoded in the kdu_exception + // value. Make a char array; reserve 1 extra byte for nul terminator. + char bytes[sizeof(kdu_exception) + 1]; + // Back up through 'bytes' + char *bptr = bytes + sizeof(bytes); + *(--bptr) = '\0'; + while (mb) + { + // store low-order byte of mb in next-left char + *(--bptr) = char(mb & 0xFF); + // then shift mb right by one byte + mb >>= 8; + } + // did that produce any characters? + if (*bptr) + { + out << " (" << bptr << ')'; + } + + return out.str(); +} } // anonymous namespace class kdc_flow_control { @@ -400,6 +436,15 @@ bool LLImageJ2CKDU::initDecode(LLImageJ2C &base, LLImageRaw &raw_image, F32 deco base.setLastError(msg.what()); return false; } + catch (kdu_exception kdu_value) + { + // KDU internally throws kdu_exception. It's possible that such an + // exception might leak out into our code. Catch kdu_exception + // specially because boost::current_exception_diagnostic_information() + // could do nothing with it. + base.setLastError(report_kdu_exception(kdu_value)); + return false; + } catch (...) { base.setLastError("Unknown J2C error: " + @@ -499,6 +544,17 @@ bool LLImageJ2CKDU::decodeImpl(LLImageJ2C &base, LLImageRaw &raw_image, F32 deco cleanupCodeStream(); return true; // done } + catch (kdu_exception kdu_value) + { + // KDU internally throws kdu_exception. It's possible that such an + // exception might leak out into our code. Catch kdu_exception + // specially because boost::current_exception_diagnostic_information() + // could do nothing with it. + base.setLastError(report_kdu_exception(kdu_value)); + base.decodeFailed(); + cleanupCodeStream(); + return true; // done + } catch (...) { base.setLastError("Unknown J2C error: " + @@ -691,6 +747,15 @@ bool LLImageJ2CKDU::encodeImpl(LLImageJ2C &base, const LLImageRaw &raw_image, co base.setLastError(msg.what()); return false; } + catch (kdu_exception kdu_value) + { + // KDU internally throws kdu_exception. It's possible that such an + // exception might leak out into our code. Catch kdu_exception + // specially because boost::current_exception_diagnostic_information() + // could do nothing with it. + base.setLastError(report_kdu_exception(kdu_value)); + return false; + } catch( ... ) { base.setLastError("Unknown J2C error: " + @@ -716,6 +781,15 @@ bool LLImageJ2CKDU::getMetadata(LLImageJ2C &base) base.setLastError(msg.what()); return false; } + catch (kdu_exception kdu_value) + { + // KDU internally throws kdu_exception. It's possible that such an + // exception might leak out into our code. Catch kdu_exception + // specially because boost::current_exception_diagnostic_information() + // could do nothing with it. + base.setLastError(report_kdu_exception(kdu_value)); + return false; + } catch (...) { base.setLastError("Unknown J2C error: " + -- cgit v1.2.3 From c7cb6636c4fae7187f321473f93cbeb6ab8522e0 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 7 Sep 2016 10:57:07 -0400 Subject: MAINT-6584, MAINT-5011: Change new 'throw' to LLTHROW() to be consistent with new exception conventions. --- indra/llkdu/llimagej2ckdu.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'indra/llkdu') diff --git a/indra/llkdu/llimagej2ckdu.cpp b/indra/llkdu/llimagej2ckdu.cpp index 0ed6ffefa9..aa405362e8 100644 --- a/indra/llkdu/llimagej2ckdu.cpp +++ b/indra/llkdu/llimagej2ckdu.cpp @@ -344,10 +344,10 @@ void LLImageJ2CKDU::setupCodeStream(LLImageJ2C &base, bool keep_codestream, ECod { // This method is only called from methods that catch KDUError. // We want to fail the image load, not crash the viewer. - throw KDUError(STRINGIZE("Component " << idx << " dimensions " + LLTHROW(KDUError(STRINGIZE("Component " << idx << " dimensions " << other_dims << " do not match component 0 dimensions " - << dims << "!")); + << dims << "!"))); } } -- cgit v1.2.3