message: correctly unpack variants of enums
When a variant contains multiple types which can be converted
from a string (such as two enums, or an enum and string), the
previous code tried a conversion from the first type in the
variant. When the first type was an enum, this often threw an
exception. When the first type was a string, this turned into
a plain string containing the dbus enum value.
Add special cases to the variant unpacking to detect types
which are convertible from strings and delegate to a template
specialization of 'convert_from_string' which handles variants.
This specialization will attempt all conversions first and then
revert to a string as a fallback.
Fixes openbmc/sdbusplus#52.
Change-Id: Ide5a0030d595fbaf01122fa8a0ecdaa19ad0078c
Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
diff --git a/include/sdbusplus/message/native_types.hpp b/include/sdbusplus/message/native_types.hpp
index 2624d34..c5bb54a 100644
--- a/include/sdbusplus/message/native_types.hpp
+++ b/include/sdbusplus/message/native_types.hpp
@@ -1,7 +1,9 @@
#pragma once
+#include <optional>
#include <string>
#include <string_view>
+#include <variant>
namespace sdbusplus
{
@@ -239,6 +241,59 @@
inline constexpr bool has_convert_from_string_v =
has_convert_from_string<T>::value;
+// Specialization of 'convert_from_string' for variant.
+template <typename... Types>
+struct convert_from_string<std::variant<Types...>>
+{
+ static auto op(const std::string& str)
+ -> std::optional<std::variant<Types...>>
+ {
+ if constexpr (0 < sizeof...(Types))
+ {
+ return process<Types...>(str);
+ }
+ return {};
+ }
+
+ // We need to iterate through all the variant types and find
+ // the one which matches the contents of the string. Often,
+ // a variant can contain both a convertible-type (ie. enum) and
+ // a string, so we need to iterate through all the convertible-types
+ // first and convert to string as a last resort.
+ template <typename T, typename... Args>
+ static auto process(const std::string& str)
+ -> std::optional<std::variant<Types...>>
+ {
+ // If convert_from_string exists for the type, attempt it.
+ if constexpr (has_convert_from_string_v<T>)
+ {
+ auto r = convert_from_string<T>::op(str);
+ if (r)
+ {
+ return r;
+ }
+ }
+
+ // If there are any more types in the variant, try them.
+ if constexpr (0 < sizeof...(Args))
+ {
+ auto r = process<Args...>(str);
+ if (r)
+ {
+ return r;
+ }
+ }
+
+ // Otherwise, if this is a string, do last-resort conversion.
+ if constexpr (std::is_same_v<std::string, std::remove_cv_t<T>>)
+ {
+ return str;
+ }
+
+ return {};
+ }
+};
+
} // namespace details
} // namespace message
diff --git a/include/sdbusplus/message/read.hpp b/include/sdbusplus/message/read.hpp
index e64066e..2e6a30d 100644
--- a/include/sdbusplus/message/read.hpp
+++ b/include/sdbusplus/message/read.hpp
@@ -375,6 +375,10 @@
template <typename... Args>
struct read_single<std::variant<Args...>>
{
+ // Downcast
+ template <typename T>
+ using Td = types::details::type_id_downcast_t<T>;
+
template <typename S, typename S1, typename... Args1>
static void read(sdbusplus::SdBusInterface* intf, sd_bus_message* m, S&& s)
{
@@ -393,8 +397,6 @@
return;
}
- std::remove_reference_t<S1> s1;
-
r = intf->sd_bus_message_enter_container(m, SD_BUS_TYPE_VARIANT,
dbusType.data());
if (r < 0)
@@ -403,7 +405,31 @@
-r, "sd_bus_message_enter_container variant");
}
- sdbusplus::message::read(intf, m, s1);
+ // If this type is an enum or string, we don't know which is the
+ // valid parsing. Delegate to 'convert_from_string' so we do the
+ // correct conversion.
+ if constexpr (std::is_enum_v<Td<S1>> ||
+ std::is_same_v<std::string, Td<S1>>)
+ {
+ std::string str{};
+ sdbusplus::message::read(intf, m, str);
+ auto r =
+ sdbusplus::message::convert_from_string<std::variant<Args...>>(
+ str);
+
+ if (!r)
+ {
+ throw sdbusplus::exception::InvalidEnumString();
+ }
+
+ s = std::move(*r);
+ }
+ else // otherise, read it out directly.
+ {
+ std::remove_reference_t<S1> s1;
+ sdbusplus::message::read(intf, m, s1);
+ s = std::move(s1);
+ }
r = intf->sd_bus_message_exit_container(m);
if (r < 0)
@@ -411,8 +437,6 @@
throw exception::SdBusError(
-r, "sd_bus_message_exit_container variant");
}
-
- s = std::move(s1);
}
template <typename S>
diff --git a/test/server/message_variant.cpp b/test/server/message_variant.cpp
index 2fa44b7..fef1eff 100644
--- a/test/server/message_variant.cpp
+++ b/test/server/message_variant.cpp
@@ -51,6 +51,9 @@
"EnumTwo does not have convert_from_string!");
static_assert(!sdbusplus::message::details::has_convert_from_string_v<size_t>,
"size_t unexpectedly has a convert_from_string!");
+static_assert(sdbusplus::message::details::has_convert_from_string_v<
+ TestIf::PropertiesVariant>,
+ "TestIf::PropertiesVariant does not convert_from_string!");
TEST_F(Object, PlainEnumOne)
{
@@ -74,7 +77,7 @@
TestIf::EnumTwo::TwoB);
}
-TEST_F(Object, DISABLED_VariantAsString)
+TEST_F(Object, VariantAsString)
{
run_test<variant_t>(std::string("Hello"));
}
@@ -84,7 +87,7 @@
run_test<variant_t>(TestIf::EnumOne::OneA);
}
-TEST_F(Object, DISABLED_VariantAsEnumTwo)
+TEST_F(Object, VariantAsEnumTwo)
{
run_test<variant_t>(TestIf::EnumTwo::TwoB);
}