On Thu, Jun 02, 2016 at 02:18:47PM -0700, Junio C Hamano wrote: > The function takes a pointer to a pathspec structure, and releases > the resources held by it, but does not free() the structure itself. > Such a function should be called "clear", not "free". Hmm, makes sense, though... > * This is just something I noticed. Among the hits in > > $ git grep free_ \*.h > > I think free_notes() is also a candidate for such renaming, but > because we are not actively working on that subsystem, we may > want to leave that dog sleeping to avoid unnecessary code churn. > The same for diff_free_filespec_data(), for which a better name > would have been diff_filespec_clear(). I think diff_filespec_clear() would not be quite right. It is freeing only the allocated _data_, but leaving the other portions intact. Generally our _clear() functions reset the object back to an initial state, from which it can be reused. I don't see that as a big problem because there is an other object for the verb "free" here: "data". We are just freeing its data, but the rest of the object remains intact and we may fill in the data again later. But I think pathspec is in similar boat; it has not been cleared back to its initial state. But it is in a much _worse_ state than the filespec, which you can continue to use. It is in a totally broken state where "nr" does not correspond to the actual number of items, the has_wildcard flag is bogus, etc. So I think it would be OK to move it to "clear", but we should probably also zero the whole thing, too. -Peff -- 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