Junio C Hamano <gitster@xxxxxxxxx> writes: > Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes: > >> Oh, we are already safely in Unrelated Tangent Land for a while, I would >> think. Nothing of what we are discussing in this thread has anything to do >> with Kevin's patch series,... > > Oh, no question about that. Go back to my review, to which your > message I am responding to is a reply to. What you wrote are all > about things after "This is a tangent, but this made me wonder if it > is safe to simply free(3) the result...", which pointed out rough > points in the hashmap API from the API user's point of view and > suggested a few possible improvement opportunities. In other words, I'd be happy with a patch like this, outside the scope of and independent from this series. When the hashmap_entry structure does acquire references to external resources (which I wouldn't judge the likelihood of), this paragraph will become stale, but that is exactly the point at which _clear() function needs to be added to the API and described here, replacing this paragraph. I do not mind having an empty _clear() before that happens, but I do not think it adds much safety. A disciplined user of the API may call that empty _clear() to make her code future-proof, but we know there are undisciplined developers and reviewers and there will be codepaths that call _init() without calling the empty _clear(), and we won't notice it. Whoever is adding the real need for _clear() must audit the codebase at that point _anyway_, and that is why I think having an empty _clear() before would not buy us much. Documentation/technical/api-hashmap.txt | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Documentation/technical/api-hashmap.txt b/Documentation/technical/api-hashmap.txt index ad7a5bd..1dcec3d 100644 --- a/Documentation/technical/api-hashmap.txt +++ b/Documentation/technical/api-hashmap.txt @@ -104,6 +104,11 @@ If `free_entries` is true, each hashmap_entry in the map is freed as well `entry` points to the entry to initialize. + `hash` is the hash code of the entry. ++ +The hashmap_entry structure does not hold references to external resources, +and it is safe to just discard it once you are done with it (i.e. if +your structure was allocated with xmalloc(), you can just free(3) it, +and if it is on stack, you can just let it go out of scope). `void *hashmap_get(const struct hashmap *map, const void *key, const void *keydata)`:: -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html