message: Change conversion ctor to inc ref-count
There are cases of calling the message constructor where the
ownership of the raw pointer is transfered and others where it
is duplicated. The ownership-transfer use-cases are likely all
embedded within sdbusplus itself and the ownership-duplicate
cases tend to be done by external users. This causes the current
API to be bug-prone as it requires something like:
sdbusplus::message::message(sd_bus_message_ref(m));
Change the conversion constructor to not require the external _ref
call and instead add a new constructor that bypasses the ref.
Thus, the calling conventions now look like this:
Ownership-duplicate:
sdbusplus::message::message(m); // calls _ref.
Ownership-transfer:
sdbusplus::message::message(m, std::false_type()); // no _ref.
Resolves openbmc/openbmc#950.
Change-Id: Ia1ae527c4d1235b1625368cfffeb4ed495457768
Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
diff --git a/sdbusplus/bus.hpp b/sdbusplus/bus.hpp
index f0ca1bf..07ce3aa 100644
--- a/sdbusplus/bus.hpp
+++ b/sdbusplus/bus.hpp
@@ -88,7 +88,7 @@
sd_bus_message* m = nullptr;
sd_bus_process(_bus.get(), &m);
- return message::message(m);
+ return message::message(m, std::false_type());
}
/** @brief Process waiting dbus messages or signals, discarding unhandled.
@@ -123,7 +123,7 @@
sd_bus_message_new_method_call(_bus.get(), &m, service, objpath,
interf, method);
- return message::message(m);
+ return message::message(m, std::false_type());
}
/** @brief Create a signal message.
@@ -139,7 +139,7 @@
sd_bus_message* m = nullptr;
sd_bus_message_new_signal(_bus.get(), &m, objpath, interf, member);
- return message::message(m);
+ return message::message(m, std::false_type());
}
/** @brief Perform a message call.
@@ -154,7 +154,7 @@
sd_bus_message* reply = nullptr;
sd_bus_call(_bus.get(), m.get(), timeout_us, nullptr, &reply);
- return message::message(reply);
+ return message::message(reply, std::false_type());
}
/** @brief Perform a message call, ignoring the reply.
diff --git a/sdbusplus/message.hpp b/sdbusplus/message.hpp
index 958e60b..7cb4c1c 100644
--- a/sdbusplus/message.hpp
+++ b/sdbusplus/message.hpp
@@ -1,6 +1,7 @@
#pragma once
#include <memory>
+#include <type_traits>
#include <systemd/sd-bus.h>
#include <sdbusplus/message/append.hpp>
#include <sdbusplus/message/read.hpp>
@@ -57,9 +58,16 @@
/** @brief Conversion constructor for 'msgp_t'.
*
+ * Takes increment ref-count of the msg-pointer and release when
+ * destructed.
+ */
+ explicit message(msgp_t m) : _msg(sd_bus_message_ref(m)) {}
+
+ /** @brief Constructor for 'msgp_t'.
+ *
* Takes ownership of the msg-pointer and releases it when done.
*/
- explicit message(msgp_t m) : _msg(m) {}
+ message(msgp_t m, std::false_type) : _msg(m) {}
/** @brief Release ownership of the stored msg-pointer. */
msgp_t release() { return _msg.release(); }
@@ -137,7 +145,7 @@
msgp_t reply = nullptr;
sd_bus_message_new_method_return(this->get(), &reply);
- return message(reply);
+ return message(reply, std::false_type());
}
/** @brief Perform a 'method-return' response call. */
diff --git a/tools/sdbusplus/templates/interface.mako.server.cpp b/tools/sdbusplus/templates/interface.mako.server.cpp
index 50a6cad..89fa2a9 100644
--- a/tools/sdbusplus/templates/interface.mako.server.cpp
+++ b/tools/sdbusplus/templates/interface.mako.server.cpp
@@ -51,7 +51,7 @@
try
{
- auto m = message::message(sd_bus_message_ref(reply));
+ auto m = message::message(reply);
auto o = static_cast<${classname}*>(context);
m.append(convertForMessage(o->${p.camelCase}()));
@@ -84,7 +84,7 @@
{
try
{
- auto m = message::message(sd_bus_message_ref(value));
+ auto m = message::message(value);
auto o = static_cast<${classname}*>(context);
diff --git a/tools/sdbusplus/templates/method.mako.prototype.hpp b/tools/sdbusplus/templates/method.mako.prototype.hpp
index 1d3cbfb..b58a2e3 100644
--- a/tools/sdbusplus/templates/method.mako.prototype.hpp
+++ b/tools/sdbusplus/templates/method.mako.prototype.hpp
@@ -127,7 +127,7 @@
{
### Need to add a ref to msg since we attached it to an
### sdbusplus::message.
- auto m = message::message(sd_bus_message_ref(msg));
+ auto m = message::message(msg);
% if len(method.parameters) != 0:
${parameters_as_local(as_param=False)}{};