Thread (2 messages) flat view 2 messages, 2 authors, 2016-06-16

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help