pnoltes commented on code in PR #591:
URL: https://github.com/apache/celix/pull/591#discussion_r1278588202
##########
libs/framework/include/celix_bundle_context.h:
##########
@@ -273,6 +274,18 @@ CELIX_FRAMEWORK_EXPORT bool
celix_bundleContext_isServiceRegistered(celix_bundle
*/
CELIX_FRAMEWORK_EXPORT void
celix_bundleContext_unregisterService(celix_bundle_context_t *ctx, long
serviceId);
+typedef struct celix_service_reg {
+ celix_bundle_context_t* ctx;
+ long svcId;
+} celix_service_reg_t;
Review Comment:
Maybe this should be named `celix_service_registration_guard_t` to emphasize
that this is a cleanup guard.
##########
libs/utils/include/celix_threads.h:
##########
@@ -90,34 +95,155 @@ CELIX_UTILS_EXPORT celix_status_t
celixThreadMutex_create(celix_thread_mutex_t *
CELIX_UTILS_EXPORT celix_status_t
celixThreadMutex_destroy(celix_thread_mutex_t *mutex);
+CELIX_DEFINE_AUTOPTR_CLEANUP_FUNC(celix_thread_mutex_t,
celixThreadMutex_destroy)
+
CELIX_UTILS_EXPORT celix_status_t celixThreadMutex_lock(celix_thread_mutex_t
*mutex);
+CELIX_UTILS_EXPORT celix_status_t
celixThreadMutex_tryLock(celix_thread_mutex_t *mutex);
+
CELIX_UTILS_EXPORT celix_status_t celixThreadMutex_unlock(celix_thread_mutex_t
*mutex);
+/**
+ * Opaque type. See celix_mutex_locker_new() for details.
+ */
+typedef void celix_mutex_locker_t;
Review Comment:
Maybe this should be called `celix_mutex_lock_guard_t` to emphasize that it
provides the same functionality as a C++ lock guard.
##########
libs/utils/include/celix_threads.h:
##########
@@ -90,34 +95,155 @@ CELIX_UTILS_EXPORT celix_status_t
celixThreadMutex_create(celix_thread_mutex_t *
CELIX_UTILS_EXPORT celix_status_t
celixThreadMutex_destroy(celix_thread_mutex_t *mutex);
+CELIX_DEFINE_AUTOPTR_CLEANUP_FUNC(celix_thread_mutex_t,
celixThreadMutex_destroy)
+
CELIX_UTILS_EXPORT celix_status_t celixThreadMutex_lock(celix_thread_mutex_t
*mutex);
+CELIX_UTILS_EXPORT celix_status_t
celixThreadMutex_tryLock(celix_thread_mutex_t *mutex);
+
CELIX_UTILS_EXPORT celix_status_t celixThreadMutex_unlock(celix_thread_mutex_t
*mutex);
+/**
+ * Opaque type. See celix_mutex_locker_new() for details.
+ */
+typedef void celix_mutex_locker_t;
+
+/**
+ * @brief Lock @a mutex and return a new celix_mutex_locker_t.
+ *
+ * Unlock with celixThreadMutexLocker_free(). Using celixThreadMutex_lock() on
@a mutex
+ * while a celix_mutex_locker_t exists can lead to undefined behaviour.
+ *
+ * No allocation is performed, it is equivalent to a celixThreadMutex_lock()
call.
+ * This is intended to be used with celix_autoptr().
+ *
+ * @param mutex A mutex to lock
+ * @return A new locker to be used with celix_autoptr().
+ */
+static inline celix_mutex_locker_t*
celixThreadMutexLocker_new(celix_thread_mutex_t* mutex) {
Review Comment:
I do not like the `_new` name. IMO new implies memory allocation, which is
not the case here.
##########
libs/utils/include/celix_threads.h:
##########
@@ -90,34 +95,155 @@ CELIX_UTILS_EXPORT celix_status_t
celixThreadMutex_create(celix_thread_mutex_t *
CELIX_UTILS_EXPORT celix_status_t
celixThreadMutex_destroy(celix_thread_mutex_t *mutex);
+CELIX_DEFINE_AUTOPTR_CLEANUP_FUNC(celix_thread_mutex_t,
celixThreadMutex_destroy)
+
CELIX_UTILS_EXPORT celix_status_t celixThreadMutex_lock(celix_thread_mutex_t
*mutex);
+CELIX_UTILS_EXPORT celix_status_t
celixThreadMutex_tryLock(celix_thread_mutex_t *mutex);
+
CELIX_UTILS_EXPORT celix_status_t celixThreadMutex_unlock(celix_thread_mutex_t
*mutex);
+/**
+ * Opaque type. See celix_mutex_locker_new() for details.
+ */
+typedef void celix_mutex_locker_t;
+
+/**
+ * @brief Lock @a mutex and return a new celix_mutex_locker_t.
+ *
+ * Unlock with celixThreadMutexLocker_free(). Using celixThreadMutex_lock() on
@a mutex
+ * while a celix_mutex_locker_t exists can lead to undefined behaviour.
+ *
+ * No allocation is performed, it is equivalent to a celixThreadMutex_lock()
call.
+ * This is intended to be used with celix_autoptr().
+ *
+ * @param mutex A mutex to lock
+ * @return A new locker to be used with celix_autoptr().
+ */
+static inline celix_mutex_locker_t*
celixThreadMutexLocker_new(celix_thread_mutex_t* mutex) {
+ celixThreadMutex_lock(mutex);
+ return (celix_mutex_locker_t*)mutex;
+}
+
+/**
+ * @brief Unlock the mutex of @a locker.
+ *
+ * See celixThreadMutexLocker_new() for details.
+ * No memory is freed, it is equivalent to a celixThreadMutex_unlock() call.
+ *
+ * @param locker A celix_mutex_locker_t
+ */
+static inline void celixThreadMutexLocker_free(celix_mutex_locker_t* locker) {
Review Comment:
I do not like the `_free` name. IMO new implies memory de-allocation, which
is not the case here.
##########
libs/framework/include/celix_bundle_context.h:
##########
@@ -273,6 +274,18 @@ CELIX_FRAMEWORK_EXPORT bool
celix_bundleContext_isServiceRegistered(celix_bundle
*/
CELIX_FRAMEWORK_EXPORT void
celix_bundleContext_unregisterService(celix_bundle_context_t *ctx, long
serviceId);
+typedef struct celix_service_reg {
Review Comment:
missing doxygen
##########
libs/framework/include/celix_bundle_context.h:
##########
@@ -273,6 +274,18 @@ CELIX_FRAMEWORK_EXPORT bool
celix_bundleContext_isServiceRegistered(celix_bundle
*/
CELIX_FRAMEWORK_EXPORT void
celix_bundleContext_unregisterService(celix_bundle_context_t *ctx, long
serviceId);
+typedef struct celix_service_reg {
+ celix_bundle_context_t* ctx;
+ long svcId;
+} celix_service_reg_t;
+
+static __attribute__((__unused__)) inline void
celix_service_unregister(celix_service_reg_t* reg) {
Review Comment:
missing doxgygen
##########
libs/framework/include_deprecated/bundle_context.h:
##########
@@ -119,6 +120,20 @@
bundleContext_retainServiceReference(celix_bundle_context_t *context, service_re
CELIX_FRAMEWORK_DEPRECATED_EXPORT celix_status_t
bundleContext_ungetServiceReference(celix_bundle_context_t *context,
service_reference_pt reference);
+typedef struct celix_service_ref {
+ service_reference_pt reference;
+ celix_bundle_context_t* context;
+} celix_service_ref_t;
+
+
+static __attribute__((__unused__)) inline void
celix_ServiceRef_put(celix_service_ref_t* ref) {
Review Comment:
Is `CELIX_UNUSED` usable here?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
