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