diff --git a/.clang-tidy b/.clang-tidy new file mode 100644 index 00000000..9192a16e --- /dev/null +++ b/.clang-tidy @@ -0,0 +1,76 @@ +--- +#Checks: "*,\ +# -modernize-use-trailing-return-type,\ +# -google-readability-braces-around-statements,\ +# -google-readability-todo,\ +# -hicpp-braces-around-statements,\ +# -readability-braces-around-statements,\ +# -hicpp-no-array-decay,\ +# -cppcoreguidelines-pro-type-vararg,\ +# -cppcoreguidelines-pro-bounds-constant-array-index,\ +# -cppcoreguidelines-pro-bounds-array-to-pointer-decay,\ +# -hicpp-vararg,\ +# -hicpp-signed-bitwise,\ +# -llvm-header-guard,\ +# -llvmlibc-*,\ +# -google-runtime-references,\ +# -readability-redundant-access-specifiers,\ +# -cppcoreguidelines-avoid-magic-numbers,\ +# -readability-magic-numbers,\ +# -altera-*,\ +# -fuchsia-*" + +# TODO: enable -llvm-include-order in the end, after clang-format +Checks: "*,\ + -llvmlibc-*,\ + -altera-*,\ + -fuchsia-*, + -cert-err58-cpp,\ + -modernize-use-trailing-return-type,\ + -cppcoreguidelines-avoid-magic-numbers,\ + -readability-magic-numbers,\ + -readability-braces-around-statements,\ + -google-readability-braces-around-statements,\ + -hicpp-braces-around-statements,\ + -hicpp-signed-bitwise,\ + -llvm-include-order,\ + -google-runtime-int,\ + -hicpp-named-parameter,\ + -readability-named-parameter,\ + -bugprone-macro-parentheses,\ + -google-readability-todo,\ + -cppcoreguidelines-pro-type-reinterpret-cast,\ + -cppcoreguidelines-pro-bounds-array-to-pointer-decay,\ + -hicpp-no-array-decay,\ + -cppcoreguidelines-avoid-c-arrays,\ + -hicpp-avoid-c-arrays,\ + -modernize-avoid-c-arrays" + +#TODO: Add readability-implicit-bool-conversion and remove // NOLINT(readability-implicit-bool-conversion) from code + +HeaderFilterRegex: 'src|sdbus-c++' +FormatStyle: file +WarningsAsErrors: "*" +CheckOptions: + - key: misc-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic + value: '1' + - key: readability-implicit-bool-conversion.AllowPointerConditions + value: '1' + - key: readability-redundant-member-init.IgnoreBaseInCopyConstructors + value: '1' + - key: cppcoreguidelines-special-member-functions.AllowSoleDefaultDtor + value: '1' + - key: hicpp-special-member-functions.AllowSoleDefaultDtor + value: '1' + - key: readability-identifier-length.IgnoredVariableNames + value: 'r|fd|id|ok|it' + - key: readability-identifier-length.IgnoredParameterNames + value: 'fd|id|b' + - key: performance-move-const-arg.CheckTriviallyCopyableMove + value: '0' + - key: hicpp-move-const-arg.CheckTriviallyCopyableMove + value: '0' +# - key: bugprone-easily-swappable-parameters.MinimumLength +# value: '3' +# - key: readability-braces-around-statements.ShortStatementLines +# value: '3' diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8463dfd7..9fe2d778 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -46,34 +46,26 @@ jobs: - name: configure-debug-gcc11 # For gcc 11, turn off the annoying deprecated-copy warning if: matrix.build == 'shared-libsystemd' && matrix.compiler == 'g++' && matrix.os == 'ubuntu-22.04' run: | - mkdir build - cd build - cmake -DCMAKE_BUILD_TYPE=Debug -DCMAKE_INSTALL_PREFIX:PATH=/usr -DCMAKE_CXX_FLAGS="-O0 -g -W -Wextra -Wall -Wnon-virtual-dtor -Wno-deprecated-copy -Werror $SDBUSCPP_EXTRA_CXX_FLAGS" -DCMAKE_VERBOSE_MAKEFILE=ON -DSDBUSCPP_INSTALL=ON -DSDBUSCPP_BUILD_TESTS=ON -DSDBUSCPP_BUILD_PERF_TESTS=ON -DSDBUSCPP_BUILD_STRESS_TESTS=ON -DSDBUSCPP_BUILD_CODEGEN=ON -DSDBUSCPP_GOOGLETEST_VERSION=1.14.0 .. + cmake -B _build -DCMAKE_BUILD_TYPE=Debug -DCMAKE_INSTALL_PREFIX:PATH=/usr -DCMAKE_CXX_FLAGS="-O0 -g -W -Wextra -Wall -Wnon-virtual-dtor -Wno-deprecated-copy -Werror $SDBUSCPP_EXTRA_CXX_FLAGS" -DCMAKE_VERBOSE_MAKEFILE=ON -DSDBUSCPP_INSTALL=ON -DSDBUSCPP_BUILD_TESTS=ON -DSDBUSCPP_BUILD_PERF_TESTS=ON -DSDBUSCPP_BUILD_STRESS_TESTS=ON -DSDBUSCPP_BUILD_CODEGEN=ON -DSDBUSCPP_GOOGLETEST_VERSION=1.14.0 - name: configure-debug if: matrix.build == 'shared-libsystemd' && (matrix.compiler != 'g++' || matrix.os != 'ubuntu-22.04') run: | - mkdir build - cd build - cmake -DCMAKE_BUILD_TYPE=Debug -DCMAKE_INSTALL_PREFIX:PATH=/usr -DCMAKE_CXX_FLAGS="-O0 -g -W -Wextra -Wall -Wnon-virtual-dtor -Werror $SDBUSCPP_EXTRA_CXX_FLAGS" -DCMAKE_VERBOSE_MAKEFILE=ON -DSDBUSCPP_INSTALL=ON -DSDBUSCPP_BUILD_TESTS=ON -DSDBUSCPP_BUILD_PERF_TESTS=ON -DSDBUSCPP_BUILD_STRESS_TESTS=ON -DSDBUSCPP_BUILD_CODEGEN=ON -DSDBUSCPP_GOOGLETEST_VERSION=1.14.0 .. + cmake -B _build -DCMAKE_BUILD_TYPE=Debug -DCMAKE_INSTALL_PREFIX:PATH=/usr -DCMAKE_CXX_FLAGS="-O0 -g -W -Wextra -Wall -Wnon-virtual-dtor -Werror $SDBUSCPP_EXTRA_CXX_FLAGS" -DCMAKE_VERBOSE_MAKEFILE=ON -DSDBUSCPP_INSTALL=ON -DSDBUSCPP_BUILD_TESTS=ON -DSDBUSCPP_BUILD_PERF_TESTS=ON -DSDBUSCPP_BUILD_STRESS_TESTS=ON -DSDBUSCPP_BUILD_CODEGEN=ON -DSDBUSCPP_GOOGLETEST_VERSION=1.14.0 - name: configure-release-with-embedded-libsystemd if: matrix.build == 'embedded-static-libsystemd' run: | - mkdir build - cd build - cmake -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX:PATH=/usr -DCMAKE_CXX_FLAGS="$SDBUSCPP_EXTRA_CXX_FLAGS" -DCMAKE_VERBOSE_MAKEFILE=ON -DSDBUSCPP_INSTALL=ON -DSDBUSCPP_BUILD_TESTS=ON -DSDBUSCPP_BUILD_PERF_TESTS=ON -DSDBUSCPP_BUILD_STRESS_TESTS=ON -DSDBUSCPP_BUILD_CODEGEN=ON -DSDBUSCPP_BUILD_LIBSYSTEMD=ON -DSDBUSCPP_LIBSYSTEMD_VERSION=252 -DSDBUSCPP_GOOGLETEST_VERSION=1.14.0 .. + cmake -B _build -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX:PATH=/usr -DCMAKE_CXX_FLAGS="$SDBUSCPP_EXTRA_CXX_FLAGS" -DCMAKE_VERBOSE_MAKEFILE=ON -DSDBUSCPP_INSTALL=ON -DSDBUSCPP_BUILD_TESTS=ON -DSDBUSCPP_BUILD_PERF_TESTS=ON -DSDBUSCPP_BUILD_STRESS_TESTS=ON -DSDBUSCPP_BUILD_CODEGEN=ON -DSDBUSCPP_BUILD_LIBSYSTEMD=ON -DSDBUSCPP_LIBSYSTEMD_VERSION=252 -DSDBUSCPP_GOOGLETEST_VERSION=1.14.0 - name: make run: | - cd build - cmake --build . -j4 + cmake --build _build -j4 - name: verify run: | - cd build - sudo cmake --build . --target install + sudo cmake --build _build --target install ctest --output-on-failure - name: pack if: matrix.build == 'shared-libsystemd' run: | - cd build + cd _build cpack -G DEB - name: 'Upload Artifact' if: matrix.build == 'shared-libsystemd' && matrix.compiler == 'g++' @@ -81,9 +73,30 @@ jobs: with: name: "debian-packages-${{ matrix.os }}-${{ matrix.compiler }}" path: | - build/sdbus-c++*.deb - build/sdbus-c++*.ddeb + _build/sdbus-c++*.deb + _build/sdbus-c++*.ddeb retention-days: 10 + static-analysis: + name: static-analysis (ubuntu-24.04, clang-tidy, shared-libsystemd) + runs-on: ubuntu-24.04 + strategy: + fail-fast: false + steps: + - uses: actions/checkout@v4 + - name: install-deps + run: | + sudo apt-get update -y + sudo apt-get install -y libsystemd-dev libgmock-dev clang + sudo update-alternatives --remove-all cc + sudo update-alternatives --install /usr/bin/cc cc /usr/bin/clang 10 + sudo update-alternatives --remove-all c++ + sudo update-alternatives --install /usr/bin/c++ c++ /usr/bin/clang++ 10 + - name: configure + run: | + cmake -B _build -DCMAKE_BUILD_TYPE=Debug -DCMAKE_CXX_FLAGS="-O0 -g" -DCMAKE_VERBOSE_MAKEFILE=ON -DSDBUSCPP_CLANG_TIDY=ON -DSDBUSCPP_BUILD_TESTS=ON -DSDBUSCPP_BUILD_PERF_TESTS=ON -DSDBUSCPP_BUILD_STRESS_TESTS=ON + - name: make + run: | + cmake --build _build -j4 freebsd-build: name: build (freebsd, clang/libc++, basu) runs-on: ubuntu-22.04 # until https://github.com/actions/runner/issues/385 diff --git a/CMakeLists.txt b/CMakeLists.txt index 06eece94..6e812515 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -35,7 +35,7 @@ option(SDBUSCPP_BUILD_DOCS "Build documentation for sdbus-c++" ON) if(SDBUSCPP_BUILD_DOCS) option(SDBUSCPP_BUILD_DOXYGEN_DOCS "Build doxygen documentation for sdbus-c++ API" OFF) endif() -#option(SDBUSCPP_CLANG_TIDY "Co-compile with clang-tidy static analyzer" OFF) +option(SDBUSCPP_CLANG_TIDY "Co-compile with clang-tidy static analyzer" OFF) #option(SDBUSCPP_COVERAGE "Build sdbus-c++ with code coverage instrumentation" OFF) # We promote the BUILD_SHARED_LIBS flag to a (global) option only if we are the main project if(CMAKE_SOURCE_DIR STREQUAL PROJECT_SOURCE_DIR) @@ -262,6 +262,26 @@ if(SDBUSCPP_BUILD_DOCS) add_subdirectory("${CMAKE_CURRENT_SOURCE_DIR}/docs") endif() +#---------------------------------- +# STATIC ANALYSIS +#---------------------------------- + +if(SDBUSCPP_CLANG_TIDY) + message(STATUS "Building with static analysis") + #set(CLANG_TIDY "/home/one/CLion2/clion-2024.3.5/bin/clang/linux/x64/bin/clang-tidy") + find_program(CLANG_TIDY NAMES clang-tidy) + if(NOT CLANG_TIDY) + message(WARNING "clang-tidy not found") + else() + message(STATUS "clang-tidy found: ${CLANG_TIDY}") + set_target_properties(sdbus-c++-objlib PROPERTIES CXX_CLANG_TIDY "${CLANG_TIDY}") + set_target_properties(sdbus-c++ PROPERTIES CXX_CLANG_TIDY "${CLANG_TIDY}") + #set_target_properties(sdbus-c++-unit-tests PROPERTIES CXX_CLANG_TIDY "${CLANG_TIDY}") + #set_target_properties(sdbus-c++-integration-tests PROPERTIES CXX_CLANG_TIDY "${CLANG_TIDY}") + set_target_properties(sdbus-c++-stress-tests PROPERTIES CXX_CLANG_TIDY "${CLANG_TIDY}") + endif() +endif() + #---------------------------------- # CMAKE CONFIG & PACKAGE CONFIG #---------------------------------- diff --git a/src/Connection.cpp b/src/Connection.cpp index 7c667b55..9fa0bc23 100644 --- a/src/Connection.cpp +++ b/src/Connection.cpp @@ -105,9 +105,19 @@ Connection::Connection(std::unique_ptr&& interface, pseudo_bus_t) } Connection::~Connection() +try { Connection::leaveEventLoop(); } +catch (...) +{ + // Theoretically, we can fail to notify the event fd or join with the joinable thread... + // What to do now here in the destructor? That is the question: + // 1. Report the problem... but how, where? + // 2. Terminate immediately... too harsh? + // 3. Ignore and go on... even when some resources may be lingering? + // Since the failure here is expected to be very unlikely, we choose the defensive approach of (3). +} void Connection::requestName(const ServiceName& name) { @@ -194,7 +204,7 @@ Slot Connection::addObjectManager(const ObjectPath& objectPath, return_slot_t) SDBUS_THROW_ERROR_IF(r < 0, "Failed to add object manager", -r); - return {slot, [this](void *slot){ sdbus_->sd_bus_slot_unref((sd_bus_slot*)slot); }}; + return {slot, [this](void *slot){ sdbus_->sd_bus_slot_unref(static_cast(slot)); }}; } void Connection::setMethodCallTimeout(uint64_t timeout) @@ -206,7 +216,7 @@ void Connection::setMethodCallTimeout(uint64_t timeout) uint64_t Connection::getMethodCallTimeout() const { - uint64_t timeout; + uint64_t timeout{}; auto r = sdbus_->sd_bus_get_method_call_timeout(bus_.get(), &timeout); @@ -230,9 +240,9 @@ Slot Connection::addMatch(const std::string& match, message_handler callback, re auto r = sdbus_->sd_bus_add_match(bus_.get(), &slot, match.c_str(), &Connection::sdbus_match_callback, matchInfo.get()); SDBUS_THROW_ERROR_IF(r < 0, "Failed to add match", -r); - matchInfo->slot = {slot, [this](void *slot){ sdbus_->sd_bus_slot_unref((sd_bus_slot*)slot); }}; + matchInfo->slot = {slot, [this](void *slot){ sdbus_->sd_bus_slot_unref(static_cast(slot)); }}; - return {matchInfo.release(), [](void *ptr){ delete static_cast(ptr); }}; + return {matchInfo.release(), [](void *ptr){ delete static_cast(ptr); }}; // NOLINT(cppcoreguidelines-owning-memory) } void Connection::addMatchAsync(const std::string& match, message_handler callback, message_handler installCallback) @@ -259,9 +269,9 @@ Slot Connection::addMatchAsync( const std::string& match , matchInfo.get()); SDBUS_THROW_ERROR_IF(r < 0, "Failed to add match", -r); - matchInfo->slot = {slot, [this](void *slot){ sdbus_->sd_bus_slot_unref((sd_bus_slot*)slot); }}; + matchInfo->slot = {slot, [this](void *slot){ sdbus_->sd_bus_slot_unref(static_cast(slot)); }}; - return {matchInfo.release(), [](void *ptr){ delete static_cast(ptr); }}; + return {matchInfo.release(), [](void *ptr){ delete static_cast(ptr); }}; // NOLINT(cppcoreguidelines-owning-memory) } void Connection::attachSdEventLoop(sd_event *event, int priority) @@ -306,7 +316,7 @@ Slot Connection::createSdEventSlot(sd_event *event) (void)sd_event_default(&event); SDBUS_THROW_ERROR_IF(!event, "Invalid sd_event handle", EINVAL); - return Slot{event, [](void* event){ sd_event_unref((sd_event*)event); }}; + return Slot{event, [](void* event){ sd_event_unref(static_cast(event)); }}; } Slot Connection::createSdTimeEventSourceSlot(sd_event *event, int priority) @@ -314,7 +324,7 @@ Slot Connection::createSdTimeEventSourceSlot(sd_event *event, int priority) sd_event_source *timeEventSource{}; auto r = sd_event_add_time(event, &timeEventSource, CLOCK_MONOTONIC, 0, 0, onSdTimerEvent, this); SDBUS_THROW_ERROR_IF(r < 0, "Failed to add timer event", -r); - Slot sdTimeEventSource{timeEventSource, [](void* source){ deleteSdEventSource((sd_event_source*)source); }}; + Slot sdTimeEventSource{timeEventSource, [](void* source){ deleteSdEventSource(static_cast(source)); }}; r = sd_event_source_set_priority(timeEventSource, priority); SDBUS_THROW_ERROR_IF(r < 0, "Failed to set time event priority", -r); @@ -325,12 +335,12 @@ Slot Connection::createSdTimeEventSourceSlot(sd_event *event, int priority) return sdTimeEventSource; } -Slot Connection::createSdIoEventSourceSlot(sd_event *event, int fd, int priority) +Slot Connection::createSdIoEventSourceSlot(sd_event *event, int fd, int priority) // NOLINT(bugprone-easily-swappable-parameters) { sd_event_source *ioEventSource{}; auto r = sd_event_add_io(event, &ioEventSource, fd, 0, onSdIoEvent, this); SDBUS_THROW_ERROR_IF(r < 0, "Failed to add io event", -r); - Slot sdIoEventSource{ioEventSource, [](void* source){ deleteSdEventSource((sd_event_source*)source); }}; + Slot sdIoEventSource{ioEventSource, [](void* source){ deleteSdEventSource(static_cast(source)); }}; r = sd_event_source_set_prepare(ioEventSource, onSdEventPrepare); SDBUS_THROW_ERROR_IF(r < 0, "Failed to set prepare callback for IO event", -r); @@ -344,12 +354,12 @@ Slot Connection::createSdIoEventSourceSlot(sd_event *event, int fd, int priority return sdIoEventSource; } -Slot Connection::createSdInternalEventSourceSlot(sd_event *event, int fd, int priority) +Slot Connection::createSdInternalEventSourceSlot(sd_event *event, int fd, int priority) // NOLINT(bugprone-easily-swappable-parameters) { sd_event_source *internalEventSource{}; auto r = sd_event_add_io(event, &internalEventSource, fd, 0, onSdInternalEvent, this); SDBUS_THROW_ERROR_IF(r < 0, "Failed to add internal event", -r); - Slot sdInternalEventSource{internalEventSource, [](void* source){ deleteSdEventSource((sd_event_source*)source); }}; + Slot sdInternalEventSource{internalEventSource, [](void* source){ deleteSdEventSource(static_cast(source)); }}; // sd-event loop calls prepare callbacks for all event sources, not just for the one that fired now. // So since onSdEventPrepare is already registered on ioEventSource, we don't need to duplicate it here. @@ -367,7 +377,7 @@ Slot Connection::createSdInternalEventSourceSlot(sd_event *event, int fd, int pr int Connection::onSdTimerEvent(sd_event_source */*s*/, uint64_t /*usec*/, void *userdata) { - auto connection = static_cast(userdata); + auto *connection = static_cast(userdata); assert(connection != nullptr); (void)connection->processPendingEvent(); @@ -377,7 +387,7 @@ int Connection::onSdTimerEvent(sd_event_source */*s*/, uint64_t /*usec*/, void * int Connection::onSdIoEvent(sd_event_source */*s*/, int /*fd*/, uint32_t /*revents*/, void *userdata) { - auto connection = static_cast(userdata); + auto *connection = static_cast(userdata); assert(connection != nullptr); (void)connection->processPendingEvent(); @@ -387,7 +397,7 @@ int Connection::onSdIoEvent(sd_event_source */*s*/, int /*fd*/, uint32_t /*reven int Connection::onSdInternalEvent(sd_event_source */*s*/, int /*fd*/, uint32_t /*revents*/, void *userdata) { - auto connection = static_cast(userdata); + auto *connection = static_cast(userdata); assert(connection != nullptr); // It's not really necessary to processPendingEvent() here. We just clear the event fd. @@ -410,7 +420,7 @@ int Connection::onSdInternalEvent(sd_event_source */*s*/, int /*fd*/, uint32_t / int Connection::onSdEventPrepare(sd_event_source */*s*/, void *userdata) { - auto connection = static_cast(userdata); + auto *connection = static_cast(userdata); assert(connection != nullptr); auto sdbusPollData = connection->getEventLoopPollData(); @@ -432,16 +442,16 @@ int Connection::onSdEventPrepare(sd_event_source */*s*/, void *userdata) // In case the timeout is infinite, we disable the timer in the sd_event loop. // This prevents a syscall error, where `timerfd_settime` returns `EINVAL`, // because the value is too big. See #324 for details - r = sd_event_source_set_enabled(sdTimeEventSource, sdbusPollData.timeout != sdbusPollData.timeout.max() ? SD_EVENT_ONESHOT : SD_EVENT_OFF); + r = sd_event_source_set_enabled(sdTimeEventSource, sdbusPollData.timeout != std::chrono::microseconds::max() ? SD_EVENT_ONESHOT : SD_EVENT_OFF); SDBUS_THROW_ERROR_IF(r < 0, "Failed to enable time event source", -r); return 1; } -void Connection::deleteSdEventSource(sd_event_source *s) +void Connection::deleteSdEventSource(sd_event_source *source) { #if LIBSYSTEMD_VERSION>=243 - sd_event_source_disable_unref(s); + sd_event_source_disable_unref(source); #else sd_event_source_set_enabled(s, SD_EVENT_OFF); sd_event_source_unref(s); @@ -467,7 +477,7 @@ Slot Connection::addObjectVTable( const ObjectPath& objectPath SDBUS_THROW_ERROR_IF(r < 0, "Failed to register object vtable", -r); - return {slot, [this](void *slot){ sdbus_->sd_bus_slot_unref((sd_bus_slot*)slot); }}; + return {slot, [this](void *slot){ sdbus_->sd_bus_slot_unref(static_cast(slot)); }}; } PlainMessage Connection::createPlainMessage() const @@ -478,6 +488,8 @@ PlainMessage Connection::createPlainMessage() const SDBUS_THROW_ERROR_IF(r < 0, "Failed to create a plain message", -r); + // TODO: const_cast..? Finish the const correctness design + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-const-cast) return Message::Factory::create(sdbusMsg, const_cast(this), adopt_message); } @@ -498,13 +510,15 @@ MethodCall Connection::createMethodCall( const char* destination auto r = sdbus_->sd_bus_message_new_method_call( bus_.get() , &sdbusMsg - , !*destination ? nullptr : destination + , *destination == '\0' ? nullptr : destination , objectPath , interfaceName , methodName); SDBUS_THROW_ERROR_IF(r < 0, "Failed to create method call", -r); + // TODO: const_cast..? Finish the const correctness design + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-const-cast) return Message::Factory::create(sdbusMsg, const_cast(this), adopt_message); } @@ -525,6 +539,8 @@ Signal Connection::createSignal( const char* objectPath SDBUS_THROW_ERROR_IF(r < 0, "Failed to create signal", -r); + // TODO: const_cast..? Finish the const correctness design + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-const-cast) return Message::Factory::create(sdbusMsg, const_cast(this), adopt_message); } @@ -544,7 +560,7 @@ void Connection::emitPropertiesChangedSignal( const char* objectPath auto r = sdbus_->sd_bus_emit_properties_changed_strv( bus_.get() , objectPath , interfaceName - , propNames.empty() ? nullptr : &names[0] ); + , propNames.empty() ? nullptr : names.data() ); SDBUS_THROW_ERROR_IF(r < 0, "Failed to emit PropertiesChanged signal", -r); } @@ -563,7 +579,7 @@ void Connection::emitInterfacesAddedSignal( const ObjectPath& objectPath auto r = sdbus_->sd_bus_emit_interfaces_added_strv( bus_.get() , objectPath.c_str() - , interfaces.empty() ? nullptr : &names[0] ); + , interfaces.empty() ? nullptr : names.data() ); SDBUS_THROW_ERROR_IF(r < 0, "Failed to emit InterfacesAdded signal", -r); } @@ -582,7 +598,7 @@ void Connection::emitInterfacesRemovedSignal( const ObjectPath& objectPath auto r = sdbus_->sd_bus_emit_interfaces_removed_strv( bus_.get() , objectPath.c_str() - , interfaces.empty() ? nullptr : &names[0] ); + , interfaces.empty() ? nullptr : names.data() ); SDBUS_THROW_ERROR_IF(r < 0, "Failed to emit InterfacesRemoved signal", -r); } @@ -599,16 +615,16 @@ Slot Connection::registerSignalHandler( const char* sender auto r = sdbus_->sd_bus_match_signal( bus_.get() , &slot - , !*sender ? nullptr : sender - , !*objectPath ? nullptr : objectPath - , !*interfaceName ? nullptr : interfaceName - , !*signalName ? nullptr : signalName + , *sender == '\0' ? nullptr : sender + , *objectPath == '\0' ? nullptr : objectPath + , *interfaceName == '\0' ? nullptr : interfaceName + , *signalName == '\0' ? nullptr : signalName , callback , userData ); SDBUS_THROW_ERROR_IF(r < 0, "Failed to register signal handler", -r); - return {slot, [this](void *slot){ sdbus_->sd_bus_slot_unref((sd_bus_slot*)slot); }}; + return {slot, [this](void *slot){ sdbus_->sd_bus_slot_unref(static_cast(slot)); }}; } sd_bus_message* Connection::incrementMessageRefCount(sd_bus_message* sdbusMsg) @@ -646,7 +662,7 @@ sd_bus_message* Connection::callMethod(sd_bus_message* sdbusMsg, uint64_t timeou sd_bus_message* sdbusReply{}; auto r = sdbus_->sd_bus_call(nullptr, sdbusMsg, timeout, &sdbusError, &sdbusReply); - if (sd_bus_error_is_set(&sdbusError)) + if (sd_bus_error_is_set(&sdbusError)) // NOLINT(readability-implicit-bool-conversion) throw Error(Error::Name{sdbusError.name}, sdbusError.message); SDBUS_THROW_ERROR_IF(r < 0, "Failed to call method", -r); @@ -664,7 +680,7 @@ Slot Connection::callMethodAsync(sd_bus_message* sdbusMsg, sd_bus_message_handle // TODO: Think of ways of optimizing these three locking/unlocking of sdbus mutex (merge into one call?) auto timeoutBefore = getEventLoopPollData().timeout; - auto r = sdbus_->sd_bus_call_async(nullptr, &slot, sdbusMsg, (sd_bus_message_handler_t)callback, userData, timeout); + auto r = sdbus_->sd_bus_call_async(nullptr, &slot, sdbusMsg, callback, userData, timeout); SDBUS_THROW_ERROR_IF(r < 0, "Failed to call method asynchronously", -r); auto timeoutAfter = getEventLoopPollData().timeout; @@ -674,7 +690,7 @@ Slot Connection::callMethodAsync(sd_bus_message* sdbusMsg, sd_bus_message_handle if (timeoutAfter < timeoutBefore || arePendingMessagesInQueues()) notifyEventLoopToWakeUpFromPoll(); - return {slot, [this](void *slot){ sdbus_->sd_bus_slot_unref((sd_bus_slot*)slot); }}; + return {slot, [this](void *slot){ sdbus_->sd_bus_slot_unref(static_cast(slot)); }}; } void Connection::sendMessage(sd_bus_message* sdbusMsg) @@ -783,7 +799,7 @@ void Connection::joinWithEventLoop() bool Connection::processPendingEvent() { - auto bus = bus_.get(); + auto *bus = bus_.get(); assert(bus != nullptr); int r = sdbus_->sd_bus_process(bus, nullptr); @@ -798,7 +814,7 @@ bool Connection::processPendingEvent() return r > 0; } -bool Connection::waitForNextEvent() +bool Connection::waitForNextEvent() // NOLINT(misc-no-recursion) { assert(bus_ != nullptr); assert(loopExitFd_.fd >= 0); @@ -821,7 +837,7 @@ bool Connection::waitForNextEvent() SDBUS_THROW_ERROR_IF(r < 0, "Failed to wait on the bus", -errno); // Wake up notification, in order that we re-enter poll with freshly read PollData (namely, new poll timeout thereof) - if (fds[1].revents & POLLIN) + if (fds[1].revents & POLLIN) // NOLINT(readability-implicit-bool-conversion) { auto cleared = eventFd_.clear(); SDBUS_THROW_ERROR_IF(!cleared, "Failed to read from the event descriptor", -errno); @@ -829,7 +845,7 @@ bool Connection::waitForNextEvent() return waitForNextEvent(); } // Loop exit notification - if (fds[2].revents & POLLIN) + if (fds[2].revents & POLLIN) // NOLINT(readability-implicit-bool-conversion) { auto cleared = loopExitFd_.clear(); SDBUS_THROW_ERROR_IF(!cleared, "Failed to read from the loop exit descriptor", -errno); @@ -854,6 +870,8 @@ Message Connection::getCurrentlyProcessedMessage() const { auto* sdbusMsg = sdbus_->sd_bus_get_current_message(bus_.get()); + // TODO: const_cast..? Finish the const correctness design + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-const-cast) return Message::Factory::create(sdbusMsg, const_cast(this)); } @@ -861,8 +879,9 @@ template std::vector Connection::to_strv(const std::vector& strings) { std::vector strv; + strv.reserve(strings.size()); for (auto& str : strings) - strv.push_back(const_cast(str.c_str())); + strv.push_back(const_cast(str.c_str())); // NOLINT(cppcoreguidelines-pro-type-const-cast) strv.push_back(nullptr); return strv; } @@ -894,8 +913,8 @@ int Connection::sdbus_match_install_callback(sd_bus_message *sdbusMessage, void } Connection::EventFd::EventFd() + : fd(eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK)) { - fd = eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK); SDBUS_THROW_ERROR_IF(fd < 0, "Failed to create event object", -errno); } @@ -905,14 +924,14 @@ Connection::EventFd::~EventFd() close(fd); } -void Connection::EventFd::notify() +void Connection::EventFd::notify() const { assert(fd >= 0); auto r = eventfd_write(fd, 1); SDBUS_THROW_ERROR_IF(r < 0, "Failed to notify event descriptor", -errno); } -bool Connection::EventFd::clear() +bool Connection::EventFd::clear() const { assert(fd >= 0); @@ -933,10 +952,10 @@ std::chrono::microseconds IConnection::PollData::getRelativeTimeout() const if (timeout == zero) return zero; - else if (timeout == max) + if (timeout == max) return max; - else - return std::max(std::chrono::duration_cast(timeout - now()), zero); + + return std::max(std::chrono::duration_cast(timeout - now()), zero); } int IConnection::PollData::getPollTimeout() const @@ -945,8 +964,8 @@ int IConnection::PollData::getPollTimeout() const if (relativeTimeout == decltype(relativeTimeout)::max()) return -1; - else - return static_cast(std::chrono::ceil(relativeTimeout).count()); + + return static_cast(std::chrono::ceil(relativeTimeout).count()); } } // namespace sdbus diff --git a/src/Connection.h b/src/Connection.h index ed28bf4d..cbd05393 100644 --- a/src/Connection.h +++ b/src/Connection.h @@ -202,24 +202,24 @@ namespace sdbus::internal { private: #ifndef SDBUS_basu // sd_event integration is not supported if instead of libsystemd we are based on basu - Slot createSdEventSlot(sd_event *event); + static Slot createSdEventSlot(sd_event *event); Slot createSdTimeEventSourceSlot(sd_event *event, int priority); Slot createSdIoEventSourceSlot(sd_event *event, int fd, int priority); Slot createSdInternalEventSourceSlot(sd_event *event, int fd, int priority); - static void deleteSdEventSource(sd_event_source *s); + static void deleteSdEventSource(sd_event_source *source); - static int onSdTimerEvent(sd_event_source *s, uint64_t usec, void *userdata); - static int onSdIoEvent(sd_event_source *s, int fd, uint32_t revents, void *userdata); - static int onSdInternalEvent(sd_event_source *s, int fd, uint32_t revents, void *userdata); - static int onSdEventPrepare(sd_event_source *s, void *userdata); + static int onSdTimerEvent(sd_event_source *source, uint64_t usec, void *userdata); + static int onSdIoEvent(sd_event_source *source, int fd, uint32_t revents, void *userdata); + static int onSdInternalEvent(sd_event_source *source, int fd, uint32_t revents, void *userdata); + static int onSdEventPrepare(sd_event_source *source, void *userdata); #endif struct EventFd { EventFd(); ~EventFd(); - void notify(); - bool clear(); + void notify() const; + bool clear() const; int fd{-1}; }; diff --git a/src/Error.cpp b/src/Error.cpp index a93e956f..38c797f7 100644 --- a/src/Error.cpp +++ b/src/Error.cpp @@ -44,6 +44,6 @@ namespace sdbus message.append(sdbusError.message); message.append(")"); - return Error(std::move(name), std::move(message)); + return {std::move(name), std::move(message)}; } } // namespace sdbus diff --git a/src/Flags.cpp b/src/Flags.cpp index ef943b06..319b12fb 100644 --- a/src/Flags.cpp +++ b/src/Flags.cpp @@ -33,7 +33,6 @@ namespace sdbus { uint64_t sdbusFlags{}; - using namespace sdbus; if (flags_.test(Flags::DEPRECATED)) sdbusFlags |= SD_BUS_VTABLE_DEPRECATED; if (!flags_.test(Flags::PRIVILEGED)) @@ -55,7 +54,6 @@ namespace sdbus { uint64_t sdbusFlags{}; - using namespace sdbus; if (flags_.test(Flags::DEPRECATED)) sdbusFlags |= SD_BUS_VTABLE_DEPRECATED; if (!flags_.test(Flags::PRIVILEGED)) @@ -70,7 +68,6 @@ namespace sdbus { uint64_t sdbusFlags{}; - using namespace sdbus; if (flags_.test(Flags::DEPRECATED)) sdbusFlags |= SD_BUS_VTABLE_DEPRECATED; @@ -81,7 +78,6 @@ namespace sdbus { uint64_t sdbusFlags{}; - using namespace sdbus; if (flags_.test(Flags::DEPRECATED)) sdbusFlags |= SD_BUS_VTABLE_DEPRECATED; //if (!flags_.test(Flags::PRIVILEGED)) @@ -103,10 +99,9 @@ namespace sdbus { auto sdbusFlags = toSdBusPropertyFlags(); - using namespace sdbus; if (!flags_.test(Flags::PRIVILEGED)) sdbusFlags |= SD_BUS_VTABLE_UNPRIVILEGED; return sdbusFlags; } -} +} // namespace sdbus diff --git a/src/Message.cpp b/src/Message.cpp index 4ccf89ae..22d2d4e1 100644 --- a/src/Message.cpp +++ b/src/Message.cpp @@ -35,6 +35,7 @@ #include #include +#include // std::ignore #include SDBUS_HEADER namespace sdbus { @@ -51,7 +52,7 @@ Message::Message(void *msg, internal::IConnection* connection) noexcept { assert(msg_ != nullptr); assert(connection_ != nullptr); - connection_->incrementMessageRefCount((sd_bus_message*)msg_); + connection_->incrementMessageRefCount(static_cast(msg_)); } Message::Message(void *msg, internal::IConnection* connection, adopt_message_t) noexcept @@ -69,14 +70,17 @@ Message::Message(const Message& other) noexcept Message& Message::operator=(const Message& other) noexcept { + if (this == &other) + return *this; + if (msg_) - connection_->decrementMessageRefCount((sd_bus_message*)msg_); + connection_->decrementMessageRefCount(static_cast(msg_)); msg_ = other.msg_; connection_ = other.connection_; ok_ = other.ok_; - connection_->incrementMessageRefCount((sd_bus_message*)msg_); + connection_->incrementMessageRefCount(static_cast(msg_)); return *this; } @@ -89,7 +93,7 @@ Message::Message(Message&& other) noexcept Message& Message::operator=(Message&& other) noexcept { if (msg_) - connection_->decrementMessageRefCount((sd_bus_message*)msg_); + connection_->decrementMessageRefCount(static_cast(msg_)); msg_ = other.msg_; other.msg_ = nullptr; @@ -104,18 +108,18 @@ Message& Message::operator=(Message&& other) noexcept Message::~Message() { if (msg_) - connection_->decrementMessageRefCount((sd_bus_message*)msg_); + connection_->decrementMessageRefCount(static_cast(msg_)); } Message& Message::operator<<(bool item) { - int intItem = item; + int intItem = item; // NOLINT(readability-implicit-bool-conversion) // Direct sd-bus method, bypassing SdBus mutex, are called here, since Message serialization/deserialization, // as well as getter/setter methods are not thread safe by design. Additionally, they are called frequently, // so some overhead is spared. What is thread-safe in Message class is Message constructors, copy/move operations // and the destructor, so the Message instance can be passed from one thread to another safely. - auto r = sd_bus_message_append_basic((sd_bus_message*)msg_, SD_BUS_TYPE_BOOLEAN, &intItem); + auto r = sd_bus_message_append_basic(static_cast(msg_), SD_BUS_TYPE_BOOLEAN, &intItem); SDBUS_THROW_ERROR_IF(r < 0, "Failed to serialize a bool value", -r); return *this; @@ -123,7 +127,7 @@ Message& Message::operator<<(bool item) Message& Message::operator<<(int16_t item) { - auto r = sd_bus_message_append_basic((sd_bus_message*)msg_, SD_BUS_TYPE_INT16, &item); + auto r = sd_bus_message_append_basic(static_cast(msg_), SD_BUS_TYPE_INT16, &item); SDBUS_THROW_ERROR_IF(r < 0, "Failed to serialize a int16_t value", -r); return *this; @@ -131,7 +135,7 @@ Message& Message::operator<<(int16_t item) Message& Message::operator<<(int32_t item) { - auto r = sd_bus_message_append_basic((sd_bus_message*)msg_, SD_BUS_TYPE_INT32, &item); + auto r = sd_bus_message_append_basic(static_cast(msg_), SD_BUS_TYPE_INT32, &item); SDBUS_THROW_ERROR_IF(r < 0, "Failed to serialize a int32_t value", -r); return *this; @@ -139,7 +143,7 @@ Message& Message::operator<<(int32_t item) Message& Message::operator<<(int64_t item) { - auto r = sd_bus_message_append_basic((sd_bus_message*)msg_, SD_BUS_TYPE_INT64, &item); + auto r = sd_bus_message_append_basic(static_cast(msg_), SD_BUS_TYPE_INT64, &item); SDBUS_THROW_ERROR_IF(r < 0, "Failed to serialize a int64_t value", -r); return *this; @@ -147,7 +151,7 @@ Message& Message::operator<<(int64_t item) Message& Message::operator<<(uint8_t item) { - auto r = sd_bus_message_append_basic((sd_bus_message*)msg_, SD_BUS_TYPE_BYTE, &item); + auto r = sd_bus_message_append_basic(static_cast(msg_), SD_BUS_TYPE_BYTE, &item); SDBUS_THROW_ERROR_IF(r < 0, "Failed to serialize a byte value", -r); return *this; @@ -155,7 +159,7 @@ Message& Message::operator<<(uint8_t item) Message& Message::operator<<(uint16_t item) { - auto r = sd_bus_message_append_basic((sd_bus_message*)msg_, SD_BUS_TYPE_UINT16, &item); + auto r = sd_bus_message_append_basic(static_cast(msg_), SD_BUS_TYPE_UINT16, &item); SDBUS_THROW_ERROR_IF(r < 0, "Failed to serialize a uint16_t value", -r); return *this; @@ -163,7 +167,7 @@ Message& Message::operator<<(uint16_t item) Message& Message::operator<<(uint32_t item) { - auto r = sd_bus_message_append_basic((sd_bus_message*)msg_, SD_BUS_TYPE_UINT32, &item); + auto r = sd_bus_message_append_basic(static_cast(msg_), SD_BUS_TYPE_UINT32, &item); SDBUS_THROW_ERROR_IF(r < 0, "Failed to serialize a uint32_t value", -r); return *this; @@ -171,7 +175,7 @@ Message& Message::operator<<(uint32_t item) Message& Message::operator<<(uint64_t item) { - auto r = sd_bus_message_append_basic((sd_bus_message*)msg_, SD_BUS_TYPE_UINT64, &item); + auto r = sd_bus_message_append_basic(static_cast(msg_), SD_BUS_TYPE_UINT64, &item); SDBUS_THROW_ERROR_IF(r < 0, "Failed to serialize a uint64_t value", -r); return *this; @@ -179,7 +183,7 @@ Message& Message::operator<<(uint64_t item) Message& Message::operator<<(double item) { - auto r = sd_bus_message_append_basic((sd_bus_message*)msg_, SD_BUS_TYPE_DOUBLE, &item); + auto r = sd_bus_message_append_basic(static_cast(msg_), SD_BUS_TYPE_DOUBLE, &item); SDBUS_THROW_ERROR_IF(r < 0, "Failed to serialize a double value", -r); return *this; @@ -187,7 +191,7 @@ Message& Message::operator<<(double item) Message& Message::operator<<(const char* item) { - auto r = sd_bus_message_append_basic((sd_bus_message*)msg_, SD_BUS_TYPE_STRING, item); + auto r = sd_bus_message_append_basic(static_cast(msg_), SD_BUS_TYPE_STRING, item); SDBUS_THROW_ERROR_IF(r < 0, "Failed to serialize a C-string value", -r); return *this; @@ -195,7 +199,7 @@ Message& Message::operator<<(const char* item) Message& Message::operator<<(const std::string& item) { - auto r = sd_bus_message_append_basic((sd_bus_message*)msg_, SD_BUS_TYPE_STRING, item.c_str()); + auto r = sd_bus_message_append_basic(static_cast(msg_), SD_BUS_TYPE_STRING, item.c_str()); SDBUS_THROW_ERROR_IF(r < 0, "Failed to serialize a string value", -r); return *this; @@ -204,7 +208,7 @@ Message& Message::operator<<(const std::string& item) Message& Message::operator<<(std::string_view item) { char* destPtr{}; - auto r = sd_bus_message_append_string_space((sd_bus_message*)msg_, item.length(), &destPtr); + auto r = sd_bus_message_append_string_space(static_cast(msg_), item.length(), &destPtr); SDBUS_THROW_ERROR_IF(r < 0, "Failed to serialize a string_view value", -r); std::memcpy(destPtr, item.data(), item.length()); @@ -221,7 +225,7 @@ Message& Message::operator<<(const Variant &item) Message& Message::operator<<(const ObjectPath &item) { - auto r = sd_bus_message_append_basic((sd_bus_message*)msg_, SD_BUS_TYPE_OBJECT_PATH, item.c_str()); + auto r = sd_bus_message_append_basic(static_cast(msg_), SD_BUS_TYPE_OBJECT_PATH, item.c_str()); SDBUS_THROW_ERROR_IF(r < 0, "Failed to serialize an ObjectPath value", -r); return *this; @@ -229,7 +233,7 @@ Message& Message::operator<<(const ObjectPath &item) Message& Message::operator<<(const Signature &item) { - auto r = sd_bus_message_append_basic((sd_bus_message*)msg_, SD_BUS_TYPE_SIGNATURE, item.c_str()); + auto r = sd_bus_message_append_basic(static_cast(msg_), SD_BUS_TYPE_SIGNATURE, item.c_str()); SDBUS_THROW_ERROR_IF(r < 0, "Failed to serialize a Signature value", -r); return *this; @@ -238,7 +242,7 @@ Message& Message::operator<<(const Signature &item) Message& Message::operator<<(const UnixFd &item) { auto fd = item.get(); - auto r = sd_bus_message_append_basic((sd_bus_message*)msg_, SD_BUS_TYPE_UNIX_FD, &fd); + auto r = sd_bus_message_append_basic(static_cast(msg_), SD_BUS_TYPE_UNIX_FD, &fd); SDBUS_THROW_ERROR_IF(r < 0, "Failed to serialize a UnixFd value", -r); return *this; @@ -246,7 +250,7 @@ Message& Message::operator<<(const UnixFd &item) Message& Message::appendArray(char type, const void *ptr, size_t size) { - auto r = sd_bus_message_append_array((sd_bus_message*)msg_, type, ptr, size); + auto r = sd_bus_message_append_array(static_cast(msg_), type, ptr, size); SDBUS_THROW_ERROR_IF(r < 0, "Failed to serialize an array", -r); return *this; @@ -254,8 +258,8 @@ Message& Message::appendArray(char type, const void *ptr, size_t size) Message& Message::operator>>(bool& item) { - int intItem; - auto r = sd_bus_message_read_basic((sd_bus_message*)msg_, SD_BUS_TYPE_BOOLEAN, &intItem); + int intItem{}; + auto r = sd_bus_message_read_basic(static_cast(msg_), SD_BUS_TYPE_BOOLEAN, &intItem); if (r == 0) ok_ = false; @@ -268,7 +272,7 @@ Message& Message::operator>>(bool& item) Message& Message::operator>>(int16_t& item) { - auto r = sd_bus_message_read_basic((sd_bus_message*)msg_, SD_BUS_TYPE_INT16, &item); + auto r = sd_bus_message_read_basic(static_cast(msg_), SD_BUS_TYPE_INT16, &item); if (r == 0) ok_ = false; @@ -279,7 +283,7 @@ Message& Message::operator>>(int16_t& item) Message& Message::operator>>(int32_t& item) { - auto r = sd_bus_message_read_basic((sd_bus_message*)msg_, SD_BUS_TYPE_INT32, &item); + auto r = sd_bus_message_read_basic(static_cast(msg_), SD_BUS_TYPE_INT32, &item); if (r == 0) ok_ = false; @@ -290,7 +294,7 @@ Message& Message::operator>>(int32_t& item) Message& Message::operator>>(int64_t& item) { - auto r = sd_bus_message_read_basic((sd_bus_message*)msg_, SD_BUS_TYPE_INT64, &item); + auto r = sd_bus_message_read_basic(static_cast(msg_), SD_BUS_TYPE_INT64, &item); if (r == 0) ok_ = false; @@ -301,7 +305,7 @@ Message& Message::operator>>(int64_t& item) Message& Message::operator>>(uint8_t& item) { - auto r = sd_bus_message_read_basic((sd_bus_message*)msg_, SD_BUS_TYPE_BYTE, &item); + auto r = sd_bus_message_read_basic(static_cast(msg_), SD_BUS_TYPE_BYTE, &item); if (r == 0) ok_ = false; @@ -312,7 +316,7 @@ Message& Message::operator>>(uint8_t& item) Message& Message::operator>>(uint16_t& item) { - auto r = sd_bus_message_read_basic((sd_bus_message*)msg_, SD_BUS_TYPE_UINT16, &item); + auto r = sd_bus_message_read_basic(static_cast(msg_), SD_BUS_TYPE_UINT16, &item); if (r == 0) ok_ = false; @@ -323,7 +327,7 @@ Message& Message::operator>>(uint16_t& item) Message& Message::operator>>(uint32_t& item) { - auto r = sd_bus_message_read_basic((sd_bus_message*)msg_, SD_BUS_TYPE_UINT32, &item); + auto r = sd_bus_message_read_basic(static_cast(msg_), SD_BUS_TYPE_UINT32, &item); if (r == 0) ok_ = false; @@ -334,7 +338,7 @@ Message& Message::operator>>(uint32_t& item) Message& Message::operator>>(uint64_t& item) { - auto r = sd_bus_message_read_basic((sd_bus_message*)msg_, SD_BUS_TYPE_UINT64, &item); + auto r = sd_bus_message_read_basic(static_cast(msg_), SD_BUS_TYPE_UINT64, &item); if (r == 0) ok_ = false; @@ -345,7 +349,7 @@ Message& Message::operator>>(uint64_t& item) Message& Message::readArray(char type, const void **ptr, size_t *size) { - auto r = sd_bus_message_read_array((sd_bus_message*)msg_, type, ptr, size); + auto r = sd_bus_message_read_array(static_cast(msg_), type, ptr, size); if (r == 0) ok_ = false; @@ -356,7 +360,7 @@ Message& Message::readArray(char type, const void **ptr, size_t *size) Message& Message::operator>>(double& item) { - auto r = sd_bus_message_read_basic((sd_bus_message*)msg_, SD_BUS_TYPE_DOUBLE, &item); + auto r = sd_bus_message_read_basic(static_cast(msg_), SD_BUS_TYPE_DOUBLE, &item); if (r == 0) ok_ = false; @@ -367,7 +371,7 @@ Message& Message::operator>>(double& item) Message& Message::operator>>(char*& item) { - auto r = sd_bus_message_read_basic((sd_bus_message*)msg_, SD_BUS_TYPE_STRING, &item); + auto r = sd_bus_message_read_basic(static_cast(msg_), SD_BUS_TYPE_STRING, &item); if (r == 0) ok_ = false; @@ -403,7 +407,7 @@ Message& Message::operator>>(Variant &item) Message& Message::operator>>(ObjectPath &item) { char* str{}; - auto r = sd_bus_message_read_basic((sd_bus_message*)msg_, SD_BUS_TYPE_OBJECT_PATH, &str); + auto r = sd_bus_message_read_basic(static_cast(msg_), SD_BUS_TYPE_OBJECT_PATH, &str); if (r == 0) ok_ = false; @@ -418,7 +422,7 @@ Message& Message::operator>>(ObjectPath &item) Message& Message::operator>>(Signature &item) { char* str{}; - auto r = sd_bus_message_read_basic((sd_bus_message*)msg_, SD_BUS_TYPE_SIGNATURE, &str); + auto r = sd_bus_message_read_basic(static_cast(msg_), SD_BUS_TYPE_SIGNATURE, &str); if (r == 0) ok_ = false; @@ -433,7 +437,7 @@ Message& Message::operator>>(Signature &item) Message& Message::operator>>(UnixFd &item) { int fd = -1; - auto r = sd_bus_message_read_basic((sd_bus_message*)msg_, SD_BUS_TYPE_UNIX_FD, &fd); + auto r = sd_bus_message_read_basic(static_cast(msg_), SD_BUS_TYPE_UNIX_FD, &fd); if (r == 0) ok_ = false; @@ -446,7 +450,7 @@ Message& Message::operator>>(UnixFd &item) Message& Message::openContainer(const char* signature) { - auto r = sd_bus_message_open_container((sd_bus_message*)msg_, SD_BUS_TYPE_ARRAY, signature); + auto r = sd_bus_message_open_container(static_cast(msg_), SD_BUS_TYPE_ARRAY, signature); SDBUS_THROW_ERROR_IF(r < 0, "Failed to open a container", -r); return *this; @@ -454,7 +458,7 @@ Message& Message::openContainer(const char* signature) Message& Message::closeContainer() { - auto r = sd_bus_message_close_container((sd_bus_message*)msg_); + auto r = sd_bus_message_close_container(static_cast(msg_)); SDBUS_THROW_ERROR_IF(r < 0, "Failed to close a container", -r); return *this; @@ -462,7 +466,7 @@ Message& Message::closeContainer() Message& Message::openDictEntry(const char* signature) { - auto r = sd_bus_message_open_container((sd_bus_message*)msg_, SD_BUS_TYPE_DICT_ENTRY, signature); + auto r = sd_bus_message_open_container(static_cast(msg_), SD_BUS_TYPE_DICT_ENTRY, signature); SDBUS_THROW_ERROR_IF(r < 0, "Failed to open a dictionary entry", -r); return *this; @@ -470,7 +474,7 @@ Message& Message::openDictEntry(const char* signature) Message& Message::closeDictEntry() { - auto r = sd_bus_message_close_container((sd_bus_message*)msg_); + auto r = sd_bus_message_close_container(static_cast(msg_)); SDBUS_THROW_ERROR_IF(r < 0, "Failed to close a dictionary entry", -r); return *this; @@ -478,7 +482,7 @@ Message& Message::closeDictEntry() Message& Message::openVariant(const char* signature) { - auto r = sd_bus_message_open_container((sd_bus_message*)msg_, SD_BUS_TYPE_VARIANT, signature); + auto r = sd_bus_message_open_container(static_cast(msg_), SD_BUS_TYPE_VARIANT, signature); SDBUS_THROW_ERROR_IF(r < 0, "Failed to open a variant", -r); return *this; @@ -486,7 +490,7 @@ Message& Message::openVariant(const char* signature) Message& Message::closeVariant() { - auto r = sd_bus_message_close_container((sd_bus_message*)msg_); + auto r = sd_bus_message_close_container(static_cast(msg_)); SDBUS_THROW_ERROR_IF(r < 0, "Failed to close a variant", -r); return *this; @@ -494,7 +498,7 @@ Message& Message::closeVariant() Message& Message::openStruct(const char* signature) { - auto r = sd_bus_message_open_container((sd_bus_message*)msg_, SD_BUS_TYPE_STRUCT, signature); + auto r = sd_bus_message_open_container(static_cast(msg_), SD_BUS_TYPE_STRUCT, signature); SDBUS_THROW_ERROR_IF(r < 0, "Failed to open a struct", -r); return *this; @@ -502,7 +506,7 @@ Message& Message::openStruct(const char* signature) Message& Message::closeStruct() { - auto r = sd_bus_message_close_container((sd_bus_message*)msg_); + auto r = sd_bus_message_close_container(static_cast(msg_)); SDBUS_THROW_ERROR_IF(r < 0, "Failed to close a struct", -r); return *this; @@ -510,7 +514,7 @@ Message& Message::closeStruct() Message& Message::enterContainer(const char* signature) { - auto r = sd_bus_message_enter_container((sd_bus_message*)msg_, SD_BUS_TYPE_ARRAY, signature); + auto r = sd_bus_message_enter_container(static_cast(msg_), SD_BUS_TYPE_ARRAY, signature); if (r == 0) ok_ = false; @@ -521,7 +525,7 @@ Message& Message::enterContainer(const char* signature) Message& Message::exitContainer() { - auto r = sd_bus_message_exit_container((sd_bus_message*)msg_); + auto r = sd_bus_message_exit_container(static_cast(msg_)); SDBUS_THROW_ERROR_IF(r < 0, "Failed to exit a container", -r); return *this; @@ -529,7 +533,7 @@ Message& Message::exitContainer() Message& Message::enterDictEntry(const char* signature) { - auto r = sd_bus_message_enter_container((sd_bus_message*)msg_, SD_BUS_TYPE_DICT_ENTRY, signature); + auto r = sd_bus_message_enter_container(static_cast(msg_), SD_BUS_TYPE_DICT_ENTRY, signature); if (r == 0) ok_ = false; @@ -540,7 +544,7 @@ Message& Message::enterDictEntry(const char* signature) Message& Message::exitDictEntry() { - auto r = sd_bus_message_exit_container((sd_bus_message*)msg_); + auto r = sd_bus_message_exit_container(static_cast(msg_)); SDBUS_THROW_ERROR_IF(r < 0, "Failed to exit a dictionary entry", -r); return *this; @@ -548,7 +552,7 @@ Message& Message::exitDictEntry() Message& Message::enterVariant(const char* signature) { - auto r = sd_bus_message_enter_container((sd_bus_message*)msg_, SD_BUS_TYPE_VARIANT, signature); + auto r = sd_bus_message_enter_container(static_cast(msg_), SD_BUS_TYPE_VARIANT, signature); if (r == 0) ok_ = false; @@ -559,7 +563,7 @@ Message& Message::enterVariant(const char* signature) Message& Message::exitVariant() { - auto r = sd_bus_message_exit_container((sd_bus_message*)msg_); + auto r = sd_bus_message_exit_container(static_cast(msg_)); SDBUS_THROW_ERROR_IF(r < 0, "Failed to exit a variant", -r); return *this; @@ -567,7 +571,7 @@ Message& Message::exitVariant() Message& Message::enterStruct(const char* signature) { - auto r = sd_bus_message_enter_container((sd_bus_message*)msg_, SD_BUS_TYPE_STRUCT, signature); + auto r = sd_bus_message_enter_container(static_cast(msg_), SD_BUS_TYPE_STRUCT, signature); if (r == 0) ok_ = false; @@ -578,7 +582,7 @@ Message& Message::enterStruct(const char* signature) Message& Message::exitStruct() { - auto r = sd_bus_message_exit_container((sd_bus_message*)msg_); + auto r = sd_bus_message_exit_container(static_cast(msg_)); SDBUS_THROW_ERROR_IF(r < 0, "Failed to exit a struct", -r); return *this; @@ -597,7 +601,7 @@ void Message::clearFlags() void Message::copyTo(Message& destination, bool complete) const { - auto r = sd_bus_message_copy((sd_bus_message*)destination.msg_, (sd_bus_message*)msg_, complete); + auto r = sd_bus_message_copy(static_cast(destination.msg_), static_cast(msg_), complete); // NOLINT(readability-implicit-bool-conversion) SDBUS_THROW_ERROR_IF(r < 0, "Failed to copy the message", -r); } @@ -605,45 +609,45 @@ void Message::seal() { const auto messageCookie = 1; const auto sealTimeout = 0; - auto r = sd_bus_message_seal((sd_bus_message*)msg_, messageCookie, sealTimeout); + auto r = sd_bus_message_seal(static_cast(msg_), messageCookie, sealTimeout); SDBUS_THROW_ERROR_IF(r < 0, "Failed to seal the message", -r); } void Message::rewind(bool complete) { - auto r = sd_bus_message_rewind((sd_bus_message*)msg_, complete); + auto r = sd_bus_message_rewind(static_cast(msg_), complete); // NOLINT(readability-implicit-bool-conversion) SDBUS_THROW_ERROR_IF(r < 0, "Failed to rewind the message", -r); } const char* Message::getInterfaceName() const { - return sd_bus_message_get_interface((sd_bus_message*)msg_); + return sd_bus_message_get_interface(static_cast(msg_)); } const char* Message::getMemberName() const { - return sd_bus_message_get_member((sd_bus_message*)msg_); + return sd_bus_message_get_member(static_cast(msg_)); } const char* Message::getSender() const { - return sd_bus_message_get_sender((sd_bus_message*)msg_); + return sd_bus_message_get_sender(static_cast(msg_)); } const char* Message::getPath() const { - return sd_bus_message_get_path((sd_bus_message*)msg_); + return sd_bus_message_get_path(static_cast(msg_)); } const char* Message::getDestination() const { - return sd_bus_message_get_destination((sd_bus_message*)msg_); + return sd_bus_message_get_destination(static_cast(msg_)); } uint64_t Message::getCookie() const { - uint64_t cookie; - auto r = sd_bus_message_get_cookie((sd_bus_message*)msg_, &cookie); + uint64_t cookie{}; + auto r = sd_bus_message_get_cookie(static_cast(msg_), &cookie); SDBUS_THROW_ERROR_IF(r < 0, "Failed to get cookie", -r); return cookie; } @@ -652,7 +656,7 @@ std::pair Message::peekType() const { char typeSignature{}; const char* contentsSignature{}; - auto r = sd_bus_message_peek_type((sd_bus_message*)msg_, &typeSignature, &contentsSignature); + auto r = sd_bus_message_peek_type(static_cast(msg_), &typeSignature, &contentsSignature); SDBUS_THROW_ERROR_IF(r < 0, "Failed to peek message type", -r); return {typeSignature, contentsSignature}; } @@ -664,12 +668,12 @@ bool Message::isValid() const bool Message::isEmpty() const { - return sd_bus_message_is_empty((sd_bus_message*)msg_) != 0; + return sd_bus_message_is_empty(static_cast(msg_)) != 0; } bool Message::isAtEnd(bool complete) const { - return sd_bus_message_at_end((sd_bus_message*)msg_, complete) > 0; + return sd_bus_message_at_end(static_cast(msg_), complete) > 0; // NOLINT(readability-implicit-bool-conversion) } // TODO: Create a RAII ownership class for creds with copy&move semantics, doing ref()/unref() under the hood. @@ -681,7 +685,7 @@ pid_t Message::getCredsPid() const sd_bus_creds *creds = nullptr; SCOPE_EXIT{ connection_->decrementCredsRefCount(creds); }; - int r = connection_->querySenderCredentials((sd_bus_message*)msg_, mask, &creds); + int r = connection_->querySenderCredentials(static_cast(msg_), mask, &creds); SDBUS_THROW_ERROR_IF(r < 0, "Failed to get bus creds", -r); pid_t pid = 0; @@ -695,10 +699,10 @@ uid_t Message::getCredsUid() const uint64_t mask = SD_BUS_CREDS_UID | SD_BUS_CREDS_AUGMENT; sd_bus_creds *creds = nullptr; SCOPE_EXIT{ connection_->decrementCredsRefCount(creds); }; - int r = connection_->querySenderCredentials((sd_bus_message*)msg_, mask, &creds); + int r = connection_->querySenderCredentials(static_cast(msg_), mask, &creds); SDBUS_THROW_ERROR_IF(r < 0, "Failed to get bus creds", -r); - uid_t uid = (uid_t)-1; + auto uid = static_cast(-1); r = sd_bus_creds_get_uid(creds, &uid); SDBUS_THROW_ERROR_IF(r < 0, "Failed to get bus cred uid", -r); return uid; @@ -709,10 +713,10 @@ uid_t Message::getCredsEuid() const uint64_t mask = SD_BUS_CREDS_EUID | SD_BUS_CREDS_AUGMENT; sd_bus_creds *creds = nullptr; SCOPE_EXIT{ connection_->decrementCredsRefCount(creds); }; - int r = connection_->querySenderCredentials((sd_bus_message*)msg_, mask, &creds); + int r = connection_->querySenderCredentials(static_cast(msg_), mask, &creds); SDBUS_THROW_ERROR_IF(r < 0, "Failed to get bus creds", -r); - uid_t euid = (uid_t)-1; + auto euid = static_cast(-1); r = sd_bus_creds_get_euid(creds, &euid); SDBUS_THROW_ERROR_IF(r < 0, "Failed to get bus cred euid", -r); return euid; @@ -723,10 +727,10 @@ gid_t Message::getCredsGid() const uint64_t mask = SD_BUS_CREDS_GID | SD_BUS_CREDS_AUGMENT; sd_bus_creds *creds = nullptr; SCOPE_EXIT{ connection_->decrementCredsRefCount(creds); }; - int r = connection_->querySenderCredentials((sd_bus_message*)msg_, mask, &creds); + int r = connection_->querySenderCredentials(static_cast(msg_), mask, &creds); SDBUS_THROW_ERROR_IF(r < 0, "Failed to get bus creds", -r); - gid_t gid = (gid_t)-1; + auto gid = static_cast(-1); r = sd_bus_creds_get_gid(creds, &gid); SDBUS_THROW_ERROR_IF(r < 0, "Failed to get bus cred gid", -r); return gid; @@ -737,10 +741,10 @@ gid_t Message::getCredsEgid() const uint64_t mask = SD_BUS_CREDS_EGID | SD_BUS_CREDS_AUGMENT; sd_bus_creds *creds = nullptr; SCOPE_EXIT{ connection_->decrementCredsRefCount(creds); }; - int r = connection_->querySenderCredentials((sd_bus_message*)msg_, mask, &creds); + int r = connection_->querySenderCredentials(static_cast(msg_), mask, &creds); SDBUS_THROW_ERROR_IF(r < 0, "Failed to get bus creds", -r); - gid_t egid = (gid_t)-1; + auto egid = static_cast(-1); r = sd_bus_creds_get_egid(creds, &egid); SDBUS_THROW_ERROR_IF(r < 0, "Failed to get bus cred egid", -r); return egid; @@ -751,7 +755,7 @@ std::vector Message::getCredsSupplementaryGids() const uint64_t mask = SD_BUS_CREDS_SUPPLEMENTARY_GIDS | SD_BUS_CREDS_AUGMENT; sd_bus_creds *creds = nullptr; SCOPE_EXIT{ connection_->decrementCredsRefCount(creds); }; - int r = connection_->querySenderCredentials((sd_bus_message*)msg_, mask, &creds); + int r = connection_->querySenderCredentials(static_cast(msg_), mask, &creds); SDBUS_THROW_ERROR_IF(r < 0, "Failed to get bus creds", -r); const gid_t *cGids = nullptr; @@ -762,7 +766,7 @@ std::vector Message::getCredsSupplementaryGids() const if (cGids != nullptr) { for (int i = 0; i < r; i++) - gids.push_back(cGids[i]); + gids.push_back(cGids[i]); // NOLINT(cppcoreguidelines-pro-bounds-pointer-arithmetic) } return gids; @@ -773,7 +777,7 @@ std::string Message::getSELinuxContext() const uint64_t mask = SD_BUS_CREDS_AUGMENT | SD_BUS_CREDS_SELINUX_CONTEXT; sd_bus_creds *creds = nullptr; SCOPE_EXIT{ connection_->decrementCredsRefCount(creds); }; - int r = connection_->querySenderCredentials((sd_bus_message*)msg_, mask, &creds); + int r = connection_->querySenderCredentials(static_cast(msg_), mask, &creds); SDBUS_THROW_ERROR_IF(r < 0, "Failed to get bus creds", -r); const char *cLabel = nullptr; @@ -792,13 +796,13 @@ MethodCall::MethodCall( void *msg void MethodCall::dontExpectReply() { - auto r = sd_bus_message_set_expect_reply((sd_bus_message*)msg_, 0); + auto r = sd_bus_message_set_expect_reply(static_cast(msg_), 0); SDBUS_THROW_ERROR_IF(r < 0, "Failed to set the dont-expect-reply flag", -r); } bool MethodCall::doesntExpectReply() const { - auto r = sd_bus_message_get_expect_reply((sd_bus_message*)msg_); + auto r = sd_bus_message_get_expect_reply(static_cast(msg_)); SDBUS_THROW_ERROR_IF(r < 0, "Failed to get the dont-expect-reply flag", -r); return r == 0; } @@ -807,59 +811,59 @@ MethodReply MethodCall::send(uint64_t timeout) const { if (!doesntExpectReply()) return sendWithReply(timeout); - else - return sendWithNoReply(); + + return sendWithNoReply(); } MethodReply MethodCall::sendWithReply(uint64_t timeout) const { - auto* sdbusReply = connection_->callMethod((sd_bus_message*)msg_, timeout); + auto* sdbusReply = connection_->callMethod(static_cast(msg_), timeout); return Factory::create(sdbusReply, connection_, adopt_message); } MethodReply MethodCall::sendWithNoReply() const { - connection_->sendMessage((sd_bus_message*)msg_); + connection_->sendMessage(static_cast(msg_)); return Factory::create(); // No reply } -Slot MethodCall::send(void* callback, void* userData, uint64_t timeout, return_slot_t) const +Slot MethodCall::send(void* callback, void* userData, uint64_t timeout, return_slot_t) const // NOLINT(bugprone-easily-swappable-parameters) { - return connection_->callMethodAsync((sd_bus_message*)msg_, (sd_bus_message_handler_t)callback, userData, timeout, return_slot); + return connection_->callMethodAsync(static_cast(msg_), reinterpret_cast(callback), userData, timeout, return_slot); } MethodReply MethodCall::createReply() const { - auto* sdbusReply = connection_->createMethodReply((sd_bus_message*)msg_); + auto* sdbusReply = connection_->createMethodReply(static_cast(msg_)); return Factory::create(sdbusReply, connection_, adopt_message); } MethodReply MethodCall::createErrorReply(const Error& error) const { - sd_bus_message* sdbusErrorReply = connection_->createErrorReplyMessage((sd_bus_message*)msg_, error); + sd_bus_message* sdbusErrorReply = connection_->createErrorReplyMessage(static_cast(msg_), error); return Factory::create(sdbusErrorReply, connection_, adopt_message); } void MethodReply::send() const { - connection_->sendMessage((sd_bus_message*)msg_); + connection_->sendMessage(static_cast(msg_)); } uint64_t MethodReply::getReplyCookie() const { - uint64_t cookie; - auto r = sd_bus_message_get_reply_cookie((sd_bus_message*)msg_, &cookie); + uint64_t cookie{}; + auto r = sd_bus_message_get_reply_cookie(static_cast(msg_), &cookie); SDBUS_THROW_ERROR_IF(r < 0, "Failed to get cookie", -r); return cookie; } void Signal::send() const { - connection_->sendMessage((sd_bus_message*)msg_); + connection_->sendMessage(static_cast(msg_)); } void Signal::setDestination(const std::string& destination) @@ -869,7 +873,7 @@ void Signal::setDestination(const std::string& destination) void Signal::setDestination(const char* destination) { - auto r = sd_bus_message_set_destination((sd_bus_message*)msg_, destination); + auto r = sd_bus_message_set_destination(static_cast(msg_), destination); SDBUS_THROW_ERROR_IF(r < 0, "Failed to set signal destination", -r); } @@ -886,16 +890,16 @@ namespace { // Another common solution is global sdbus-c++ startup/shutdown functions, but that would be an intrusive change. #ifdef __cpp_constinit -constinit static bool pseudoConnectionDestroyed{}; +constinit bool pseudoConnectionDestroyed{}; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) #else -static bool pseudoConnectionDestroyed{}; +bool pseudoConnectionDestroyed{}; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) #endif std::unique_ptr createPseudoConnection() { auto deleter = [](sdbus::internal::IConnection* con) { - delete con; + delete con; // NOLINT(cppcoreguidelines-owning-memory) pseudoConnectionDestroyed = true; }; @@ -909,7 +913,7 @@ sdbus::internal::IConnection& getPseudoConnectionInstance() if (pseudoConnectionDestroyed) { connection = createPseudoConnection(); // Phoenix rising from the ashes - atexit([](){ connection.~unique_ptr(); }); // We have to manually take care of deleting the phoenix + std::ignore = atexit([](){ connection.~unique_ptr(); }); // We have to manually take care of deleting the phoenix pseudoConnectionDestroyed = false; } @@ -918,7 +922,7 @@ sdbus::internal::IConnection& getPseudoConnectionInstance() return *connection; } -} +} // namespace PlainMessage createPlainMessage() { @@ -930,4 +934,4 @@ PlainMessage createPlainMessage() return connection.createPlainMessage(); } -} +} // namespace sdbus diff --git a/src/Object.cpp b/src/Object.cpp index f0fce0f5..c72657e8 100644 --- a/src/Object.cpp +++ b/src/Object.cpp @@ -69,12 +69,12 @@ Slot Object::addVTable(InterfaceName interfaceName, std::vector vtab // 3rd step -- register the vtable with sd-bus internalVTable->slot = connection_.addObjectVTable( objectPath_ , internalVTable->interfaceName - , &internalVTable->sdbusVTable[0] + , internalVTable->sdbusVTable.data() , internalVTable.get() , return_slot ); // Return vtable wrapped in a Slot object - return {internalVTable.release(), [](void *ptr){ delete static_cast(ptr); }}; + return {internalVTable.release(), [](void *ptr){ delete static_cast(ptr); }}; // NOLINT(cppcoreguidelines-owning-memory) } void Object::unregister() @@ -181,9 +181,9 @@ Object::VTable Object::createInternalVTable(InterfaceName interfaceName, std::ve } // Sort arrays so we can do fast searching for an item in sd-bus callback handlers - std::sort(internalVTable.methods.begin(), internalVTable.methods.end(), [](const auto& a, const auto& b){ return a.name < b.name; }); - std::sort(internalVTable.signals.begin(), internalVTable.signals.end(), [](const auto& a, const auto& b){ return a.name < b.name; }); - std::sort(internalVTable.properties.begin(), internalVTable.properties.end(), [](const auto& a, const auto& b){ return a.name < b.name; }); + std::sort(internalVTable.methods.begin(), internalVTable.methods.end(), [](const auto& lhs, const auto& rhs){ return lhs.name < rhs.name; }); + std::sort(internalVTable.signals.begin(), internalVTable.signals.end(), [](const auto& lhs, const auto& rhs){ return lhs.name < rhs.name; }); + std::sort(internalVTable.properties.begin(), internalVTable.properties.end(), [](const auto& lhs, const auto& rhs){ return lhs.name < rhs.name; }); internalVTable.object = this; @@ -389,7 +389,7 @@ int Object::sdbus_property_set_callback( sd_bus */*bus*/ return ok ? 1 : -1; } -} +} // namespace sdbus::internal namespace sdbus { @@ -401,4 +401,4 @@ std::unique_ptr createObject(sdbus::IConnection& connection, Obj return std::make_unique(*sdbusConnection, std::move(objectPath)); } -} +} // namespace sdbus diff --git a/src/Object.h b/src/Object.h index 2bf5eb76..d6432578 100644 --- a/src/Object.h +++ b/src/Object.h @@ -126,12 +126,12 @@ namespace sdbus::internal { }; VTable createInternalVTable(InterfaceName interfaceName, std::vector vtable); - void writeInterfaceFlagsToVTable(InterfaceFlagsVTableItem flags, VTable& vtable); - void writeMethodRecordToVTable(MethodVTableItem method, VTable& vtable); - void writeSignalRecordToVTable(SignalVTableItem signal, VTable& vtable); - void writePropertyRecordToVTable(PropertyVTableItem property, VTable& vtable); + static void writeInterfaceFlagsToVTable(InterfaceFlagsVTableItem flags, VTable& vtable); + static void writeMethodRecordToVTable(MethodVTableItem method, VTable& vtable); + static void writeSignalRecordToVTable(SignalVTableItem signal, VTable& vtable); + static void writePropertyRecordToVTable(PropertyVTableItem property, VTable& vtable); - std::vector createInternalSdBusVTable(const VTable& vtable); + static std::vector createInternalSdBusVTable(const VTable& vtable); static void startSdBusVTable(const Flags& interfaceFlags, std::vector& vtable); static void writeMethodRecordToSdBusVTable(const VTable::MethodItem& method, std::vector& vtable); static void writeSignalRecordToSdBusVTable(const VTable::SignalItem& signal, std::vector& vtable); diff --git a/src/Proxy.cpp b/src/Proxy.cpp index 14029c9b..59fa9930 100644 --- a/src/Proxy.cpp +++ b/src/Proxy.cpp @@ -124,7 +124,7 @@ PendingAsyncCall Proxy::callMethodAsync(const MethodCall& message, async_reply_h , .proxy = *this , .floating = false }); - asyncCallInfo->slot = message.send((void*)&Proxy::sdbus_async_reply_handler, asyncCallInfo.get(), timeout, return_slot); + asyncCallInfo->slot = message.send(reinterpret_cast(&Proxy::sdbus_async_reply_handler), asyncCallInfo.get(), timeout, return_slot); auto asyncCallInfoWeakPtr = std::weak_ptr{asyncCallInfo}; @@ -141,9 +141,9 @@ Slot Proxy::callMethodAsync(const MethodCall& message, async_reply_handler async , .proxy = *this , .floating = true }); - asyncCallInfo->slot = message.send((void*)&Proxy::sdbus_async_reply_handler, asyncCallInfo.get(), timeout, return_slot); + asyncCallInfo->slot = message.send(reinterpret_cast(&Proxy::sdbus_async_reply_handler), asyncCallInfo.get(), timeout, return_slot); - return {asyncCallInfo.release(), [](void *ptr){ delete static_cast(ptr); }}; + return {asyncCallInfo.release(), [](void *ptr){ delete static_cast(ptr); }}; // NOLINT(cppcoreguidelines-owning-memory) } std::future Proxy::callMethodAsync(const MethodCall& message, with_future_t) @@ -212,7 +212,7 @@ Slot Proxy::registerSignalHandler( const char* interfaceName , signalInfo.get() , return_slot ); - return {signalInfo.release(), [](void *ptr){ delete static_cast(ptr); }}; + return {signalInfo.release(), [](void *ptr){ delete static_cast(ptr); }}; // NOLINT(cppcoreguidelines-owning-memory) } void Proxy::unregister() @@ -326,7 +326,7 @@ void Proxy::FloatingAsyncCallSlots::clear() // mutex) is in progress in a different thread, we get double-mutex deadlock. } -} +} // namespace sdbus::internal namespace sdbus { @@ -354,7 +354,7 @@ bool PendingAsyncCall::isPending() const return !callInfo_.expired(); } -} +} // namespace sdbus namespace sdbus { @@ -374,11 +374,9 @@ std::unique_ptr createProxy( std::unique_ptr&& conne , ServiceName destination , ObjectPath objectPath ) { - auto* sdbusConnection = dynamic_cast(connection.get()); + auto* sdbusConnection = dynamic_cast(connection.release()); SDBUS_THROW_ERROR_IF(!sdbusConnection, "Connection is not a real sdbus-c++ connection", EINVAL); - connection.release(); - return std::make_unique( std::unique_ptr(sdbusConnection) , std::move(destination) , std::move(objectPath) ); @@ -389,11 +387,9 @@ std::unique_ptr createProxy( std::unique_ptr&& conne , ObjectPath objectPath , dont_run_event_loop_thread_t ) { - auto* sdbusConnection = dynamic_cast(connection.get()); + auto* sdbusConnection = dynamic_cast(connection.release()); SDBUS_THROW_ERROR_IF(!sdbusConnection, "Connection is not a real sdbus-c++ connection", EINVAL); - connection.release(); - return std::make_unique( std::unique_ptr(sdbusConnection) , std::move(destination) , std::move(objectPath) @@ -440,4 +436,4 @@ std::unique_ptr createLightWeightProxy(ServiceName destination, O return createProxy(std::move(destination), std::move(objectPath), dont_run_event_loop_thread); } -} +} // namespace sdbus diff --git a/src/SdBus.cpp b/src/SdBus.cpp index 7774212d..4cd8a628 100644 --- a/src/SdBus.cpp +++ b/src/SdBus.cpp @@ -26,87 +26,87 @@ */ #include "SdBus.h" -#include +#include "sdbus-c++/Error.h" #include namespace sdbus::internal { -sd_bus_message* SdBus::sd_bus_message_ref(sd_bus_message *m) +sd_bus_message* SdBus::sd_bus_message_ref(sd_bus_message *msg) { std::lock_guard lock(sdbusMutex_); - return ::sd_bus_message_ref(m); + return ::sd_bus_message_ref(msg); } -sd_bus_message* SdBus::sd_bus_message_unref(sd_bus_message *m) +sd_bus_message* SdBus::sd_bus_message_unref(sd_bus_message *msg) { std::lock_guard lock(sdbusMutex_); - return ::sd_bus_message_unref(m); + return ::sd_bus_message_unref(msg); } -int SdBus::sd_bus_send(sd_bus *bus, sd_bus_message *m, uint64_t *cookie) +int SdBus::sd_bus_send(sd_bus *bus, sd_bus_message *msg, uint64_t *cookie) { std::lock_guard lock(sdbusMutex_); - auto r = ::sd_bus_send(bus, m, cookie); + auto r = ::sd_bus_send(bus, msg, cookie); if (r < 0) return r; return r; } -int SdBus::sd_bus_call(sd_bus *bus, sd_bus_message *m, uint64_t usec, sd_bus_error *ret_error, sd_bus_message **reply) +int SdBus::sd_bus_call(sd_bus *bus, sd_bus_message *msg, uint64_t usec, sd_bus_error *ret_error, sd_bus_message **reply) { std::lock_guard lock(sdbusMutex_); - return ::sd_bus_call(bus, m, usec, ret_error, reply); + return ::sd_bus_call(bus, msg, usec, ret_error, reply); } -int SdBus::sd_bus_call_async(sd_bus *bus, sd_bus_slot **slot, sd_bus_message *m, sd_bus_message_handler_t callback, void *userdata, uint64_t usec) +int SdBus::sd_bus_call_async(sd_bus *bus, sd_bus_slot **slot, sd_bus_message *msg, sd_bus_message_handler_t callback, void *userdata, uint64_t usec) { std::lock_guard lock(sdbusMutex_); - auto r = ::sd_bus_call_async(bus, slot, m, callback, userdata, usec); + auto r = ::sd_bus_call_async(bus, slot, msg, callback, userdata, usec); if (r < 0) return r; return r; } -int SdBus::sd_bus_message_new(sd_bus *bus, sd_bus_message **m, uint8_t type) +int SdBus::sd_bus_message_new(sd_bus *bus, sd_bus_message **msg, uint8_t type) { std::lock_guard lock(sdbusMutex_); - return ::sd_bus_message_new(bus, m, type); + return ::sd_bus_message_new(bus, msg, type); } -int SdBus::sd_bus_message_new_method_call(sd_bus *bus, sd_bus_message **m, const char *destination, const char *path, const char *interface, const char *member) +int SdBus::sd_bus_message_new_method_call(sd_bus *bus, sd_bus_message **msg, const char *destination, const char *path, const char *interface, const char *member) { std::lock_guard lock(sdbusMutex_); - return ::sd_bus_message_new_method_call(bus, m, destination, path, interface, member); + return ::sd_bus_message_new_method_call(bus, msg, destination, path, interface, member); } -int SdBus::sd_bus_message_new_signal(sd_bus *bus, sd_bus_message **m, const char *path, const char *interface, const char *member) +int SdBus::sd_bus_message_new_signal(sd_bus *bus, sd_bus_message **msg, const char *path, const char *interface, const char *member) { std::lock_guard lock(sdbusMutex_); - return ::sd_bus_message_new_signal(bus, m, path, interface, member); + return ::sd_bus_message_new_signal(bus, msg, path, interface, member); } -int SdBus::sd_bus_message_new_method_return(sd_bus_message *call, sd_bus_message **m) +int SdBus::sd_bus_message_new_method_return(sd_bus_message *call, sd_bus_message **msg) { std::lock_guard lock(sdbusMutex_); - return ::sd_bus_message_new_method_return(call, m); + return ::sd_bus_message_new_method_return(call, msg); } -int SdBus::sd_bus_message_new_method_error(sd_bus_message *call, sd_bus_message **m, const sd_bus_error *e) +int SdBus::sd_bus_message_new_method_error(sd_bus_message *call, sd_bus_message **msg, const sd_bus_error *err) { std::lock_guard lock(sdbusMutex_); - return ::sd_bus_message_new_method_error(call, m, e); + return ::sd_bus_message_new_method_error(call, msg, err); } int SdBus::sd_bus_set_method_call_timeout(sd_bus *bus, uint64_t usec) @@ -197,14 +197,14 @@ int SdBus::sd_bus_open_user_with_address(sd_bus **ret, const char* address) if (r < 0) return r; - r = ::sd_bus_set_bus_client(bus, true); + r = ::sd_bus_set_bus_client(bus, true); // NOLINT(readability-implicit-bool-conversion) if (r < 0) return r; // Copying behavior from // https://github.com/systemd/systemd/blob/fee6441601c979165ebcbb35472036439f8dad5f/src/libsystemd/sd-bus/sd-bus.c#L1381 // Here, we make the bus as trusted - r = ::sd_bus_set_trusted(bus, true); + r = ::sd_bus_set_trusted(bus, true); // NOLINT(readability-implicit-bool-conversion) if (r < 0) return r; @@ -276,7 +276,7 @@ int SdBus::sd_bus_open_server(sd_bus **ret, int fd) if (r < 0) return r; - r = ::sd_bus_set_server(bus, true, id); + r = ::sd_bus_set_server(bus, true, id); // NOLINT(readability-implicit-bool-conversion) if (r < 0) return r; @@ -373,11 +373,11 @@ int SdBus::sd_bus_start(sd_bus *bus) return ::sd_bus_start(bus); } -int SdBus::sd_bus_process(sd_bus *bus, sd_bus_message **r) +int SdBus::sd_bus_process(sd_bus *bus, sd_bus_message **msg) { std::lock_guard lock(sdbusMutex_); - return ::sd_bus_process(bus, r); + return ::sd_bus_process(bus, msg); } sd_bus_message* SdBus::sd_bus_get_current_message(sd_bus *bus) @@ -404,7 +404,7 @@ int SdBus::sd_bus_get_poll_data(sd_bus *bus, PollData* data) return r; } -int SdBus::sd_bus_get_n_queued(sd_bus *bus, uint64_t *read, uint64_t* write) +int SdBus::sd_bus_get_n_queued(sd_bus *bus, uint64_t *read, uint64_t* write) // NOLINT(bugprone-easily-swappable-parameters) { std::lock_guard lock(sdbusMutex_); @@ -434,81 +434,81 @@ sd_bus* SdBus::sd_bus_close_unref(sd_bus *bus) #endif } -int SdBus::sd_bus_message_set_destination(sd_bus_message *m, const char *destination) +int SdBus::sd_bus_message_set_destination(sd_bus_message *msg, const char *destination) { std::lock_guard lock(sdbusMutex_); - return ::sd_bus_message_set_destination(m, destination); + return ::sd_bus_message_set_destination(msg, destination); } -int SdBus::sd_bus_query_sender_creds(sd_bus_message *m, uint64_t mask, sd_bus_creds **c) +int SdBus::sd_bus_query_sender_creds(sd_bus_message *msg, uint64_t mask, sd_bus_creds **creds) { std::lock_guard lock(sdbusMutex_); - return ::sd_bus_query_sender_creds(m, mask, c); + return ::sd_bus_query_sender_creds(msg, mask, creds); } -sd_bus_creds* SdBus::sd_bus_creds_ref(sd_bus_creds *c) +sd_bus_creds* SdBus::sd_bus_creds_ref(sd_bus_creds *creds) { std::lock_guard lock(sdbusMutex_); - return ::sd_bus_creds_ref(c); + return ::sd_bus_creds_ref(creds); } -sd_bus_creds* SdBus::sd_bus_creds_unref(sd_bus_creds *c) +sd_bus_creds* SdBus::sd_bus_creds_unref(sd_bus_creds *creds) { std::lock_guard lock(sdbusMutex_); - return ::sd_bus_creds_unref(c); + return ::sd_bus_creds_unref(creds); } -int SdBus::sd_bus_creds_get_pid(sd_bus_creds *c, pid_t *pid) +int SdBus::sd_bus_creds_get_pid(sd_bus_creds *creds, pid_t *pid) { std::lock_guard lock(sdbusMutex_); - return ::sd_bus_creds_get_pid(c, pid); + return ::sd_bus_creds_get_pid(creds, pid); } -int SdBus::sd_bus_creds_get_uid(sd_bus_creds *c, uid_t *uid) +int SdBus::sd_bus_creds_get_uid(sd_bus_creds *creds, uid_t *uid) { std::lock_guard lock(sdbusMutex_); - return ::sd_bus_creds_get_uid(c, uid); + return ::sd_bus_creds_get_uid(creds, uid); } -int SdBus::sd_bus_creds_get_euid(sd_bus_creds *c, uid_t *euid) +int SdBus::sd_bus_creds_get_euid(sd_bus_creds *creds, uid_t *euid) { std::lock_guard lock(sdbusMutex_); - return ::sd_bus_creds_get_euid(c, euid); + return ::sd_bus_creds_get_euid(creds, euid); } -int SdBus::sd_bus_creds_get_gid(sd_bus_creds *c, gid_t *gid) +int SdBus::sd_bus_creds_get_gid(sd_bus_creds *creds, gid_t *gid) { std::lock_guard lock(sdbusMutex_); - return ::sd_bus_creds_get_gid(c, gid); + return ::sd_bus_creds_get_gid(creds, gid); } -int SdBus::sd_bus_creds_get_egid(sd_bus_creds *c, uid_t *egid) +int SdBus::sd_bus_creds_get_egid(sd_bus_creds *creds, uid_t *egid) { std::lock_guard lock(sdbusMutex_); - return ::sd_bus_creds_get_egid(c, egid); + return ::sd_bus_creds_get_egid(creds, egid); } -int SdBus::sd_bus_creds_get_supplementary_gids(sd_bus_creds *c, const gid_t **gids) +int SdBus::sd_bus_creds_get_supplementary_gids(sd_bus_creds *creds, const gid_t **gids) { std::lock_guard lock(sdbusMutex_); - return ::sd_bus_creds_get_supplementary_gids(c, gids); + return ::sd_bus_creds_get_supplementary_gids(creds, gids); } -int SdBus::sd_bus_creds_get_selinux_context(sd_bus_creds *c, const char **label) +int SdBus::sd_bus_creds_get_selinux_context(sd_bus_creds *creds, const char **label) { std::lock_guard lock(sdbusMutex_); - return ::sd_bus_creds_get_selinux_context(c, label); + return ::sd_bus_creds_get_selinux_context(creds, label); } -} +} // namespace sdbus::internal diff --git a/src/SdBus.h b/src/SdBus.h index 7b8127d9..7167de50 100644 --- a/src/SdBus.h +++ b/src/SdBus.h @@ -62,7 +62,7 @@ class SdBus final : public ISdBus virtual int sd_bus_open_system(sd_bus **ret) override; virtual int sd_bus_open_user(sd_bus **ret) override; virtual int sd_bus_open_user_with_address(sd_bus **ret, const char* address) override; - virtual int sd_bus_open_system_remote(sd_bus **ret, const char* hsot) override; + virtual int sd_bus_open_system_remote(sd_bus **ret, const char* host) override; virtual int sd_bus_open_direct(sd_bus **ret, const char* address) override; virtual int sd_bus_open_direct(sd_bus **ret, int fd) override; virtual int sd_bus_open_server(sd_bus **ret, int fd) override; @@ -79,7 +79,7 @@ class SdBus final : public ISdBus virtual int sd_bus_new(sd_bus **ret) override; virtual int sd_bus_start(sd_bus *bus) override; - virtual int sd_bus_process(sd_bus *bus, sd_bus_message **r) override; + virtual int sd_bus_process(sd_bus *bus, sd_bus_message **msg) override; virtual sd_bus_message* sd_bus_get_current_message(sd_bus *bus) override; virtual int sd_bus_get_poll_data(sd_bus *bus, PollData* data) override; virtual int sd_bus_get_n_queued(sd_bus *bus, uint64_t *read, uint64_t* write) override; diff --git a/tests/stresstests/sdbus-c++-stress-tests.cpp b/tests/stresstests/sdbus-c++-stress-tests.cpp index 66f82463..b95e76c3 100644 --- a/tests/stresstests/sdbus-c++-stress-tests.cpp +++ b/tests/stresstests/sdbus-c++-stress-tests.cpp @@ -62,13 +62,18 @@ class CelsiusThermometerAdaptor final : public sdbus::AdaptorInterfaces(celsiusProxy_.getCurrentTemperature() * 1.8 + 32.); } - virtual void createDelegateObject(sdbus::Result&& result) override + void createDelegateObject(sdbus::Result&& result) override { static size_t objectCounter{}; objectCounter++; @@ -180,7 +195,7 @@ class FahrenheitThermometerAdaptor final : public sdbus::AdaptorInterfaces< org: cond_.notify_one(); } - virtual void destroyDelegateObject(sdbus::Result<>&& /*result*/, sdbus::ObjectPath delegate) override + void destroyDelegateObject(sdbus::Result<>&& /*result*/, sdbus::ObjectPath delegate) override { std::unique_lock lock(mutex_); requests_.push(WorkItem{0, std::move(delegate), {}}); @@ -216,6 +231,11 @@ class FahrenheitThermometerProxy : public sdbus::ProxyInterfaces< org::sdbuscpp: registerProxy(); } + FahrenheitThermometerProxy(const FahrenheitThermometerProxy&) = delete; + FahrenheitThermometerProxy& operator=(const FahrenheitThermometerProxy&) = delete; + FahrenheitThermometerProxy(FahrenheitThermometerProxy&&) = delete; + FahrenheitThermometerProxy& operator=(FahrenheitThermometerProxy&&) = delete; + ~FahrenheitThermometerProxy() { unregisterProxy(); @@ -262,6 +282,11 @@ class ConcatenatorAdaptor final : public sdbus::AdaptorInterfaces&& result, std::map params) override + void concatenate(sdbus::Result&& result, std::map params) override { std::unique_lock lock(mutex_); requests_.push(WorkItem{std::move(params), std::move(result)}); @@ -303,13 +328,18 @@ class ConcatenatorProxy final : public sdbus::ProxyInterfaces error) override + void onConcatenateReply(const std::string& result, [[maybe_unused]] std::optional error) override { assert(error == std::nullopt); @@ -318,21 +348,21 @@ class ConcatenatorProxy final : public sdbus::ProxyInterfaces> aString; assert(aString == "sdbus-c++-stress-tests"); - uint32_t aNumber; + uint32_t aNumber{}; str >> aNumber; assert(aNumber > 0); ++repliesReceived_; } - virtual void onConcatenatedSignal(const std::string& concatenatedString) override + void onConcatenatedSignal(const std::string& concatenatedString) override { std::stringstream str(concatenatedString); std::string aString; str >> aString; assert(aString == "sdbus-c++-stress-tests"); - uint32_t aNumber; + uint32_t aNumber{}; str >> aNumber; assert(aNumber > 0); @@ -345,10 +375,10 @@ class ConcatenatorProxy final : public sdbus::ProxyInterfaces(concatenator.repliesReceived_); + concatenationSignalsReceived = static_cast(concatenator.signalsReceived_); } } });