PengZheng commented on a change in pull request #399:
URL: https://github.com/apache/celix/pull/399#discussion_r810608465



##########
File path: libs/framework/include/service_registration.h
##########
@@ -39,9 +39,6 @@ FRAMEWORK_EXPORT celix_status_t 
serviceRegistration_unregister(service_registrat
 FRAMEWORK_EXPORT celix_status_t
 serviceRegistration_getProperties(service_registration_t *registration, 
celix_properties_t **properties);
 
-FRAMEWORK_EXPORT celix_status_t
-serviceRegistration_setProperties(service_registration_t *registration, 
celix_properties_t *properties);

Review comment:
       There are two problems with the current design:
   
   1. memory leak in `serviceRegistration_setProperties`
   2. `serviceRegistration_getProperties` is inherently unsafe in 
multi-threading
   
   Given that there is currently no user of setProperties, I suggest making 
properties read-only.




-- 
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