From e9b7640f842504794d8819440e18ad3358abdbea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pedro=20J=2E=20Est=C3=A9banez?= Date: Fri, 26 May 2017 21:07:39 +0200 Subject: [PATCH] Make error handling more convenient By adding some PRAY_* macros that encapsulate both the check and the returning of a null reference (UB). Plus transient avoidance of the flood of warnings emitted by Clang when checking 'this' for NULL. Plus explanation about the do-while(0) loop in some error macros. --- core/dvector.h | 6 +----- core/error_macros.h | 44 ++++++++++++++++++++++++++++++++++++++++++-- core/hash_map.h | 3 +-- core/list.h | 14 ++++---------- core/map.h | 6 +++--- core/object.h | 10 ++++++++++ core/vector.h | 11 +++-------- core/vmap.h | 6 ++---- 8 files changed, 66 insertions(+), 34 deletions(-) diff --git a/core/dvector.h b/core/dvector.h index ef7ec6b4c50..48888520268 100644 --- a/core/dvector.h +++ b/core/dvector.h @@ -311,13 +311,9 @@ void DVector::push_back(const T &p_val) { template const T DVector::operator[](int p_index) const { - if (p_index < 0 || p_index >= size()) { - T &aux = *((T *)0); //nullreturn - ERR_FAIL_COND_V(p_index < 0 || p_index >= size(), aux); - } + PRAY_BAD_INDEX(p_index, size(), T); Read r = read(); - return r[p_index]; } diff --git a/core/error_macros.h b/core/error_macros.h index 42a0ce7e74c..6456a8d7a1c 100644 --- a/core/error_macros.h +++ b/core/error_macros.h @@ -114,6 +114,8 @@ extern bool _err_error_exists; #define FUNCTION_STR __FUNCTION__ #endif +// (*): See https://stackoverflow.com/questions/257418/do-while-0-what-is-it-good-for + #define ERR_FAIL_INDEX(m_index, m_size) \ do { \ if ((m_index) < 0 || (m_index) >= (m_size)) { \ @@ -121,7 +123,7 @@ extern bool _err_error_exists; return; \ } else \ _err_error_exists = false; \ - } while (0); + } while (0); // (*) /** An index has failed if m_index<0 or m_index >=m_size, the function exists. * This function returns an error value, if returning Error, please select the most @@ -135,7 +137,20 @@ extern bool _err_error_exists; return m_retval; \ } else \ _err_error_exists = false; \ - } while (0); + } while (0); // (*) + +/** Use this one if there is no sensible fallback, that is, the error is unrecoverable. + * We'll do UB by returning a null reference and pray that we wont't crash. + */ +#define PRAY_BAD_INDEX(m_index, m_size, m_type) \ + do { \ + if ((m_index) < 0 || (m_index) >= (m_size)) { \ + _err_print_error(FUNCTION_STR, __FILE__, __LINE__, "SEVERE: Index " _STR(m_index) " out of size (" _STR(m_size) ")."); \ + m_type *n = (m_type *)0; /* two-step to avoid warning */ \ + return *n; \ + } else \ + _err_error_exists = false; \ + } while (0); // (*) /** An error condition happened (m_cond tested true) (WARNING this is the opposite as assert(). * the function will exit. @@ -187,6 +202,20 @@ extern bool _err_error_exists; _err_error_exists = false; \ } +/** Use this one if there is no sensible fallback, that is, the error is unrecoverable. + * We'll do UB by returning a null reference and pray that we wont't crash. + */ + +#define PRAY_COND(m_cond, m_type) \ + { \ + if (m_cond) { \ + _err_print_error(FUNCTION_STR, __FILE__, __LINE__, "SEVERE: Condition ' " _STR(m_cond) " ' is true."); \ + m_type *n = (m_type *)0; /* two-step to avoid warning */ \ + return *n; \ + } else \ + _err_error_exists = false; \ + } + /** An error condition happened (m_cond tested true) (WARNING this is the opposite as assert(). * the loop will skip to the next iteration. */ @@ -233,6 +262,17 @@ extern bool _err_error_exists; return m_value; \ } +/** Use this one if there is no sensible fallback, that is, the error is unrecoverable. + * We'll do UB by returning a null reference and pray that we wont't crash. + */ + +#define PRAY(m_type) \ + { \ + _err_print_error(FUNCTION_STR, __FILE__, __LINE__, "SEVERE: Method/Function Failed."); \ + m_type *n = (m_type *)0; /* two-step to avoid warning */ \ + return *n; \ + } + /** Print an error string. */ diff --git a/core/hash_map.h b/core/hash_map.h index 1bbe6c1877f..93006ddb095 100644 --- a/core/hash_map.h +++ b/core/hash_map.h @@ -465,8 +465,7 @@ public: if (!e) { e = create_entry(p_key); - if (!e) - return *(TData *)NULL; /* panic! */ + PRAY_COND(!e, TData); check_hash_table(); // perform mantenience routine } diff --git a/core/list.h b/core/list.h index 2fc8db1b317..1e3c0958895 100644 --- a/core/list.h +++ b/core/list.h @@ -398,10 +398,7 @@ public: T &operator[](int p_index) { - if (p_index < 0 || p_index >= size()) { - T &aux = *((T *)0); //nullreturn - ERR_FAIL_COND_V(p_index < 0 || p_index >= size(), aux); - } + PRAY_BAD_INDEX(p_index, size(), T); Element *I = front(); int c = 0; @@ -415,15 +412,12 @@ public: c++; } - ERR_FAIL_V(*((T *)0)); // bug!! + PRAY(T); // bug!! } const T &operator[](int p_index) const { - if (p_index < 0 || p_index >= size()) { - T &aux = *((T *)0); //nullreturn - ERR_FAIL_COND_V(p_index < 0 || p_index >= size(), aux); - } + PRAY_BAD_INDEX(p_index, size(), T); const Element *I = front(); int c = 0; @@ -437,7 +431,7 @@ public: c++; } - ERR_FAIL_V(*((T *)0)); // bug! + PRAY(T); // bug!! } void move_to_back(Element *p_I) { diff --git a/core/map.h b/core/map.h index 1b3b26df98c..4a47712de6c 100644 --- a/core/map.h +++ b/core/map.h @@ -599,9 +599,9 @@ public: const V &operator[](const K &p_key) const { - ERR_FAIL_COND_V(!_data._root, *(V *)NULL); // crash on purpose + PRAY_COND(!_data._root, V); const Element *e = find(p_key); - ERR_FAIL_COND_V(!e, *(V *)NULL); // crash on purpose + PRAY_COND(!e, V); return e->_value; } V &operator[](const K &p_key) { @@ -613,7 +613,7 @@ public: if (!e) e = insert(p_key, V()); - ERR_FAIL_COND_V(!e, *(V *)NULL); // crash on purpose + PRAY_COND(!e, V); return e->_value; } diff --git a/core/object.h b/core/object.h index 9b73f8b72f8..fcfc9f02d07 100644 --- a/core/object.h +++ b/core/object.h @@ -509,6 +509,12 @@ public: void add_change_receptor(Object *p_receptor); void remove_change_receptor(Object *p_receptor); +// TODO: ensure 'this' is never NULL since it's UB, but by now, avoid warning flood +#ifdef __clang__ +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wundefined-bool-conversion" +#endif + template T *cast_to() { @@ -539,6 +545,10 @@ public: #endif } +#ifdef __clang__ +#pragma clang diagnostic pop +#endif + enum { NOTIFICATION_POSTINITIALIZE = 0, diff --git a/core/vector.h b/core/vector.h index a73863b2e39..7de487f3df8 100644 --- a/core/vector.h +++ b/core/vector.h @@ -133,10 +133,7 @@ public: inline T &operator[](int p_index) { - if (p_index < 0 || p_index >= size()) { - T &aux = *((T *)0); //nullreturn - ERR_FAIL_COND_V(p_index < 0 || p_index >= size(), aux); - } + PRAY_BAD_INDEX(p_index, size(), T); _copy_on_write(); // wants to write, so copy on write. @@ -145,10 +142,8 @@ public: inline const T &operator[](int p_index) const { - if (p_index < 0 || p_index >= size()) { - const T &aux = *((T *)0); //nullreturn - ERR_FAIL_COND_V(p_index < 0 || p_index >= size(), aux); - } + PRAY_BAD_INDEX(p_index, size(), T); + // no cow needed, since it's reading return _get_data()[p_index]; } diff --git a/core/vmap.h b/core/vmap.h index ad079733080..19ff88237b5 100644 --- a/core/vmap.h +++ b/core/vmap.h @@ -180,10 +180,8 @@ public: inline const V &operator[](const T &p_key) const { int pos = _find_exact(p_key); - if (pos < 0) { - const T &aux = *((T *)0); //nullreturn - ERR_FAIL_COND_V(pos < 1, aux); - } + + PRAY_COND(pos < 0, V); return _data[pos].value; }