asio: unpack in place
The existing code uses several different tuples to do the unpack, then
concatenates them with tuple_cat. Simplify the code by generating a
tuple of references to the dbus args, and unpack directly to the main
struct, thus simplifying the flow.
To accomplish this the make_dbus_args_tuple helper method is added,
which strips the non-dbus arguments from a callback so they can be
unpacked separately. This is relatively complex because this code tries
to support
callback(boost::error_code, message_t, dbus_arg);
as well as
callback(boost::error_code, dbus_arg);
Dynamically dependent on which overload is provided. There's likely
more cleanup that can happen there to separate these two, but this
should at least help.
Tested: OpenBMC launches and runs. Redfish service validator (which
should hit the majority of these overloads) passes.
Change-Id: I6bb7ee960459a505eb8633b1cdb30cd2b3037466
Signed-off-by: Ed Tanous <etanous@nvidia.com>
diff --git a/include/sdbusplus/asio/connection.hpp b/include/sdbusplus/asio/connection.hpp
index e33769c..76e03bf 100644
--- a/include/sdbusplus/asio/connection.hpp
+++ b/include/sdbusplus/asio/connection.hpp
@@ -33,6 +33,7 @@
#include <boost/callable_traits/args.hpp>
#include <sdbusplus/asio/detail/async_send_handler.hpp>
#include <sdbusplus/message.hpp>
+#include <sdbusplus/utility/make_dbus_args_tuple.hpp>
#include <sdbusplus/utility/read_into_tuple.hpp>
#include <sdbusplus/utility/type_traits.hpp>
@@ -107,52 +108,32 @@
#endif
template <typename MessageHandler>
- static void unpack(boost::system::error_code ec, message_t& r,
+ static void unpack(const boost::system::error_code& ec, message_t& r,
MessageHandler&& handler)
{
using FunctionTuple = boost::callable_traits::args_t<MessageHandler>;
using FunctionTupleType = utility::decay_tuple_t<FunctionTuple>;
- constexpr bool returnWithMsg = []() {
- if constexpr ((std::tuple_size_v<FunctionTupleType>) > 1)
- {
- return std::is_same_v<
- std::tuple_element_t<1, FunctionTupleType>,
- sdbusplus::message_t>;
- }
- return false;
- }();
- using UnpackType = utility::strip_first_n_args_t<returnWithMsg ? 2 : 1,
- FunctionTupleType>;
- UnpackType responseData;
- if (!ec)
+ FunctionTupleType responseData;
+ if (ec)
+ {
+ std::get<0>(responseData) = ec;
+ }
+ else
{
try
{
- utility::read_into_tuple(responseData, r);
+ auto unpack = utility::make_dbus_args_tuple(responseData);
+ std::apply([&r](auto&&... x) { (r.read(x), ...); }, unpack);
}
catch (const std::exception&)
{
// Set error code if not already set
- ec = boost::system::errc::make_error_code(
- boost::system::errc::invalid_argument);
+ std::get<0>(responseData) =
+ boost::system::errc::make_error_code(
+ boost::system::errc::invalid_argument);
}
}
- // Note. Callback is called whether or not the unpack was
- // successful to allow the user to implement their own
- // handling
- if constexpr (returnWithMsg)
- {
- auto response =
- std::tuple_cat(std::make_tuple(ec), std::forward_as_tuple(r),
- std::move(responseData));
- std::apply(handler, response);
- }
- else
- {
- auto response =
- std::tuple_cat(std::make_tuple(ec), std::move(responseData));
- std::apply(handler, response);
- }
+ std::apply(handler, responseData);
}
/** @brief Perform an asynchronous method call, with input parameter packing
diff --git a/include/sdbusplus/utility/make_dbus_args_tuple.hpp b/include/sdbusplus/utility/make_dbus_args_tuple.hpp
new file mode 100644
index 0000000..ceea03e
--- /dev/null
+++ b/include/sdbusplus/utility/make_dbus_args_tuple.hpp
@@ -0,0 +1,54 @@
+#pragma once
+
+#include <sdbusplus/message.hpp>
+
+#include <cstddef>
+#include <cstdint>
+#include <tuple>
+#include <utility>
+
+namespace sdbusplus
+{
+namespace utility
+{
+namespace details
+{
+template <std::size_t FirstArgIndex, typename Tuple, std::size_t... Is>
+auto make_sub_tuple_impl(Tuple& t, std::index_sequence<Is...>)
+{
+ return std::tie(std::get<FirstArgIndex + Is>(t)...);
+}
+
+template <typename TupleType>
+constexpr size_t args_to_skip()
+{
+ constexpr std::size_t input_size = std::tuple_size_v<TupleType>;
+ if constexpr (input_size > 1)
+ {
+ if (std::is_same_v<std::tuple_element_t<1, TupleType>,
+ sdbusplus::message_t>)
+ {
+ return 2;
+ }
+ }
+ return 1;
+}
+
+} // namespace details
+
+/*
+ * Given a tuple of function args to a method callback, return a tuple
+ * with references to only the dbus message arguments.
+ * */
+template <typename... Args>
+constexpr auto make_dbus_args_tuple(std::tuple<Args...>& t)
+{
+ using tuple_type = std::remove_reference_t<decltype(t)>;
+ constexpr std::size_t input_size = std::tuple_size_v<tuple_type>;
+ constexpr size_t skip_args = details::args_to_skip<tuple_type>();
+ constexpr std::size_t new_size = input_size - skip_args;
+ std::index_sequence seq = std::make_index_sequence<new_size>{};
+ return details::make_sub_tuple_impl<skip_args>(t, seq);
+}
+} // namespace utility
+} // namespace sdbusplus
diff --git a/test/meson.build b/test/meson.build
index 300c3bc..2b7bcfc 100644
--- a/test/meson.build
+++ b/test/meson.build
@@ -37,6 +37,7 @@
'message/types',
'timer',
'unpack_properties',
+ 'utility/make_dbus_args_tuple',
'utility/tuple_to_array',
'utility/type_traits',
]
diff --git a/test/utility/make_dbus_args_tuple.cpp b/test/utility/make_dbus_args_tuple.cpp
new file mode 100644
index 0000000..6ec2d71
--- /dev/null
+++ b/test/utility/make_dbus_args_tuple.cpp
@@ -0,0 +1,49 @@
+#include <boost/system/error_code.hpp>
+#include <sdbusplus/message.hpp>
+#include <sdbusplus/utility/make_dbus_args_tuple.hpp>
+
+#include <gtest/gtest.h>
+
+namespace sdbusplus
+{
+namespace utility
+{
+
+TEST(MakeDbusArgsTuple, MessageFirst)
+{
+ std::tuple<boost::system::error_code, sdbusplus::message_t, int>
+ input_tuple;
+ auto tuple_out = make_dbus_args_tuple(input_tuple);
+ static_assert(
+ std::is_same_v<std::tuple_element_t<0, decltype(tuple_out)>, int&>,
+ "Second type wasn't int");
+ static_assert(std::tuple_size_v<decltype(tuple_out)> == 1,
+ "Size was wrong");
+ // Verify the output reference is now the first member, and references the 2
+ // index tuple arg
+ EXPECT_EQ(&std::get<2>(input_tuple), &std::get<0>(tuple_out));
+}
+TEST(MakeDbusArgsTuple, ArgFirst)
+{
+ std::tuple<boost::system::error_code, int> input_tuple{
+ boost::system::error_code(), 42};
+ auto tuple_out = make_dbus_args_tuple(input_tuple);
+ static_assert(
+ std::is_same_v<std::tuple_element_t<0, decltype(tuple_out)>, int&>,
+ "Second type wasn't int");
+ static_assert(std::tuple_size_v<decltype(tuple_out)> == 1,
+ "Size was wrong");
+ // Verify the output reference is now the first member, and references the 1
+ // index tuple arg
+ EXPECT_EQ(&std::get<1>(input_tuple), &std::get<0>(tuple_out));
+ EXPECT_EQ(std::get<0>(tuple_out), 42);
+}
+TEST(MakeDbusArgsTuple, NoArgs)
+{
+ std::tuple<boost::system::error_code> input_tuple;
+ auto tuple_out = make_dbus_args_tuple(input_tuple);
+ static_assert(std::tuple_size_v<decltype(tuple_out)> == 0,
+ "Size was wrong");
+}
+} // namespace utility
+} // namespace sdbusplus