retval refinement

common/alog.h

@@ -513,19 +513,19 @@ struct ERRNO
 
 LogBuffer& operator << (LogBuffer& log, ERRNO e);
 
-inline LogBuffer& operator << (LogBuffer& log, const photon::retval_base& rvb) {
+inline LogBuffer& operator << (LogBuffer& log, const photon::reterr& rvb) {
     auto x = rvb._errno;
     return x ? (log << ERRNO((int)x)) : log;
 }
 
 template<typename T> inline
 LogBuffer& operator << (LogBuffer& log, const photon::retval<T>& v) {
-    return v.succeeded() ? (log << v.get()) : (log << v.base());
+    return v.succeeded() ? (log << v.get()) : (log << v.error());
 }
 
 template<> inline
 LogBuffer& operator << <void> (LogBuffer& log, const photon::retval<void>& v) {
-    return log << v.base();
+    return log << v.error();
 }
 
 template<typename T>
@@ -573,18 +573,15 @@ inline LogBuffer& operator<<(LogBuffer& log, const NamedValue<T>& v) {
     return retv;                                    \
 }
 
+// err can be either an error number of int, or an retval<T>
 #define LOG_ERROR_RETVAL(err, ...) do {             \
-    if (std::is_same<decltype(err), int>::value) {  \
-        retval_base e{err};                         \
-        assert(e.failed());                         \
-        LOG_ERROR(__VA_ARGS__, ' ', e);             \
-        return e;                                   \
-    } else {                                        \
-        LOG_ERROR(__VA_ARGS__);                     \
-        return err;                                 \
-    }                                               \
+    reterr e{err};                                  \
+    LOG_ERROR(__VA_ARGS__, ' ', e);                 \
+    return e;                                       \
 } while(0)
 
+#define LOG_ERRNO_RETVAL(...) LOG_ERROR_RETVAL(errno, __VA_ARGS__)
+
 // Acts like a LogBuilder
 // but able to do operations when log builds
 template <typename Builder, typename Append>

common/retval.h

@@ -17,44 +17,52 @@ limitations under the License.
 #pragma once
 #include <inttypes.h>
 #include <assert.h>
+#include <errno.h>
+#include <type_traits>
+#include <photon/common/utility.h>
 
 namespace photon {
 
-struct retval_base {
-    // use uint64_t to make sure the result is returned
+struct reterr {
+    // use int64_t to make sure the result is returned
     // via another register, so that it is accessed easily
-    uint64_t _errno = 0;
-    bool failed() const { return _errno; }
+    int64_t _errno = 0;
+    bool failed() const { return unlikely(_errno); }
     bool succeeded() const { return !failed(); }
-    int get_errno() const { assert(_errno > 0); return (int)_errno; }
+    int get_errno() const { return (int)_errno; }
+    operator int() const { return get_errno(); }
 };
 
-template<typename T> inline
-T failure_value() { return 0; }
+template<typename T> inline typename
+std::enable_if<std::is_signed<T>::value, T>::type
+failure_value() { return -1; }
+
+template<typename T> inline typename
+std::enable_if<!std::is_signed<T>::value, T>::type
+failure_value() { return 0; }
+
+template<typename T> inline typename
+std::enable_if<std::is_signed<T>::value, bool>::type
+is_failure(T x) { return unlikely(x < 0); }
+
+template<typename T> inline typename
+std::enable_if<!std::is_signed<T>::value, bool>::type
+is_failure(T x) { return unlikely(x == 0); }
 
 template<typename T>
-struct retval : public retval_base {
+struct retval : public reterr {
     T _val;
-    retval(T x) : _val(x) { }
-    retval(int _errno, T val) : retval_base{(uint64_t)_errno}, _val(val) {
-        assert(failed());
-    }
-    retval(const retval_base& rvb) : retval_base(rvb) {
-        assert(failed());
+    retval(const retval&) = default;
+    retval(T val) : reterr{is_failure(val) ? errno : 0}, _val(val) { }
+    retval(T val, int _errno) : reterr{_errno}, _val(val) { }
+    retval(const reterr& rvb) : reterr(rvb) {  // for failure
         _val = failure_value<T>();
+        assert(failed());
     }
-    operator T() const {
-        return get();
-    }
-    T operator->() {
-        return get();
-    }
-    T get() const {
-        return _val;
-    }
-    retval_base base() const {
-        return *this;
-    }
+    operator T() const { return get(); }
+    T operator->() { return get(); }
+    T get() const { return _val; }
+    reterr error() const { return (const reterr) *this; }
     bool operator==(const retval& rhs) const {
         return _errno ? (_errno == rhs._errno) : (_val == rhs._val);
     }
@@ -70,13 +78,11 @@ struct retval : public retval_base {
 };
 
 template<>
-struct retval<void> : public retval_base {
-    retval(int errno_ = 0) : retval_base{(uint64_t)errno_} { }
-    retval(const retval_base& rvb) : retval_base(rvb) { }
+struct retval<void> : public reterr {
+    retval(int errno_ = 0) : reterr{errno_} { }
+    retval(const reterr& rvb) : reterr(rvb) { }
     void get() const { }
-    retval_base base() const {
-        return *this;
-    }
+    reterr error() const { return (const reterr) *this; }
     bool operator==(const retval& rhs) const {
         return _errno == rhs._errno;
     }
@@ -86,12 +92,3 @@ struct retval<void> : public retval_base {
 };
 
 }
-
-#define DEFINE_FAILURE_VALUE(type, val) namespace photon {  \
-    template<> inline type failure_value<type>() { return val; } }
-
-DEFINE_FAILURE_VALUE(int8_t,  -1)
-DEFINE_FAILURE_VALUE(int16_t, -1)
-DEFINE_FAILURE_VALUE(int32_t, -1)
-DEFINE_FAILURE_VALUE(int64_t, -1)
-

common/test/test.cpp

@@ -893,15 +893,15 @@ TEST(generator, example)
 }
 
 retval<double> bar() {
-    return {EALREADY, 0};   // return a failure
+    return {-1, EALREADY};   // return a failure
 }
 
 photon::retval<int> foo(int i) {
     switch (i) {
     default:
         return 32;
     case 1:
-        return retval_base{EINVAL};
+        return reterr{EINVAL};
     case 2:
         LOG_ERROR_RETVAL(EADDRINUSE, "trying to use LOG_ERROR_RETVAL() with an error number constant");
     case 3:
@@ -920,13 +920,24 @@ retval<void> ret_succeeded() {
     return {/* 0 */};
 }
 
+template<typename T>
+void check(T rv, decltype(T::_val) v, bool succeeded, int error) {
+    EXPECT_EQ(rv, v);
+    EXPECT_EQ(rv.succeeded(), succeeded);
+    EXPECT_EQ(rv.error(), error);
+}
+
 TEST(retval, basic) {
+    errno = EALREADY;
     const static retval<int> rvs[] =
-        {{32}, {EINVAL, -2345}, {EADDRINUSE, -1234}, {EALREADY, -5234}};
-    EXPECT_EQ(rvs[0], 32);
-    EXPECT_EQ(rvs[1], -2345);
-    EXPECT_EQ(rvs[2], -1234);
-    EXPECT_EQ(rvs[3], -5234);
+        {{32}, {-2345, EINVAL}, {-1234, EADDRINUSE}, {-5234}};
+    check(rvs[0],  32,   true,  0);
+    check(rvs[1], -2345, false, EINVAL);
+    check(rvs[2], -1234, false, EADDRINUSE);
+    check(rvs[3], -5234, false, EALREADY);
+
+    retval<float*> asdf = {nullptr, ECANCELED};
+    check(asdf, nullptr, false, ECANCELED);
 
     for (auto i: xrange(LEN(rvs))) {
         static_assert(std::is_same<decltype(i), size_t>::value, "...");

ecosystem/simple_dom.cpp

@@ -233,7 +233,7 @@ class XMLNode : public DocNode<XMLNode> {
     unique_ptr<XMLNode> __attributes__{nullptr};
     retval<XMLNode*> emplace_back(vector<XMLNode>& nodes, xml_base<char>* x) {
         if (x->name_size() == 0)
-            return {ECANCELED, 0};
+            return {nullptr, ECANCELED};
         str k{x->name(),  x->name_size()};
         str v{x->value(), x->value_size()};
         nodes.emplace_back(k, v, get_root());

thread/test/st_utest.cpp

@@ -26,7 +26,9 @@ GTEST_API_ int main(int argc, char **argv) {
 #endif
 
     // Initialize state-threads, create idle coroutine.
-    assert(st_init() == 0);
+    int ret = st_init();
+    assert(ret == 0);
+    if (ret < 0) return ret;
 
     testing::InitGoogleTest(&argc, argv);
     return RUN_ALL_TESTS();
@@ -40,4 +42,3 @@ VOID TEST(SampleTest, ExampleIntSizeTest)
     EXPECT_EQ(4, (int)sizeof(int32_t));
     EXPECT_EQ(8, (int)sizeof(int64_t));
 }
-