You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@celix.apache.org by pn...@apache.org on 2022/11/13 18:02:14 UTC
[celix] branch feature/some_coverity_fixes created (now 4b0d378a)
This is an automated email from the ASF dual-hosted git repository.
pnoltes pushed a change to branch feature/some_coverity_fixes
in repository https://gitbox.apache.org/repos/asf/celix.git
at 4b0d378a Fix several resource leaks and locking issues reported by coverity
This branch includes the following new commits:
new 4b0d378a Fix several resource leaks and locking issues reported by coverity
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[celix] 01/01: Fix several resource leaks and locking issues reported by coverity
Posted by pn...@apache.org.
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 <pe...@gmail.com>
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();