PengZheng commented on code in PR #517:
URL: https://github.com/apache/celix/pull/517#discussion_r1161399646
##########
bundles/pubsub/examples/pubsub/subscriber/private/src/ps_sub_activator.c:
##########
@@ -85,7 +86,7 @@ celix_status_t bundleActivator_stop(void * userData,
celix_bundle_context_t *con
struct subscriberActivator * act = (struct subscriberActivator *) userData;
int i;
- for (i = 0; i < arrayList_size(act->registrationList); i++) {
+ for (i = 0; i < celix_arrayList_size(act->registrationList); i++) {
service_registration_pt reg = arrayList_get(act->registrationList, i);
Review Comment:
```suggestion
service_registration_pt reg =
celix_arrayList_get(act->registrationList, i);
```
##########
bundles/remote_services/rsa_dfi_utils/src/dfi_utils.c:
##########
@@ -87,19 +80,17 @@ static celix_status_t dfi_findFileForBundle(const
celix_bundle_t *bundle, const
}
static celix_status_t dfi_findAvprFileForBundle(const celix_bundle_t *bundle,
const char* fileName, FILE **out) {
- celix_status_t status;
- char *path = NULL;
- status = bundle_getEntry(bundle, fileName, &path);
+ celix_status_t status = CELIX_SUCCESS;
+ char* path = celix_bundle_getEntry(bundle, fileName);
char metaInfFileName[512];
- if (status != CELIX_SUCCESS || path == NULL) {
- free(path);
+ if (path == NULL) {
snprintf(metaInfFileName, sizeof(metaInfFileName), "META-INF/avpr/%s",
fileName);
- status = bundle_getEntry(bundle, metaInfFileName, &path);
+ path = celix_bundle_getEntry(bundle, metaInfFileName);
}
- if (status == CELIX_SUCCESS && path != NULL) {
+ if (path != NULL) {
Review Comment:
`path == NULL` should be an error.
##########
bundles/remote_services/rsa_dfi_utils/src/dfi_utils.c:
##########
@@ -20,59 +20,52 @@
#include "dfi_utils.h"
#include <stdlib.h>
#include <unistd.h>
-#include "bundle_context.h"
+#include "celix_bundle_context.h"
static celix_status_t dfi_findFileForFramework(celix_bundle_context_t
*context, const char *fileName, FILE **out) {
- celix_status_t status;
+ celix_status_t status = CELIX_SUCCESS;
char pwd[1024];
char path[1024];
- const char *extPath = NULL;
-
- status = bundleContext_getProperty(context,
"CELIX_FRAMEWORK_EXTENDER_PATH", &extPath);
- if (status != CELIX_SUCCESS || extPath == NULL) {
+ const char *extPath = celix_bundleContext_getProperty(context,
"CELIX_FRAMEWORK_EXTENDER_PATH", NULL);
+ if (extPath == NULL) {
getcwd(pwd, sizeof(pwd));
extPath = pwd;
}
snprintf(path, sizeof(path), "%s/%s", extPath, fileName);
- if (status == CELIX_SUCCESS) {
- FILE *df = fopen(path, "r");
- if (df == NULL) {
- status = CELIX_FILE_IO_EXCEPTION;
- } else {
- *out = df;
- }
+ FILE *df = fopen(path, "r");
+ if (df == NULL) {
+ status = CELIX_FILE_IO_EXCEPTION;
+ } else {
+ *out = df;
}
return status;
}
static celix_status_t dfi_findFileForBundle(const celix_bundle_t *bundle,
const char *fileName, FILE **out) {
- celix_status_t status;
+ celix_status_t status = CELIX_SUCCESS;
//Checking if descriptor is in root dir of bundle
- char *path = NULL;
- status = bundle_getEntry(bundle, fileName, &path);
+ char *path = celix_bundle_getEntry(bundle, fileName);
char metaInfFileName[512];
- if (status != CELIX_SUCCESS || path == NULL) {
- free(path);
+ if (path == NULL) {
//Checking if descriptor is in META-INF/descriptors
snprintf(metaInfFileName, sizeof(metaInfFileName),
"META-INF/descriptors/%s", fileName);
- status = bundle_getEntry(bundle, metaInfFileName, &path);
+ path = celix_bundle_getEntry(bundle, metaInfFileName);
}
- if (status != CELIX_SUCCESS || path == NULL) {
- free(path);
+ if (path == NULL) {
//Checking if descriptor is in META-INF/descriptors/services
snprintf(metaInfFileName, sizeof(metaInfFileName),
"META-INF/descriptors/services/%s", fileName);
- status = bundle_getEntry(bundle, metaInfFileName, &path);
+ path = celix_bundle_getEntry(bundle, metaInfFileName);
}
- if (status == CELIX_SUCCESS && path != NULL) {
+ if (path != NULL) {
Review Comment:
What if path == NULL? Previously this is an error condition, but now it is
`CELIX_SUCCESS`.
##########
libs/framework/gtest/src/BundleArchiveWithErrorInjectionTestSuite.cc:
##########
@@ -26,9 +26,10 @@
#include "celix_utils_ei.h"
#include "asprintf_ei.h"
-//including private headers, which should only be used for testing
-#include "bundle_archive_private.h"
-#include "bundle_revision_private.h"
+//extern declarations for testing purposes. Note signatures are not correct,
but that is not important for the test.
+extern "C" celix_status_t bundleArchive_create(void);
+extern "C" celix_status_t manifest_create(void);
+extern "C" celix_status_t bundleRevision_create(void);
Review Comment:
OK, this is fair use. But it is not the case in `c_shell_activator.c`.
##########
bundles/shell/shell/src/c_shell_activator.c:
##########
@@ -22,13 +22,15 @@
#include <string.h>
#include "shell_private.h"
-#include "bundle_activator.h"
+#include "celix_bundle_activator.h"
#include "std_commands.h"
-#include "service_tracker.h"
#include "celix_constants.h"
#include "celix_shell_command.h"
#include "std_commands.h"
+//declaration of bundleArchive_create used for testing
+extern celix_status_t bundleArchive_create(celix_framework_t* fw, const char
*archiveRoot, long id, const char *location, bundle_archive_pt *bundle_archive);
Review Comment:
It seems unused in this translation unit, why do we need it?
##########
libs/framework/include_deprecated/framework_exports.h:
##########
Review Comment:
As mentioned in
https://github.com/apache/celix/issues/509#issuecomment-1495322259 and #442, we
should control symbol visibility for shared objects throughout the project. Any
plan for this?
##########
cmake/celix_project/CelixProject.cmake:
##########
@@ -103,9 +106,25 @@ celix_deprecated_utils_headers(<target_name>))
]]
function(celix_deprecated_utils_headers)
list(GET ARGN 0 TARGET_NAME)
- target_include_directories(${TARGET_NAME} PRIVATE
${CMAKE_SOURCE_DIR}/libs/utils/include_deprecated)
+ get_target_property(UTILS_SOURCE__DIR Celix::utils "SOURCE_DIR")
+ target_include_directories(${TARGET_NAME} PRIVATE
${UTILS_SOURCE__DIR}/include_deprecated)
set_property(TARGET celix-deprecated APPEND PROPERTY "UTIL_TARGETS"
"${TARGET_NAME}")
endfunction()
+
+#[[
+Add include path for the Celix framework deprecated headers to the provided
target (as PRIVATE)
+
+```CMake
+celix_deprecated_framework_headers(<target_name>))
+```
+]]
+function(celix_deprecated_framework_headers)
+ list(GET ARGN 0 TARGET_NAME)
+ get_target_property(UTILS_SOURCE__DIR Celix::framework "SOURCE_DIR")
Review Comment:
`FRAMEWORK_SOURCE__DIR` instead of `UTILS_SOURCE__DIR`?
--
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
