On Wed, 9 May 2018 17:40:11 -0700 Stefan Beller <sbeller@xxxxxxxxxx> wrote: > if (obj->type == OBJ_TREE) > - release_tree_node((struct tree*)obj); > + free_tree_buffer((struct tree*)obj); > else if (obj->type == OBJ_COMMIT) > - release_commit_node((struct commit*)obj); > + release_commit_memory((struct commit*)obj); > else if (obj->type == OBJ_TAG) > - release_tag_node((struct tag*)obj); > + free_tag_buffer((struct tag*)obj); This might seem a bit bikesheddy, but I wouldn't call it free_tag_buffer(), since what's being freed is not the buffer of the object itself, but just a string. If you want such a function, I would just call it release_tag_memory() to match release_commit_memory(). Other than that, all the patches look fine to me. Some optional comments (this is almost certainly bikeshedding): - I would call them release_commit() and release_tag(), to match strbuf_release(). - It might be better to just inline the handling of releasing commit and tag memory. This code already knows that, for a tree, it needs to free its buffer and only its buffer, so it is not much of a stretch to think that it similarly knows the details of commit and tag objects too.
