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: dev-unsubscr...@celix.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to