| Message ID | b295e9393a8998b3b9263ab7cd195907d4002e36.1598035949.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add struct strmap and associated utility functions | expand |
On Fri, Aug 21, 2020 at 06:52:25PM +0000, Elijah Newren via GitGitGadget wrote: > From: Elijah Newren <newren@gmail.com> > > The existence of hashmap_free() and hashmap_free_entries() confused me, > and the docs weren't clear enough. I had to consult other source code > examples and the implementation. Add a brief note to clarify, > especially since hashmap_clear*() variants may be added in the future. Thanks, I think this is worth doing and the text looks clear and correct to me. > /* > - * Frees a hashmap structure and allocated memory, leaves entries undisturbed > + * Frees a hashmap structure and allocated memory for the table, but does not > + * free the entries nor anything they point to. > + * > + * Usage note: > + * > + * Many callers will need to iterate over all entries and free the data each > + * entry points to; in such a case, they can free the entry itself while at it. > + * Thus, you might see: > + * hashmap_for_each_entry(map, hashmap_iter, e, hashmap_entry_name) { > + * free(e->somefield); > + * free(e); > + * } > + * hashmap_free(map); > + * instead of > + * hashmap_for_each_entry(map, hashmap_iter, e, hashmap_entry_name) { > + * free(e->somefield); > + * } > + * hashmap_free_entries(map, struct my_entry_struct, hashmap_entry_name); > + * to avoid the implicit extra loop over the entries. However, if there are > + * no special fields in your entry that need to be freed beyond the entry > + * itself, it is probably simpler to avoid the explicit loop and just call > + * hashmap_free_entries(). A minor nit, but a blank line between the code snippets and the text might make it a little more readable. -Peff
diff --git a/hashmap.h b/hashmap.h index ef220de4c6..a2f4adc1b3 100644 --- a/hashmap.h +++ b/hashmap.h @@ -236,13 +236,36 @@ void hashmap_init(struct hashmap *map, void hashmap_free_(struct hashmap *map, ssize_t offset); /* - * Frees a hashmap structure and allocated memory, leaves entries undisturbed + * Frees a hashmap structure and allocated memory for the table, but does not + * free the entries nor anything they point to. + * + * Usage note: + * + * Many callers will need to iterate over all entries and free the data each + * entry points to; in such a case, they can free the entry itself while at it. + * Thus, you might see: + * hashmap_for_each_entry(map, hashmap_iter, e, hashmap_entry_name) { + * free(e->somefield); + * free(e); + * } + * hashmap_free(map); + * instead of + * hashmap_for_each_entry(map, hashmap_iter, e, hashmap_entry_name) { + * free(e->somefield); + * } + * hashmap_free_entries(map, struct my_entry_struct, hashmap_entry_name); + * to avoid the implicit extra loop over the entries. However, if there are + * no special fields in your entry that need to be freed beyond the entry + * itself, it is probably simpler to avoid the explicit loop and just call + * hashmap_free_entries(). */ #define hashmap_free(map) hashmap_free_(map, -1) /* * Frees @map and all entries. @type is the struct type of the entry - * where @member is the hashmap_entry struct used to associate with @map + * where @member is the hashmap_entry struct used to associate with @map. + * + * See usage note above hashmap_free(). */ #define hashmap_free_entries(map, type, member) \ hashmap_free_(map, offsetof(type, member));