retval refinement

alibaba · Oct 6, 2024 · 153747e · 153747e
1 parent ec1e8de
commit 153747e
Show file tree

Hide file tree

Showing 5 changed files with 62 additions and 57 deletions.
diff --git a/common/alog.h b/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>

diff --git a/common/retval.h b/common/retval.h
@@ -17,44 +17,51 @@ limitations under the License.
 #pragma once
 #include <inttypes.h>
 #include <assert.h>
+#include <errno.h>
+#include <type_traits>
 
 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;
+    int64_t _errno = 0;
     bool failed() const { return _errno; }
     bool succeeded() const { return !failed(); }
     int get_errno() const { assert(_errno > 0); 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 x < 0; }
+
+template<typename T> inline typename
+std::enable_if<!std::is_signed<T>::value, bool>::type
+is_failure(T x) { return 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 +77,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 +91,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)
-
diff --git a/common/test/test.cpp b/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:
@@ -921,12 +921,23 @@ retval<void> ret_succeeded() {
 }
 
 TEST(retval, basic) {
+    errno = EALREADY;
     const static retval<int> rvs[] =
-        {{32}, {EINVAL, -2345}, {EADDRINUSE, -1234}, {EALREADY, -5234}};
+        {{32}, {-2345, EINVAL}, {-1234, EADDRINUSE}, {-5234}};
     EXPECT_EQ(rvs[0], 32);
+    EXPECT_EQ(rvs[0].succeeded(), true);
+    EXPECT_EQ(rvs[0].error(), 0);
     EXPECT_EQ(rvs[1], -2345);
+    EXPECT_EQ(rvs[1].succeeded(), false);
+    EXPECT_EQ(rvs[1].error(), EINVAL);
     EXPECT_EQ(rvs[2], -1234);
+    EXPECT_EQ(rvs[2].succeeded(), false);
+    EXPECT_EQ(rvs[2].error(), EADDRINUSE);
     EXPECT_EQ(rvs[3], -5234);
+    EXPECT_EQ(rvs[3].succeeded(), false);
+    EXPECT_EQ(rvs[3].error(), EALREADY);
+
+    retval<float*> asdf = {nullptr, ECANCELED};
 
     for (auto i: xrange(LEN(rvs))) {
         static_assert(std::is_same<decltype(i), size_t>::value, "...");

diff --git a/ecosystem/simple_dom.cpp b/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());

diff --git a/thread/test/st_utest.cpp b/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));
 }
-