Skip to content
Permalink
Browse files
Use gcc built-in atomic operations without introducing C11 and fix ot…
…her code review issues.
  • Loading branch information
PengZheng committed Jan 24, 2022
1 parent b64eaa7 commit 3b65e359f0b1a0ab497539d97b5e2eae37890d2e
Show file tree
Hide file tree
Showing 8 changed files with 76 additions and 49 deletions.
@@ -44,7 +44,7 @@ endif ()
set(ENABLE_MORE_WARNINGS OFF)

# Set C specific flags
set(CMAKE_C_FLAGS "-D_GNU_SOURCE -std=gnu11 -fPIC ${CMAKE_C_FLAGS}")
set(CMAKE_C_FLAGS "-D_GNU_SOURCE -std=gnu99 -fPIC ${CMAKE_C_FLAGS}")
set(CMAKE_C_FLAGS "-Wall -Werror ${CMAKE_C_FLAGS}")

# Set C++ specific flags
@@ -76,41 +76,36 @@ celix_status_t bundleContext_create(framework_pt framework, celix_framework_logg

}
}
//FIXME: context == NULL?
framework_logIfError(context->framework->logger, status, NULL, "Failed to create context");
framework_logIfError(logger, status, NULL, "Failed to create context");

return status;
}

celix_status_t bundleContext_destroy(bundle_context_pt context) {
celix_status_t status = CELIX_SUCCESS;

if (context != NULL) {
assert(hashMap_size(context->bundleTrackers) == 0);
hashMap_destroy(context->bundleTrackers, false, false);
assert(hashMap_size(context->serviceTrackers) == 0);
hashMap_destroy(context->serviceTrackers, false, false);
assert(hashMap_size(context->metaTrackers) == 0);
hashMap_destroy(context->metaTrackers, false, false);
assert(celix_arrayList_size(context->svcRegistrations) == 0);
celix_arrayList_destroy(context->svcRegistrations);
hashMap_destroy(context->stoppingTrackerEventIds, false, false);

celixThreadMutex_destroy(&context->mutex);

if (context->mng != NULL) {
celix_dependencyManager_removeAllComponents(context->mng);
celix_private_dependencyManager_destroy(context->mng);
context->mng = NULL;
}
if(context == NULL) {
return CELIX_ILLEGAL_ARGUMENT;
}
assert(hashMap_size(context->bundleTrackers) == 0);
hashMap_destroy(context->bundleTrackers, false, false);
assert(hashMap_size(context->serviceTrackers) == 0);
hashMap_destroy(context->serviceTrackers, false, false);
assert(hashMap_size(context->metaTrackers) == 0);
hashMap_destroy(context->metaTrackers, false, false);
assert(celix_arrayList_size(context->svcRegistrations) == 0);
celix_arrayList_destroy(context->svcRegistrations);
hashMap_destroy(context->stoppingTrackerEventIds, false, false);

free(context);
} else {
status = CELIX_ILLEGAL_ARGUMENT;
}
//FIXME: context == NULL?
framework_logIfError(context->framework->logger, status, NULL, "Failed to destroy context");
celixThreadMutex_destroy(&context->mutex);

if (context->mng != NULL) {
celix_dependencyManager_removeAllComponents(context->mng);
celix_private_dependencyManager_destroy(context->mng);
context->mng = NULL;
}

free(context);
return status;
}

@@ -232,8 +227,14 @@ celix_status_t bundleContext_getServiceReference(bundle_context_pt context, cons

if (serviceName != NULL) {
if (bundleContext_getServiceReferences(context, serviceName, NULL, &services) == CELIX_SUCCESS) {
reference = (arrayList_size(services) > 0) ? arrayList_get(services, 0) : NULL;
//FIXME: unget service reference
unsigned int size = arrayList_size(services);
for(unsigned int i = 0; i < size; i++) {
if(i == 0) {
reference = arrayList_get(services, 0);
} else {
bundleContext_ungetServiceReference(context, arrayList_get(services, i));
}
}
arrayList_destroy(services);
*service_reference = reference;
} else {
@@ -84,7 +84,7 @@ celix_status_t serviceReference_release(service_reference_pt ref, bool *out) {

static void serviceReference_doRelease(struct celix_ref *refCount) {
service_reference_pt ref = (service_reference_pt)refCount;
BUILD_ASSERT(offsetof(struct serviceReference, refCount) == 0);
CELIX_BUILD_ASSERT(offsetof(struct serviceReference, refCount) == 0);
if(ref->registration != NULL) {
serviceRegistration_release(ref->registration);
}
@@ -137,11 +137,8 @@ celix_status_t serviceReference_getUsageCount(service_reference_pt ref, size_t *
}

celix_status_t serviceReference_getReferenceCount(service_reference_pt ref, size_t *count) {
celix_status_t status = CELIX_SUCCESS;
celixThreadRwlock_readLock(&ref->lock);
*count = ref->refCount.count;
celixThreadRwlock_unlock(&ref->lock);
return status;
*count = celix_ref_read(&ref->refCount);
return CELIX_SUCCESS;
}

celix_status_t serviceReference_getService(service_reference_pt ref, const void **service) {
@@ -84,7 +84,7 @@ void serviceRegistration_release(service_registration_pt registration) {

static void serviceRegistration_destroy(struct celix_ref *refCount) {
service_registration_pt registration = (service_registration_pt )refCount;
BUILD_ASSERT(offsetof(service_registration_t, refCount) == 0);
CELIX_BUILD_ASSERT(offsetof(service_registration_t, refCount) == 0);

//fw_log(logger, CELIX_LOG_LEVEL_DEBUG, "Destroying service registration %p\n", registration);
free(registration->className);
@@ -94,9 +94,10 @@ celix_status_t serviceTracker_create(bundle_context_pt context, const char * ser
if (service == NULL || *tracker != NULL) {
status = CELIX_ILLEGAL_ARGUMENT;
} else {
char filter[512];
snprintf(filter, sizeof(filter), "(%s=%s)", OSGI_FRAMEWORK_OBJECTCLASS, service);
char *filter = NULL;
asprintf(&filter, "(%s=%s)", OSGI_FRAMEWORK_OBJECTCLASS, service);
serviceTracker_createWithFilter(context, filter, customizer, tracker);
free(filter);
}

framework_logIfError(context->framework->logger, status, NULL, "Cannot create service tracker");
@@ -340,8 +341,9 @@ static void serviceTracker_serviceChanged(void *handle, celix_service_event_t *e
switch (event->type) {
case OSGI_FRAMEWORK_SERVICE_EVENT_REGISTERED:
case OSGI_FRAMEWORK_SERVICE_EVENT_MODIFIED:
if(!closing)
if(!closing) {
serviceTracker_track(tracker, event->reference, event);
}
break;
case OSGI_FRAMEWORK_SERVICE_EVENT_UNREGISTERING:
//after this call the registration can be gone, to prevent that happens before the tracker finishing its cleanup job with the corresponding service,
@@ -17,10 +17,10 @@
* under the License.
*/

#ifndef CELIX_CELIX_BUILD_ASSERT_H
#define CELIX_CELIX_BUILD_ASSERT_H
#ifndef CELIX_BUILD_ASSERT_H
#define CELIX_BUILD_ASSERT_H

#define BUILD_ASSERT(cond) \
#define CELIX_BUILD_ASSERT(cond) \
do { (void) sizeof(char [1 - 2*!(cond)]); } while(0)

#endif //CELIX_CELIX_BUILD_ASSERT_H
#endif //CELIX_BUILD_ASSERT_H
@@ -20,30 +20,57 @@
#ifndef CELIX_CELIX_REF_H
#define CELIX_CELIX_REF_H

#include <stdatomic.h>
#include <limits.h>
#include <assert.h>

struct celix_ref {
atomic_int count;
int count;
};

/**
* @brief Initialize object.
* @param ref object in question.
*/
static inline void celix_ref_init(struct celix_ref *ref) {
ref->count = 1;
}

/**
* @brief Read reference count for object.
* @param ref object.
* @return the current reference count.
*/
static inline int celix_ref_read(const struct celix_ref *ref)
{
return __atomic_load_n(&ref->count, __ATOMIC_RELAXED);
}

/**
* @brief Increase reference count for object.
* @param ref object.
*/
static inline void celix_ref_get(struct celix_ref *ref) {
int val;
val = atomic_fetch_add_explicit(&(ref->count), 1, memory_order_relaxed);
val = __atomic_fetch_add(&(ref->count), 1, __ATOMIC_RELAXED);
assert(val < INT_MAX && val > 0);
(void)val;
}

/**
* @brief Decrease reference count for object.
*
* Decrement the reference count, and if 0, call release().
*
* @param ref object.
* @param release pointer to the function that will clean up the object when the
* last reference to the object is released.
* @return 1 if the object was removed, otherwise return 0.
*/
static inline int celix_ref_put(struct celix_ref *ref, void (*release)(struct celix_ref *ref)) {
int val;
val = atomic_fetch_sub_explicit(&(ref->count), 1, memory_order_release);
val = __atomic_fetch_sub(&(ref->count), 1, __ATOMIC_RELEASE);
if(val == 1) {
atomic_thread_fence(memory_order_acquire);
__atomic_thread_fence(__ATOMIC_ACQUIRE);
release(ref);
return 1;
}
@@ -61,7 +61,7 @@ static celix_status_t arrayList_elementEquals(const void *a, const void *b, bool
}

static bool celix_arrayList_defaultEquals(celix_array_list_entry_t a, celix_array_list_entry_t b) {
BUILD_ASSERT(sizeof(a.voidPtrVal) == sizeof(a));
CELIX_BUILD_ASSERT(sizeof(a.voidPtrVal) == sizeof(a));
return a.voidPtrVal== b.voidPtrVal;
}

0 comments on commit 3b65e35

Please sign in to comment.