This is an automated email from the ASF dual-hosted git repository. pnoltes pushed a commit to branch feature/some_coverity_fixes in repository https://gitbox.apache.org/repos/asf/celix.git
commit 4b0d378a16404a26d94bf234a28d568049d9909e Author: Pepijn Noltes <[email protected]> AuthorDate: Sun Nov 13 18:58:29 2022 +0100 Fix several resource leaks and locking issues reported by coverity Also fix an "unsigned compared against 0" issue and some auto loop performance issues. --- README.md | 2 +- bundles/pubsub/pubsub_admin_tcp/src/pubsub_tcp_handler.c | 1 + bundles/remote_services/rsa_common/src/endpoint_description.c | 2 +- bundles/shell/shell/api/celix_shell_constants.h | 4 +++- bundles/shell/shell/src/Shell.cc | 8 ++++---- bundles/shell/shell/src/lb_command.c | 5 +---- bundles/shell/shell/src/query_command.c | 5 +---- .../readme_cxx_examples/src/CalcTrackerBundleActivator.cc | 2 +- libs/framework/include/celix/Trackers.h | 3 ++- libs/framework/src/celix_framework_utils.c | 3 +++ libs/pushstreams/api/celix/impl/AbstractPushEventSource.h | 2 +- 11 files changed, 19 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index 46e42d15..d4d8853b 100644 --- a/README.md +++ b/README.md @@ -247,7 +247,7 @@ public: explicit CalcTrackerBundleActivator(const std::shared_ptr<celix::BundleContext>& ctx) { tracker = ctx->trackServices<ICalc>() .build(); - for (auto calc : tracker->getServices()) { + for (auto& calc : tracker->getServices()) { std::cout << "result is " << std::to_string(calc->add(2, 3)) << std::endl; } } diff --git a/bundles/pubsub/pubsub_admin_tcp/src/pubsub_tcp_handler.c b/bundles/pubsub/pubsub_admin_tcp/src/pubsub_tcp_handler.c index eb02f79d..bdf35ba7 100644 --- a/bundles/pubsub/pubsub_admin_tcp/src/pubsub_tcp_handler.c +++ b/bundles/pubsub/pubsub_admin_tcp/src/pubsub_tcp_handler.c @@ -607,6 +607,7 @@ int pubsub_tcpHandler_listen(pubsub_tcpHandler_t *handle, char *url) { } celixThreadRwlock_unlock(&handle->dbLock); } else { + free(pUrl); L_ERROR("[TCP Socket] Error listen socket cannot bind to %s: %s\n", url ? url : "", strerror(errno)); } } diff --git a/bundles/remote_services/rsa_common/src/endpoint_description.c b/bundles/remote_services/rsa_common/src/endpoint_description.c index ec65ad8d..896160f2 100644 --- a/bundles/remote_services/rsa_common/src/endpoint_description.c +++ b/bundles/remote_services/rsa_common/src/endpoint_description.c @@ -92,7 +92,7 @@ static celix_status_t endpointDescription_verifyLongProperty(celix_properties_t } bool endpointDescription_isInvalid(const endpoint_description_t *description) { - return description == NULL || description->properties == NULL || description->serviceId < 0 + return description == NULL || description->properties == NULL || description->service == NULL || strlen(description->service) > NAME_MAX || description->frameworkUUID == NULL || description->id == NULL; } diff --git a/bundles/shell/shell/api/celix_shell_constants.h b/bundles/shell/shell/api/celix_shell_constants.h index 26d75817..872afd53 100644 --- a/bundles/shell/shell/api/celix_shell_constants.h +++ b/bundles/shell/shell/api/celix_shell_constants.h @@ -20,7 +20,9 @@ #ifndef CELIX_SHELL_CONSTANTS_H_ #define CELIX_SHELL_CONSTANTS_H_ +#include <stdbool.h> + #define CELIX_SHELL_USE_ANSI_COLORS "SHELL_USE_ANSI_COLORS" -#define CELIX_SHELL_USE_ANSI_COLORS_DEFAULT_VALUE "true" +#define CELIX_SHELL_USE_ANSI_COLORS_DEFAULT_VALUE true #endif /* CELIX_SHELL_CONSTANTS_H_ */ diff --git a/bundles/shell/shell/src/Shell.cc b/bundles/shell/shell/src/Shell.cc index cd531099..b6df4e19 100644 --- a/bundles/shell/shell/src/Shell.cc +++ b/bundles/shell/shell/src/Shell.cc @@ -108,19 +108,19 @@ namespace celix { } void addCCommand(const std::shared_ptr<celix_shell_command>& command, const std::shared_ptr<const celix::Properties>& properties) { - addEntry(command, nullptr, properties); + addEntry(command, {}, properties); } void remCCommand(const std::shared_ptr<celix_shell_command>& command, const std::shared_ptr<const celix::Properties>& properties) { - remEntry(command, nullptr, properties); + remEntry(command, {}, properties); } void addCxxCommand(const std::shared_ptr<celix::IShellCommand>& command, const std::shared_ptr<const celix::Properties>& properties) { - addEntry(nullptr, command, properties); + addEntry({}, command, properties); } void remCxxCommand(const std::shared_ptr<celix::IShellCommand>& command, const std::shared_ptr<const celix::Properties>& properties) { - remEntry(nullptr, command, properties); + remEntry({}, command, properties); } void setLogService(const std::shared_ptr<celix_log_service> ls) { diff --git a/bundles/shell/shell/src/lb_command.c b/bundles/shell/shell/src/lb_command.c index 12e889e1..dd1fd4eb 100644 --- a/bundles/shell/shell/src/lb_command.c +++ b/bundles/shell/shell/src/lb_command.c @@ -188,10 +188,7 @@ bool lbCommand_execute(void *handle, const char *const_command_line_str, FILE *o lb_options_t opts; memset(&opts, 0, sizeof(opts)); - const char* config = celix_bundleContext_getProperty(ctx, CELIX_SHELL_USE_ANSI_COLORS, CELIX_SHELL_USE_ANSI_COLORS_DEFAULT_VALUE); - opts.useColors = config != NULL && strncmp("true", config, 5) == 0; - - + opts.useColors = celix_bundleContext_getPropertyAsBool(ctx, CELIX_SHELL_USE_ANSI_COLORS, CELIX_SHELL_USE_ANSI_COLORS_DEFAULT_VALUE); opts.show_location = false; opts.show_symbolic_name = false; opts.show_update_location = false; diff --git a/bundles/shell/shell/src/query_command.c b/bundles/shell/shell/src/query_command.c index f4f1603a..739d3450 100644 --- a/bundles/shell/shell/src/query_command.c +++ b/bundles/shell/shell/src/query_command.c @@ -184,10 +184,7 @@ bool queryCommand_execute(void *_ptr, const char *command_line_str, FILE *sout, opts.nameQueries = celix_arrayList_create(); opts.filterQueries = celix_arrayList_create(); - - const char* config = celix_bundleContext_getProperty(ctx, CELIX_SHELL_USE_ANSI_COLORS, CELIX_SHELL_USE_ANSI_COLORS_DEFAULT_VALUE); - opts.useColors = config != NULL && strncmp("true", config, 5) == 0; - + opts.useColors = celix_bundleContext_getPropertyAsBool(ctx, CELIX_SHELL_USE_ANSI_COLORS, CELIX_SHELL_USE_ANSI_COLORS_DEFAULT_VALUE); bool validCommand = true; char *sub_str = NULL; diff --git a/examples/celix-examples/readme_cxx_examples/src/CalcTrackerBundleActivator.cc b/examples/celix-examples/readme_cxx_examples/src/CalcTrackerBundleActivator.cc index 8faf2ae4..18756843 100644 --- a/examples/celix-examples/readme_cxx_examples/src/CalcTrackerBundleActivator.cc +++ b/examples/celix-examples/readme_cxx_examples/src/CalcTrackerBundleActivator.cc @@ -26,7 +26,7 @@ public: explicit CalcTrackerBundleActivator(const std::shared_ptr<celix::BundleContext>& ctx) { tracker = ctx->trackServices<ICalc>() .build(); - for (auto calc : tracker->getServices()) { + for (auto& calc : tracker->getServices()) { std::cout << "result is " << std::to_string(calc->add(2, 3)) << std::endl; } } diff --git a/libs/framework/include/celix/Trackers.h b/libs/framework/include/celix/Trackers.h index 4a2b491e..6ad67fcb 100644 --- a/libs/framework/include/celix/Trackers.h +++ b/libs/framework/include/celix/Trackers.h @@ -430,7 +430,7 @@ namespace celix { }; opts.setWithOwner = [](void *handle, void *voidSvc, const celix_properties_t *cProps, const celix_bundle_t *cBnd) { auto tracker = static_cast<ServiceTracker<I>*>(handle); - std::lock_guard<std::mutex> lck{tracker->mutex}; + std::unique_lock<std::mutex> lck{tracker->mutex}; auto prevEntry = tracker->highestRankingServiceEntry; if (voidSvc) { tracker->highestRankingServiceEntry = createEntry(voidSvc, cProps, cBnd); @@ -445,6 +445,7 @@ namespace celix { cb(nullptr, nullptr, nullptr); } } + lck.unlock(); tracker->waitForExpiredSvcEntry(prevEntry); }; } diff --git a/libs/framework/src/celix_framework_utils.c b/libs/framework/src/celix_framework_utils.c index f8a2c2f6..055abf63 100644 --- a/libs/framework/src/celix_framework_utils.c +++ b/libs/framework/src/celix_framework_utils.c @@ -108,6 +108,7 @@ static bool isEmbeddedBundleUrlValid(celix_framework_t *fw, const char* bundleUR void* main = dlopen(NULL, RTLD_NOW); void* start = dlsym(main, startSymbol); void* end = dlsym(main, endSymbol); + dlclose(main); if (start == NULL || end == NULL) { valid = false; @@ -150,6 +151,7 @@ static bool extractBundleEmbedded(celix_framework_t *fw, const char* embeddedBun void* main = dlopen(NULL, RTLD_NOW); void* start = dlsym(main, startSymbol); void* end = dlsym(main, endSymbol); + dlclose(main); if (start == NULL || end == NULL) { FW_LOG(CELIX_LOG_LEVEL_ERROR, "Cannot extract embedded bundle, could not find symbols `%s` and/or `%s` for embedded bundle `%s`", startSymbol, endSymbol, embeddedBundle); @@ -235,6 +237,7 @@ celix_array_list_t* celix_framework_utils_listEmbeddedBundles() { } free(bundles); } + dlclose(main); return list; } diff --git a/libs/pushstreams/api/celix/impl/AbstractPushEventSource.h b/libs/pushstreams/api/celix/impl/AbstractPushEventSource.h index 96b8b96a..71ed518d 100644 --- a/libs/pushstreams/api/celix/impl/AbstractPushEventSource.h +++ b/libs/pushstreams/api/celix/impl/AbstractPushEventSource.h @@ -93,7 +93,7 @@ void celix::AbstractPushEventSource<T>::open(std::shared_ptr<celix::IPushEventCo _eventConsumer->accept(celix::ClosePushEvent<T>()); } else { eventConsumers.push_back(_eventConsumer); - for(auto connect: connected) { + for(auto& connect: connected) { connect.resolve(); } connected.clear();
