Skip to content

Commit

Permalink
refactor: make Variant constructor explicit (#370)
Browse files Browse the repository at this point in the history
This makes the library more robust and prone to user's errors when the user writes an extension for their custom type. In case they forget to implement a serialization function for that type and yet insert an object of that type into sdbus::Message, the current behavior is that, surprisingly, the library masks the error as it resolves the call to the Variant overload, because Variant provides an implicit template converting constructor, so the library tries to construct first the Variant object from the object of custom type, and then inserting into the message that Variant object. Variant constructor serializes the underlying object into its internal message object, which resolves to the same message insertion overload, creating an infinite recursion and ultimately the stack overflow. This is undesired and plain wrong. Marking this Variant converting constructor solves these problems, plus in overall it makes the code a little safer and more verbose. With explicit Variant constructor, when the user forgets to implement a serialization function for their type, the call of such function will fail with an expressive compilation error, and will produce no undesired, surprising results.
  • Loading branch information
sangelovic committed Apr 24, 2024
1 parent f9a482c commit 13db519
Show file tree
Hide file tree
Showing 3 changed files with 2 additions and 2 deletions.
1 change: 1 addition & 0 deletions ChangeLog
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,7 @@ v2.0.0
what before had to be obtained through `PollData::getAbsoluteTimeout()` call.
- `PollData::getRelativeTimeout()` return type was changed to `std::chrono::microseconds`.
- `IConnection::processPendingRequest()` was renamed to `IConnection::processPendingEvent()`.
- `Variant` constructor is now explicit.
- Systemd of at least v238 is required to compile sdbus-c++
- A proper fix for timeout handling
- Fix for external event loops in which the event loop thread ID was not correctly initialized (now fixed and simplified by not needing the thread ID anymore)
Expand Down
2 changes: 1 addition & 1 deletion include/sdbus-c++/Types.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ namespace sdbus {
Variant();

template <typename _ValueType>
Variant(const _ValueType& value)
explicit Variant(const _ValueType& value)
: Variant()
{
msg_.openVariant(signature_of<_ValueType>::str());
Expand Down
1 change: 0 additions & 1 deletion tests/integrationtests/DBusSignalsTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,6 @@ TYPED_TEST(SdbusTestObject, EmitsSignalWithVariantSuccesfully)
{
double d = 3.14;
this->m_adaptor->emitSignalWithVariant(sdbus::Variant{d});
this->m_adaptor->emitSignalWithVariant(d);

ASSERT_TRUE(waitUntil(this->m_proxy->m_gotSignalWithVariant));
ASSERT_THAT(this->m_proxy->m_variantFromSignal, DoubleEq(d));
Expand Down

0 comments on commit 13db519

Please sign in to comment.