Fix uninitialized property, aserver instance
Had some
"conditional move or branch depends on uninitialized value",
and digging revealed this. The 'property.py' is modified to
zero-initialize the properties [1].
The newly introduced test cases show that the uninitialized value only
occurred on the aserver instance for some reason.
Without this patch, one of these tests would fail.
The patch is more of a band-aid than anything else, since
zero-initialization or default constructing
sdbusplus::message::object_path is actually not a valid object path [2].
So there needs to be another patch to add some default constructor
there.
While doing this, found that the
- interface.server.hpp.mako
- interface.aserver.hpp.mako
use a different template to define the properties, which is confusing.
Cleaned that up so they declare their properties in the same way.
Tested: Unit tests pass
References:
[1] https://en.cppreference.com/w/cpp/language/zero_initialization
[2] https://dbus.freedesktop.org/doc/dbus-specification.html#message-protocol-marshaling-object-path
Change-Id: I6ec5823f24fe9430546fc119e83e647ae2b3f8c9
Signed-off-by: Alexander Hansen <alexander.hansen@9elements.com>
diff --git a/test/gen/test_aserver_no_uninitialized_value_constructor.cpp b/test/gen/test_aserver_no_uninitialized_value_constructor.cpp
new file mode 100644
index 0000000..b03a4d6
--- /dev/null
+++ b/test/gen/test_aserver_no_uninitialized_value_constructor.cpp
@@ -0,0 +1,25 @@
+#include "sdbusplus/async.hpp"
+#include "server/Test/aserver.hpp"
+
+#include <sdbusplus/async/context.hpp>
+
+#include <memory>
+#include <regex>
+
+#include <gtest/gtest.h>
+
+class A : public sdbusplus::aserver::server::Test<A>
+{};
+
+int main()
+{
+ sdbusplus::async::context ctx;
+
+ sdbusplus::aserver::server::Test<A> t2(ctx, "/");
+
+ assert(t2.some_value() == 0);
+
+ t2.some_value(4);
+
+ return 0;
+}
diff --git a/test/gen/test_server_no_uninitialized_value_constructor.cpp b/test/gen/test_server_no_uninitialized_value_constructor.cpp
new file mode 100644
index 0000000..14039dc
--- /dev/null
+++ b/test/gen/test_server_no_uninitialized_value_constructor.cpp
@@ -0,0 +1,21 @@
+
+#include "sdbusplus/server.hpp"
+#include "server/Test/server.hpp"
+
+#include <memory>
+#include <regex>
+
+#include <gtest/gtest.h>
+
+int main()
+{
+ auto bus = sdbusplus::bus::new_default();
+
+ sdbusplus::server::server::Test t(bus, "/");
+
+ assert(t.someValue() == 0);
+
+ t.someValue(3);
+
+ return 0;
+}
diff --git a/test/meson.build b/test/meson.build
index ca7ee4c..aa07577 100644
--- a/test/meson.build
+++ b/test/meson.build
@@ -113,3 +113,21 @@
dependencies: [gmock_dep, gtest_dep, server_test_dep],
),
)
+
+uninit_tests = [
+ 'test_server_no_uninitialized_value_constructor',
+ 'test_aserver_no_uninitialized_value_constructor',
+]
+
+foreach t : uninit_tests
+ test(
+ t,
+ executable(
+ t,
+ f'gen/@t@.cpp',
+ generated_sources,
+ include_directories: [root_inc],
+ dependencies: [sdbusplus_dep, server_test_dep],
+ ),
+ )
+endforeach
diff --git a/tools/sdbusplus/property.py b/tools/sdbusplus/property.py
index a055fe5..c1b24bb 100644
--- a/tools/sdbusplus/property.py
+++ b/tools/sdbusplus/property.py
@@ -88,7 +88,7 @@
def default_value(self, interface):
if self.defaultValue is None:
- return ""
+ return " = {}"
value = str(self.defaultValue)
enum_prefix = ""
if self.is_enum():
diff --git a/tools/sdbusplus/templates/interface.server.hpp.mako b/tools/sdbusplus/templates/interface.server.hpp.mako
index 5d203c5..3fed9a2 100644
--- a/tools/sdbusplus/templates/interface.server.hpp.mako
+++ b/tools/sdbusplus/templates/interface.server.hpp.mako
@@ -141,19 +141,10 @@
sdbusplus::server::interface_t
_${interface.joinedName("_", "interface")};
bus_t& _sdbusplus_bus;
+
% for p in interface.properties:
- % if p.defaultValue is not None:
- ${p.cppTypeParam(interface.name)} _${p.camelCase} = \
- % if p.is_enum():
-${p.cppTypeParam(interface.name)}::\
- % endif
-${p.defaultValue};
- % else:
- ${p.cppTypeParam(interface.name)} _${p.camelCase}{};
- % endif
-
+ ${p.cppTypeParam(interface.name)} _${p.camelCase}${p.default_value(interface.name)};
% endfor
-
};
} // namespace sdbusplus::server::${interface.cppNamespace()}
diff --git a/tools/sdbusplus/templates/property.aserver.callback.hpp.mako b/tools/sdbusplus/templates/property.aserver.callback.hpp.mako
index 39dda8a..0e78fff 100644
--- a/tools/sdbusplus/templates/property.aserver.callback.hpp.mako
+++ b/tools/sdbusplus/templates/property.aserver.callback.hpp.mako
@@ -16,7 +16,7 @@
auto m = sdbusplus::message_t{reply};
// Set up the transaction.
- server::transaction::set_id(m);
+ sdbusplus::server::transaction::set_id(m);
// Get property value and add to message.
if constexpr (server_details::has_get_property_msg<${p_tag},
@@ -64,7 +64,7 @@
auto m = sdbusplus::message_t{value};
// Set up the transaction.
- server::transaction::set_id(m);
+ sdbusplus::server::transaction::set_id(m);
auto new_value = m.unpack<${p_type}>();