diff --git a/docs/using-sdbus-c++.md b/docs/using-sdbus-c++.md index 0771d73..e83dae5 100644 --- a/docs/using-sdbus-c++.md +++ b/docs/using-sdbus-c++.md @@ -1780,6 +1780,8 @@ sdbus-c++ v2 is a major release that comes with a number of breaking API/ABI/beh * `createDefaultBusConnection()` has been renamed to `createBusConnection()`. * Change in behavior: `Proxy`s now by default call `createBusConnection()` to get a connection when the connection is not provided explicitly by the caller, so they connect to either the session bus or the system bus depending on the context (as opposed to always to the system bus like before). * Callbacks taking `const sdbus::Error* error` were changed to take `std::optional`, which better expresses the intent and meaning. +* `getInterfaceName()`, `getMemberName()`, `getSender()`, `getPath()` and `getDestination()` methods of `Message` class now return `const char*` instead of `std::string`, for efficiency reasons. +* `peekType()` method of `Message` class now returns a pair of `char` (type signature) and `const char*` (contents signature), for expressiveness and efficiency reasons. * D-Bus signatures when using high-level API are now assembled at compile time. There are breaking changes inside `signature_of` type traits and `Message` serialization/deserialization methods. This only interests you if you extend sdbus-c++ type system with your own types. See the updated tutorial on extending sdbus-c++ type system. * Types and methods marked deprecated in sdbus-c++ v1 were removed completely. * CMake options got `SDBUSCPP_` prefix for better usability and minimal risk of conflicts in downstream CMake projects. `SDBUSCPP_INSTALL` CMake option was added. diff --git a/include/sdbus-c++/Message.h b/include/sdbus-c++/Message.h index de735ab..bc872f3 100644 --- a/include/sdbus-c++/Message.h +++ b/include/sdbus-c++/Message.h @@ -34,6 +34,7 @@ #include #include #include +#include #include #include #ifdef __has_include @@ -52,14 +53,10 @@ namespace sdbus { class Variant; class ObjectPath; - class InterfaceName; - class MemberName; class Signature; template class Struct; class UnixFd; class MethodReply; - class BusName; - using ConnectionName = BusName; namespace internal { class ISdBus; } @@ -202,12 +199,13 @@ namespace sdbus { explicit operator bool() const; void clearFlags(); - InterfaceName getInterfaceName() const; - MemberName getMemberName() const; - ConnectionName getSender() const; - ObjectPath getPath() const; - ConnectionName getDestination() const; - void peekType(std::string& type, std::string& contents) const; + const char* getInterfaceName() const; + const char* getMemberName() const; + const char* getSender() const; + const char* getPath() const; + const char* getDestination() const; + // TODO: short docs in whole Message API + std::pair peekType() const; bool isValid() const; bool isEmpty() const; bool isAtEnd(bool complete) const; @@ -302,7 +300,8 @@ namespace sdbus { public: Signal() = default; - void setDestination(const ConnectionName& destination); + void setDestination(const std::string& destination); + void setDestination(const char* destination); void send() const; }; @@ -474,15 +473,14 @@ namespace sdbus { namespace detail { template - bool deserialize_variant(Message& msg, std::variant<_Elements...>& value, const std::string& signature) + bool deserialize_variant(Message& msg, std::variant<_Elements...>& value, const char* signature) { - constexpr auto elemSignature = sdbus::signature_of_v<_Element>; - // TODO: Try to optimize - if (signature != std::string(elemSignature.begin(), elemSignature.end())) + constexpr auto elemSignature = as_null_terminated(sdbus::signature_of_v<_Element>); + if (std::strcmp(signature, elemSignature.data()) != 0) return false; _Element temp; - msg.enterVariant(signature.c_str()); + msg.enterVariant(signature); msg >> temp; msg.exitVariant(); value = std::move(temp); @@ -493,11 +491,8 @@ namespace sdbus { template inline Message& Message::operator>>(std::variant& value) { - std::string type; - std::string contentType; - // TODO: Refactor ppekType prior to release/v2.0 to return pair of const char* or string_view... - peekType(type, contentType); - bool result = (detail::deserialize_variant(*this, value, contentType) || ...); + auto [type, contents] = peekType(); + bool result = (detail::deserialize_variant(*this, value, contents) || ...); SDBUS_THROW_ERROR_IF(!result, "Failed to deserialize variant: signature did not match any of the variant types", EINVAL); return *this; } diff --git a/include/sdbus-c++/Types.h b/include/sdbus-c++/Types.h index 776bdc8..c927903 100644 --- a/include/sdbus-c++/Types.h +++ b/include/sdbus-c++/Types.h @@ -30,6 +30,7 @@ #include #include +#include #include #include #include @@ -105,14 +106,14 @@ namespace sdbus { bool containsValueOfType() const { constexpr auto signature = as_null_terminated(signature_of_v<_Type>); - return signature.data() == peekValueType(); + return std::strcmp(signature.data(), peekValueType()) == 0; } bool isEmpty() const; void serializeTo(Message& msg) const; void deserializeFrom(Message& msg); - std::string peekValueType() const; + const char* peekValueType() const; private: mutable PlainMessage msg_{}; diff --git a/src/Message.cpp b/src/Message.cpp index cda9ccd..0d04a5d 100644 --- a/src/Message.cpp +++ b/src/Message.cpp @@ -612,44 +612,38 @@ void Message::rewind(bool complete) SDBUS_THROW_ERROR_IF(r < 0, "Failed to rewind the message", -r); } -InterfaceName Message::getInterfaceName() const +const char* Message::getInterfaceName() const { - const auto* interface = sd_bus_message_get_interface((sd_bus_message*)msg_); - return interface != nullptr ? InterfaceName{interface} : InterfaceName{}; + return sd_bus_message_get_interface((sd_bus_message*)msg_); } -MemberName Message::getMemberName() const +const char* Message::getMemberName() const { - const auto* member = sd_bus_message_get_member((sd_bus_message*)msg_); - return member != nullptr ? MemberName{member} : MemberName{}; + return sd_bus_message_get_member((sd_bus_message*)msg_); } -ConnectionName Message::getSender() const +const char* Message::getSender() const { - const auto* sender = sd_bus_message_get_sender((sd_bus_message*)msg_); - return ConnectionName{sender}; + return sd_bus_message_get_sender((sd_bus_message*)msg_); } -ObjectPath Message::getPath() const +const char* Message::getPath() const { - const auto* path = sd_bus_message_get_path((sd_bus_message*)msg_); - return path != nullptr ? ObjectPath{path} : ObjectPath{}; + return sd_bus_message_get_path((sd_bus_message*)msg_); } -ConnectionName Message::getDestination() const +const char* Message::getDestination() const { - const auto* destination = sd_bus_message_get_destination((sd_bus_message*)msg_); - return destination != nullptr ? ConnectionName{destination} : ConnectionName{}; + return sd_bus_message_get_destination((sd_bus_message*)msg_); } -void Message::peekType(std::string& type, std::string& contents) const +std::pair Message::peekType() const { - char typeSig; - const char* contentsSig; - auto r = sd_bus_message_peek_type((sd_bus_message*)msg_, &typeSig, &contentsSig); + char typeSignature{}; + const char* contentsSignature{}; + auto r = sd_bus_message_peek_type((sd_bus_message*)msg_, &typeSignature, &contentsSignature); SDBUS_THROW_ERROR_IF(r < 0, "Failed to peek message type", -r); - type = typeSig; - contents = contentsSig ? contentsSig : ""; + return {typeSignature, contentsSignature}; } bool Message::isValid() const @@ -877,9 +871,14 @@ void Signal::send() const SDBUS_THROW_ERROR_IF(r < 0, "Failed to emit signal", -r); } -void Signal::setDestination(const ConnectionName& destination) +void Signal::setDestination(const std::string& destination) { - auto r = sdbus_->sd_bus_message_set_destination((sd_bus_message*)msg_, destination.c_str()); + return setDestination(destination.c_str()); +} + +void Signal::setDestination(const char* destination) +{ + auto r = sdbus_->sd_bus_message_set_destination((sd_bus_message*)msg_, destination); SDBUS_THROW_ERROR_IF(r < 0, "Failed to set signal destination", -r); } diff --git a/src/Object.cpp b/src/Object.cpp index 70300c1..3e2a8db 100644 --- a/src/Object.cpp +++ b/src/Object.cpp @@ -300,8 +300,6 @@ const Object::VTable::MethodItem* Object::findMethod(const VTable& vtable, std:: return methodItem.name < methodName; }); - (void)it; - return it != vtable.methods.end() && it->name == methodName ? &*it : nullptr; } @@ -312,8 +310,6 @@ const Object::VTable::PropertyItem* Object::findProperty(const VTable& vtable, s return propertyItem.name < propertyName; }); - (void)it; - return it != vtable.properties.end() && it->name == propertyName ? &*it : nullptr; } @@ -333,7 +329,7 @@ int Object::sdbus_method_callback(sd_bus_message *sdbusMessage, void *userData, auto message = Message::Factory::create(sdbusMessage, &vtable->object->connection_.getSdBusInterface()); - const auto* methodItem = findMethod(*vtable, message.getMemberName()); // TODO: optimize the situation around getMemberName() + const auto* methodItem = findMethod(*vtable, message.getMemberName()); assert(methodItem != nullptr); assert(methodItem->callback); diff --git a/src/Proxy.cpp b/src/Proxy.cpp index a26ea3a..135b702 100644 --- a/src/Proxy.cpp +++ b/src/Proxy.cpp @@ -113,7 +113,6 @@ PendingAsyncCall Proxy::callMethodAsync(const MethodCall& message, async_reply_h pendingAsyncCalls_.addCall(std::move(callData)); - // TODO: Instead of PendingAsyncCall consider using Slot implementation for simplicity and consistency return {weakData}; } diff --git a/src/Types.cpp b/src/Types.cpp index 81d27e8..4f91e93 100644 --- a/src/Types.cpp +++ b/src/Types.cpp @@ -55,12 +55,10 @@ void Variant::deserializeFrom(Message& msg) msg_.seal(); } -std::string Variant::peekValueType() const +const char* Variant::peekValueType() const { msg_.rewind(false); - std::string type; - std::string contents; - msg_.peekType(type, contents); + auto [type, contents] = msg_.peekType(); return contents; } diff --git a/tests/integrationtests/DBusGeneralTests.cpp b/tests/integrationtests/DBusGeneralTests.cpp index 7f9dae0..294cd0f 100644 --- a/tests/integrationtests/DBusGeneralTests.cpp +++ b/tests/integrationtests/DBusGeneralTests.cpp @@ -44,6 +44,7 @@ using ::testing::ElementsAre; using ::testing::Eq; using namespace std::chrono_literals; using namespace sdbus::test; +using namespace std::string_view_literals; using ADirectConnection = TestFixtureWithDirectConnection; @@ -157,7 +158,7 @@ TYPED_TEST(AConnection, WillNotPassToMatchCallbackMessagesThatDoNotMatchTheRule) std::atomic numberOfMatchingMessages{}; auto slot = this->s_proxyConnection->addMatch(matchRule, [&](sdbus::Message msg) { - if(msg.getMemberName() == "simpleSignal") + if(msg.getMemberName() == "simpleSignal"sv) numberOfMatchingMessages++; }); auto adaptor2 = std::make_unique(*this->s_adaptorConnection, OBJECT_PATH_2); diff --git a/tests/unittests/Message_test.cpp b/tests/unittests/Message_test.cpp index d1d4175..05e4468 100644 --- a/tests/unittests/Message_test.cpp +++ b/tests/unittests/Message_test.cpp @@ -32,8 +32,10 @@ #include using ::testing::Eq; +using ::testing::StrEq; using ::testing::Gt; using ::testing::DoubleEq; +using ::testing::IsNull; using namespace std::string_literals; namespace @@ -465,11 +467,10 @@ TEST(AMessage, CanPeekASimpleType) msg << 123; msg.seal(); - std::string type; - std::string contents; - msg.peekType(type, contents); - ASSERT_THAT(type, "i"); - ASSERT_THAT(contents, ""); + auto [type, contents] = msg.peekType(); + + ASSERT_THAT(type, Eq('i')); + ASSERT_THAT(contents, IsNull()); } TEST(AMessage, CanPeekContainerContents) @@ -478,11 +479,10 @@ TEST(AMessage, CanPeekContainerContents) msg << std::map{{1, "one"}, {2, "two"}}; msg.seal(); - std::string type; - std::string contents; - msg.peekType(type, contents); - ASSERT_THAT(type, "a"); - ASSERT_THAT(contents, "{is}"); + auto [type, contents] = msg.peekType(); + + ASSERT_THAT(type, Eq('a')); + ASSERT_THAT(contents, StrEq("{is}")); } TEST(AMessage, CanCarryDBusArrayGivenAsCustomType)