PengZheng commented on code in PR #459:
URL: https://github.com/apache/celix/pull/459#discussion_r1059625028
##########
bundles/remote_services/discovery_common/src/discovery_activator.c:
##########
@@ -139,7 +137,10 @@ celix_status_t bundleActivator_start(void * userData,
celix_bundle_context_t *co
activator->endpointListener = endpointListener;
}
}
- }
+ } else {
Review Comment:
In error, not only `props` should be destroyed, but also the opened
`serviceTracker`. But this belongs to another PR.
Please take care of this when unifying service discovery. @xuzhenbao
##########
bundles/pubsub/pubsub_protocol/pubsub_protocol_lib/gtest/CMakeLists.txt:
##########
@@ -15,14 +15,12 @@
# specific language governing permissions and limitations
# under the License.
-set(SOURCES
- src/main.cc
- src/PS_WP_common_tests.cc
- )
-add_executable(celix_pswp_common_tests ${SOURCES})
-#target_include_directories(celix_cxx_pswp_tests SYSTEM PRIVATE gtest)
+add_executable(celix_pswp_common_tests src/PS_WP_common_tests.cc)
target_include_directories(celix_pswp_common_tests PRIVATE
${CMAKE_CURRENT_SOURCE_DIR}/../src)
-target_link_libraries(celix_pswp_common_tests PRIVATE
celix_pubsub_protocol_lib GTest::gtest Celix::pubsub_spi)
+target_link_libraries(celix_pswp_common_tests PRIVATE
celix_pubsub_protocol_lib GTest::gtest Celix::pubsub_spi GTest::gtest_main
${CMAKE_DL_LIBS})
+if (NOT ENABLE_ADDRESS_SANITIZER)
Review Comment:
Let's merge the current malloc mocking. I'll make another PR proposing a
general error injector playing nicely with address sanitizers.
##########
libs/framework/src/framework.c:
##########
@@ -326,7 +326,7 @@ celix_status_t framework_destroy(framework_pt framework) {
bundle_t *bnd = entry->bnd;
if (count > 0) {
const char *bndName = celix_bundle_getSymbolicName(bnd);
- fw_log(framework->logger, CELIX_LOG_LEVEL_FATAL, "Cannot destroy
framework. The use count of bundle %s (bnd id %li) is not 0, but %u.", bndName,
entry->bndId, count);
+ fw_log(framework->logger, CELIX_LOG_LEVEL_FATAL, "Cannot destroy
framework. The use count of bundle %s (bnd id %li) is not 0, but %lu.",
bndName, entry->bndId, count);
Review Comment:
```suggestion
fw_log(framework->logger, CELIX_LOG_LEVEL_FATAL, "Cannot destroy
framework. The use count of bundle %s (bnd id %li) is not 0, but %zu.",
bndName, entry->bndId, count);
```
We need to check all format specifiers for `size_t`. I guess this is not the
last place.
##########
libs/framework/src/framework.c:
##########
@@ -76,7 +76,7 @@ static inline void
fw_bundleEntry_waitTillUseCountIs(celix_framework_bundle_entr
if (entry->useCount != desiredUseCount) {
struct timespec now = celix_gettime(CLOCK_MONOTONIC);
if (celix_difftime(&start, &now) > 5) {
- fw_log(celix_frameworkLogger_globalLogger(),
CELIX_LOG_LEVEL_WARNING, "Bundle '%s' (bnd id = %li) still in use. Use count is
%u, desired is %li", celix_bundle_getSymbolicName(entry->bnd), entry->bndId,
entry->useCount, desiredUseCount);
+ fw_log(celix_frameworkLogger_globalLogger(),
CELIX_LOG_LEVEL_WARNING, "Bundle '%s' (bnd id = %li) still in use. Use count is
%lu, desired is %li", celix_bundle_getSymbolicName(entry->bnd), entry->bndId,
entry->useCount, desiredUseCount);
Review Comment:
```suggestion
fw_log(celix_frameworkLogger_globalLogger(),
CELIX_LOG_LEVEL_WARNING, "Bundle '%s' (bnd id = %li) still in use. Use count is
%zu, desired is %zu", celix_bundle_getSymbolicName(entry->bnd), entry->bndId,
entry->useCount, desiredUseCount);
```
`%zu` is the correct specifier for `size_t`, `%lu` will cause gcc warning on
some ARM platform.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@celix.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
