From a2b76a4a5f751b2753d37fc6843864306fb2f7f2 Mon Sep 17 00:00:00 2001 From: Francisco Paisana Date: Tue, 17 Mar 2020 17:26:50 +0000 Subject: [PATCH] use references rather than pointers --- lib/include/srslte/common/block_queue.h | 12 +++++----- lib/include/srslte/common/expected.h | 26 +++++++++++---------- lib/include/srslte/upper/rlc_tx_queue.h | 5 ++++- lib/src/upper/rlc_am_lte.cc | 14 ++++++------ lib/src/upper/rlc_tm.cc | 12 +++++----- lib/src/upper/rlc_um_base.cc | 14 ++++++------ lib/test/common/expected_test.cc | 30 ++++++++++++------------- 7 files changed, 60 insertions(+), 53 deletions(-) diff --git a/lib/include/srslte/common/block_queue.h b/lib/include/srslte/common/block_queue.h index c5cd4faa1..ae91beb13 100644 --- a/lib/include/srslte/common/block_queue.h +++ b/lib/include/srslte/common/block_queue.h @@ -29,6 +29,7 @@ #ifndef SRSLTE_BLOCK_QUEUE_H #define SRSLTE_BLOCK_QUEUE_H +#include "srslte/common/expected.h" #include #include #include @@ -93,7 +94,7 @@ public: bool try_push(const myobj& value) { return push_(value, false); } - std::pair try_push(myobj&& value) { return push_(std::move(value), false); } + srslte::expected try_push(myobj&& value) { return push_(std::move(value), false); } bool try_pop(myobj* value) { return pop_(value, false); } @@ -159,7 +160,6 @@ private: bool check_queue_space_unlocked(bool block) { num_threads++; - bool ret = false; if (capacity > 0) { if (block) { while (q.size() >= (uint32_t)capacity && enable) { @@ -178,10 +178,10 @@ private: return true; } - std::pair push_(myobj&& value, bool block) + srslte::expected push_(myobj&& value, bool block) { if (!enable) { - return std::make_pair(false, std::move(value)); + return std::move(value); } pthread_mutex_lock(&mutex); bool ret = check_queue_space_unlocked(block); @@ -190,10 +190,12 @@ private: mutexed_callback->pushing(value); } q.push(std::move(value)); + pthread_mutex_unlock(&mutex); pthread_cond_signal(&cv_empty); + return true; } pthread_mutex_unlock(&mutex); - return std::make_pair(ret, std::move(value)); + return std::move(value); } bool push_(const myobj& value, bool block) diff --git a/lib/include/srslte/common/expected.h b/lib/include/srslte/common/expected.h index 09c8ae0e0..99033d448 100644 --- a/lib/include/srslte/common/expected.h +++ b/lib/include/srslte/common/expected.h @@ -26,8 +26,6 @@ namespace srslte { -enum class error : uint8_t { success, error }; - struct default_error_t { }; @@ -38,17 +36,17 @@ public: expected() : has_val(true), val(T{}) {} expected(T&& t) : has_val(true), val(std::forward(t)) {} expected(E&& e) : has_val(false), unexpected(std::forward(e)) {} - expected(const expected& other) : has_val(other.has_val) + expected(const expected& other) { - if (has_val) { + if (other.has_val) { construct_val(other.val); } else { construct_error(other.unexpected); } } - expected(expected&& other) noexcept : has_val(other.has_val) + expected(expected&& other) noexcept { - if (has_val) { + if (other.has_val) { construct_val(std::move(other.val)); } else { construct_error(std::move(other.unexpected)); @@ -105,12 +103,16 @@ public: unexpected = std::forward(other); } } - operator bool() const { return has_value(); } - bool has_value() const { return has_val; } - T* value() { return has_val ? &val : nullptr; } - const T* value() const { return has_val ? &val : nullptr; } - E* error() { return has_val ? nullptr : &unexpected; } - const E* error() const { return has_val ? nullptr : &unexpected; } + operator bool() const { return has_value(); } + bool has_value() const { return has_val; } + const T& value() const& { return val; } + T& value() & { return val; } + T&& value() && { return std::move(val); } + const T&& value() const&& { return std::move(val); } + const E& error() const& { return unexpected; } + E& error() & { return unexpected; } + E&& error() && { return std::move(unexpected); } + const E&& error() const&& { return std::move(unexpected); } void swap(expected& other) noexcept { diff --git a/lib/include/srslte/upper/rlc_tx_queue.h b/lib/include/srslte/upper/rlc_tx_queue.h index 8ada1b080..e6d9875f4 100644 --- a/lib/include/srslte/upper/rlc_tx_queue.h +++ b/lib/include/srslte/upper/rlc_tx_queue.h @@ -56,7 +56,10 @@ public: } void write(unique_byte_buffer_t msg) { queue.push(std::move(msg)); } - std::pair try_write(unique_byte_buffer_t&& msg) { return queue.try_push(std::move(msg)); } + srslte::expected try_write(unique_byte_buffer_t&& msg) + { + return queue.try_push(std::move(msg)); + } unique_byte_buffer_t read() { return queue.wait_pop(); } diff --git a/lib/src/upper/rlc_am_lte.cc b/lib/src/upper/rlc_am_lte.cc index 86664c6b5..3bd44ff56 100644 --- a/lib/src/upper/rlc_am_lte.cc +++ b/lib/src/upper/rlc_am_lte.cc @@ -354,19 +354,19 @@ void rlc_am_lte::rlc_am_lte_tx::write_sdu(unique_byte_buffer_t sdu, bool blockin tx_sdu_queue.write(std::move(sdu)); } else { // non-blocking write - uint8_t* msg_ptr = sdu->msg; - uint32_t nof_bytes = sdu->N_bytes; - std::pair ret = tx_sdu_queue.try_write(std::move(sdu)); - if (ret.first) { + uint8_t* msg_ptr = sdu->msg; + uint32_t nof_bytes = sdu->N_bytes; + srslte::expected ret = tx_sdu_queue.try_write(std::move(sdu)); + if (ret) { log->info_hex( msg_ptr, nof_bytes, "%s Tx SDU (%d B, tx_sdu_queue_len=%d)\n", RB_NAME, nof_bytes, tx_sdu_queue.size()); } else { // in case of fail, the try_write returns back the sdu - log->info_hex(ret.second->msg, - ret.second->N_bytes, + log->info_hex(ret.error()->msg, + ret.error()->N_bytes, "[Dropped SDU] %s Tx SDU (%d B, tx_sdu_queue_len=%d)\n", RB_NAME, - ret.second->N_bytes, + ret.error()->N_bytes, tx_sdu_queue.size()); } } diff --git a/lib/src/upper/rlc_tm.cc b/lib/src/upper/rlc_tm.cc index d50ffec5a..e67ebe2e7 100644 --- a/lib/src/upper/rlc_tm.cc +++ b/lib/src/upper/rlc_tm.cc @@ -97,10 +97,10 @@ void rlc_tm::write_sdu(unique_byte_buffer_t sdu, bool blocking) ul_queue.size_bytes()); ul_queue.write(std::move(sdu)); } else { - uint8_t* msg_ptr = sdu->msg; - uint32_t nof_bytes = sdu->N_bytes; - std::pair ret = ul_queue.try_write(std::move(sdu)); - if (ret.first) { + uint8_t* msg_ptr = sdu->msg; + uint32_t nof_bytes = sdu->N_bytes; + srslte::expected ret = ul_queue.try_write(std::move(sdu)); + if (ret) { log->info_hex(msg_ptr, nof_bytes, "%s Tx SDU, queue size=%d, bytes=%d", @@ -108,8 +108,8 @@ void rlc_tm::write_sdu(unique_byte_buffer_t sdu, bool blocking) ul_queue.size(), ul_queue.size_bytes()); } else { - log->info_hex(ret.second->msg, - ret.second->N_bytes, + log->info_hex(ret.error()->msg, + ret.error()->N_bytes, "[Dropped SDU] %s Tx SDU, queue size=%d, bytes=%d", rrc->get_rb_name(lcid).c_str(), ul_queue.size(), diff --git a/lib/src/upper/rlc_um_base.cc b/lib/src/upper/rlc_um_base.cc index 6cb2b3bb3..22fcefe93 100644 --- a/lib/src/upper/rlc_um_base.cc +++ b/lib/src/upper/rlc_um_base.cc @@ -254,18 +254,18 @@ void rlc_um_base::rlc_um_base_tx::write_sdu(unique_byte_buffer_t sdu) void rlc_um_base::rlc_um_base_tx::try_write_sdu(unique_byte_buffer_t sdu) { if (sdu) { - uint8_t* msg_ptr = sdu->msg; - uint32_t nof_bytes = sdu->N_bytes; - std::pair ret = tx_sdu_queue.try_write(std::move(sdu)); - if (ret.first) { + uint8_t* msg_ptr = sdu->msg; + uint32_t nof_bytes = sdu->N_bytes; + srslte::expected ret = tx_sdu_queue.try_write(std::move(sdu)); + if (ret) { log->info_hex( msg_ptr, nof_bytes, "%s Tx SDU (%d B, tx_sdu_queue_len=%d)", rb_name.c_str(), nof_bytes, tx_sdu_queue.size()); } else { - log->info_hex(ret.second->msg, - ret.second->N_bytes, + log->info_hex(ret.error()->msg, + ret.error()->N_bytes, "[Dropped SDU] %s Tx SDU (%d B, tx_sdu_queue_len=%d)", rb_name.c_str(), - ret.second->N_bytes, + ret.error()->N_bytes, tx_sdu_queue.size()); } } else { diff --git a/lib/test/common/expected_test.cc b/lib/test/common/expected_test.cc index 4747cba93..90c7ada8f 100644 --- a/lib/test/common/expected_test.cc +++ b/lib/test/common/expected_test.cc @@ -30,7 +30,7 @@ int test_expected_trivial() exp = 5; TESTASSERT(exp.has_value()); - TESTASSERT(*exp.value() == 5); + TESTASSERT(exp.value() == 5); TESTASSERT(exp); exp.set_error(srslte::default_error_t{}); @@ -40,7 +40,7 @@ int test_expected_trivial() int i = 2; exp = i; TESTASSERT(exp); - TESTASSERT(*exp.value() == 2); + TESTASSERT(exp.value() == 2); exp.set_error(); TESTASSERT(not exp); @@ -48,12 +48,12 @@ int test_expected_trivial() exp = 3; { srslte::expected exp2 = exp; - TESTASSERT(exp2 and *exp2.value() == 3); + TESTASSERT(exp2 and exp2.value() == 3); srslte::expected exp3; exp3 = exp2; - TESTASSERT(exp3 and *exp3.value() == 3); + TESTASSERT(exp3 and exp3.value() == 3); } - TESTASSERT(exp and *exp.value() == 3); + TESTASSERT(exp and exp.value() == 3); exp.set_error(); { @@ -102,25 +102,25 @@ int test_expected_struct() { srslte::expected exp; exp = C{5}; - TESTASSERT(exp and exp.value()->val == 5); + TESTASSERT(exp and exp.value().val == 5); TESTASSERT(C::count == 1); { auto exp2 = exp; - TESTASSERT(exp2 and exp2.value()->val == 5); + TESTASSERT(exp2 and exp2.value().val == 5); TESTASSERT(C::count == 2); } - TESTASSERT(exp and exp.value()->val == 5); + TESTASSERT(exp and exp.value().val == 5); TESTASSERT(C::count == 1); { auto exp2 = std::move(exp); - TESTASSERT(exp2 and exp2.value()->val == 5); - TESTASSERT(exp and exp.value()->val == 0); + TESTASSERT(exp2 and exp2.value().val == 5); + TESTASSERT(exp and exp.value().val == 0); } exp.set_error(2); - TESTASSERT(not exp and *exp.error() == 2); + TESTASSERT(not exp and exp.error() == 2); return SRSLTE_SUCCESS; } @@ -129,13 +129,13 @@ int test_unique_ptr() { srslte::expected > exp; TESTASSERT(exp); - exp.value()->reset(new C{2}); - TESTASSERT(exp.value()->get()->val == 2); + exp.value().reset(new C{2}); + TESTASSERT(exp.value()->val == 2); { auto exp2 = std::move(exp); - TESTASSERT(exp.value()->get() == nullptr); - TESTASSERT(exp2 and exp2.value()->get()->val == 2); + TESTASSERT(exp.value() == nullptr); + TESTASSERT(exp2 and exp2.value()->val == 2); } return SRSLTE_SUCCESS;