pnoltes commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1389455788


##########
libs/utils/include/celix_string_hash_map.h:
##########
@@ -90,14 +94,37 @@ typedef struct celix_string_hash_map_create_options {
      * only the simpledRemoveCallback will be used.
      *
      * Default is NULL.
+     *
+     * @param[in] data The void pointer to the data that was provided when the 
callback was set as removedCallbackData.
+     * @param[in] removedKey The key of the value that was removed from the 
hash map.
+     *                       Note that the removedKey can still be in use if 
the a value entry for the same key is
+     *                       replaced, so this callback should not free the 
removedKey.
+     * @param[in] removedValue The value that was removed from the hash map. 
This value is no longer used by the
+     *                         hash map and can be freed.
      */
     void (*removedCallback)(void* data, const char* removedKey, 
celix_hash_map_value_t removedValue) CELIX_OPTS_INIT;
 
+    /**
+     * @brief A removed key callback, which if provided will be called if a 
key is no longer used in the hash map.
+     *
+     * @param[in] data The void pointer to the data that was provided when the 
callback was set as removedCallbackData.
+     * @param[in] key The key that is no longer used in the hash map. if 
`storeKeysWeakly` was configured as true,
+     *                the key can be freed.
+     */
+    void (*removedKeyCallback)(void* data, char* key) CELIX_OPTS_INIT;
+
     /**
      * @brief If set to true, the string hash map will not make of copy of the 
keys and assumes
      * that the keys are in scope/memory for the complete lifecycle of the 
string hash map.
      *
-     * Note that this changes the default behaviour of the 
celix_stringHashMap_put* functions.
+     * When keys are stored weakly it is the caller responsibility to check 
the return value of
+     * celix_stringHashMap_put* function calls.
+     * If a celix_stringHashMap_put* function call returns true, the key is 
used in the hash map and the key
+     * should never be freed or freed in a configured removedKeyCallback.
+     * If a celix_stringHashMap_put* function call returns false, a value is 
replaced and the already existing
+     * key is reused. If the needed the caller should free the provided key.

Review Comment:
   I updated the documentation to clarify that the users should first check if 
a key exists with `celix_stringHashMap_hasKey`. This will lead to some more 
code, but gives users the flexibility - if possible - to only allocate a key if 
needed (as is done in properties).
   
   It is also possible to call the removeKeyCallback for every not used key, 
but I think this does not really work with the short properties string 
optimization buffer.
   
   An other option could be to add a set of  `celix_stringHashMap_replace*` 
functions, with a boolean output argument. 



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