Re: [PATCH] pathspec: rename free_pathspec() to clear_pathspec()
From: Jeff King <hidden>
Date: 2016-06-16 02:19:42
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