pnoltes commented on code in PR #751:
URL: https://github.com/apache/celix/pull/751#discussion_r1632273373
##########
libs/framework/src/framework.c:
##########
@@ -624,63 +648,62 @@ bool
celix_framework_getConfigPropertyAsBool(celix_framework_t* framework, const
static celix_status_t
celix_framework_installBundleInternalImpl(celix_framework_t* framework, const
char* bndLoc, long* bndId) {
- celix_status_t status = CELIX_SUCCESS;
- celix_bundle_t* bundle = NULL;
- long id = -1L;
-
- bundle_state_e state = CELIX_BUNDLE_STATE_UNKNOWN;
-
bool valid = celix_framework_utils_isBundleUrlValid(framework, bndLoc,
false);
if (!valid) {
return CELIX_FILE_IO_EXCEPTION;
}
- //increase use count of framework bundle to prevent a stop.
- celix_framework_bundle_entry_t *fwBundleEntry =
celix_framework_bundleEntry_getBundleEntryAndIncreaseUseCount(framework,
-
framework->bundleId);
- status = CELIX_DO_IF(status, bundle_getState(framework->bundle, &state));
- if (status == CELIX_SUCCESS) {
- if (state == CELIX_BUNDLE_STATE_STOPPING || state ==
CELIX_BUNDLE_STATE_UNINSTALLED) {
- fw_log(framework->logger, CELIX_LOG_LEVEL_INFO, "The framework is
being shutdown");
- status = CELIX_FRAMEWORK_SHUTDOWN;
- }
+ // increase use count of framework bundle to prevent a stop.
+ celix_framework_bundle_entry_t* fwBundleEntry =
+
celix_framework_bundleEntry_getBundleEntryAndIncreaseUseCount(framework,
framework->bundleId);
+
+ celix_bundle_state_e bndState = celix_bundle_getState(framework->bundle);
+ if (bndState == CELIX_BUNDLE_STATE_STOPPING || bndState ==
CELIX_BUNDLE_STATE_UNINSTALLED) {
+ fw_log(framework->logger, CELIX_LOG_LEVEL_INFO, "The framework is
being shutdown");
+ return CELIX_FRAMEWORK_SHUTDOWN;
}
- if (status == CELIX_SUCCESS) {
- if (*bndId == -1L) {
- id = framework_getBundle(framework, bndLoc);
- if (id != -1L) {
- celix_framework_bundleEntry_decreaseUseCount(fwBundleEntry);
- *bndId = id;
- return CELIX_SUCCESS;
- }
- long alreadyExistingBndId =
celix_bundleCache_findBundleIdForLocation(framework->cache, bndLoc);
- id = alreadyExistingBndId == -1 ?
framework_getNextBundleId(framework) : alreadyExistingBndId;
- } else {
- id = *bndId;
+ long id;
+ if (*bndId == -1L) {
+ id = framework_getBundle(framework, bndLoc);
+ if (id != -1L) {
+ celix_framework_bundleEntry_decreaseUseCount(fwBundleEntry);
+ *bndId = id;
+ return CELIX_SUCCESS;
}
- bundle_archive_t* archive = NULL;
- status = CELIX_DO_IF(status,
celix_bundleCache_createArchive(framework->cache, id, bndLoc, &archive));
- status = CELIX_DO_IF(status, celix_bundle_createFromArchive(framework,
archive, &bundle));
- if (status == CELIX_SUCCESS) {
- celix_framework_bundle_entry_t *bEntry =
fw_bundleEntry_create(bundle);
- celix_framework_bundleEntry_increaseUseCount(bEntry);
- celixThreadMutex_lock(&framework->installedBundles.mutex);
- celix_arrayList_add(framework->installedBundles.entries, bEntry);
- celixThreadMutex_unlock(&framework->installedBundles.mutex);
- fw_fireBundleEvent(framework,
OSGI_FRAMEWORK_BUNDLE_EVENT_INSTALLED, bEntry);
- celix_framework_bundleEntry_decreaseUseCount(bEntry);
+ long alreadyExistingBndId;
+ celix_status_t status =
+ celix_bundleCache_findBundleIdForLocation(framework->cache,
bndLoc, &alreadyExistingBndId);
+ if (status != CELIX_SUCCESS) {
+ celix_framework_bundleEntry_decreaseUseCount(fwBundleEntry);
+ fw_logCode(framework->logger, CELIX_LOG_LEVEL_ERROR, status,
"Could not install bundle");
Review Comment:
Improve messaged, note that bundle archive already logs a message that a
bundle archive cannot be created.
##########
libs/framework/src/framework.c:
##########
@@ -624,63 +648,62 @@ bool
celix_framework_getConfigPropertyAsBool(celix_framework_t* framework, const
static celix_status_t
celix_framework_installBundleInternalImpl(celix_framework_t* framework, const
char* bndLoc, long* bndId) {
- celix_status_t status = CELIX_SUCCESS;
- celix_bundle_t* bundle = NULL;
- long id = -1L;
-
- bundle_state_e state = CELIX_BUNDLE_STATE_UNKNOWN;
-
bool valid = celix_framework_utils_isBundleUrlValid(framework, bndLoc,
false);
if (!valid) {
return CELIX_FILE_IO_EXCEPTION;
}
- //increase use count of framework bundle to prevent a stop.
- celix_framework_bundle_entry_t *fwBundleEntry =
celix_framework_bundleEntry_getBundleEntryAndIncreaseUseCount(framework,
-
framework->bundleId);
- status = CELIX_DO_IF(status, bundle_getState(framework->bundle, &state));
- if (status == CELIX_SUCCESS) {
- if (state == CELIX_BUNDLE_STATE_STOPPING || state ==
CELIX_BUNDLE_STATE_UNINSTALLED) {
- fw_log(framework->logger, CELIX_LOG_LEVEL_INFO, "The framework is
being shutdown");
- status = CELIX_FRAMEWORK_SHUTDOWN;
- }
+ // increase use count of framework bundle to prevent a stop.
+ celix_framework_bundle_entry_t* fwBundleEntry =
+
celix_framework_bundleEntry_getBundleEntryAndIncreaseUseCount(framework,
framework->bundleId);
+
+ celix_bundle_state_e bndState = celix_bundle_getState(framework->bundle);
+ if (bndState == CELIX_BUNDLE_STATE_STOPPING || bndState ==
CELIX_BUNDLE_STATE_UNINSTALLED) {
+ fw_log(framework->logger, CELIX_LOG_LEVEL_INFO, "The framework is
being shutdown");
+ return CELIX_FRAMEWORK_SHUTDOWN;
}
- if (status == CELIX_SUCCESS) {
- if (*bndId == -1L) {
- id = framework_getBundle(framework, bndLoc);
- if (id != -1L) {
- celix_framework_bundleEntry_decreaseUseCount(fwBundleEntry);
- *bndId = id;
- return CELIX_SUCCESS;
- }
- long alreadyExistingBndId =
celix_bundleCache_findBundleIdForLocation(framework->cache, bndLoc);
- id = alreadyExistingBndId == -1 ?
framework_getNextBundleId(framework) : alreadyExistingBndId;
- } else {
- id = *bndId;
+ long id;
+ if (*bndId == -1L) {
+ id = framework_getBundle(framework, bndLoc);
+ if (id != -1L) {
+ celix_framework_bundleEntry_decreaseUseCount(fwBundleEntry);
+ *bndId = id;
+ return CELIX_SUCCESS;
}
- bundle_archive_t* archive = NULL;
- status = CELIX_DO_IF(status,
celix_bundleCache_createArchive(framework->cache, id, bndLoc, &archive));
- status = CELIX_DO_IF(status, celix_bundle_createFromArchive(framework,
archive, &bundle));
- if (status == CELIX_SUCCESS) {
- celix_framework_bundle_entry_t *bEntry =
fw_bundleEntry_create(bundle);
- celix_framework_bundleEntry_increaseUseCount(bEntry);
- celixThreadMutex_lock(&framework->installedBundles.mutex);
- celix_arrayList_add(framework->installedBundles.entries, bEntry);
- celixThreadMutex_unlock(&framework->installedBundles.mutex);
- fw_fireBundleEvent(framework,
OSGI_FRAMEWORK_BUNDLE_EVENT_INSTALLED, bEntry);
- celix_framework_bundleEntry_decreaseUseCount(bEntry);
+ long alreadyExistingBndId;
+ celix_status_t status =
+ celix_bundleCache_findBundleIdForLocation(framework->cache,
bndLoc, &alreadyExistingBndId);
+ if (status != CELIX_SUCCESS) {
+ celix_framework_bundleEntry_decreaseUseCount(fwBundleEntry);
+ fw_logCode(framework->logger, CELIX_LOG_LEVEL_ERROR, status,
"Could not install bundle");
Review Comment:
Improved message, note that bundle archive already logs a message that a
bundle archive cannot be created.
--
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
