On 10/08/2014 09:36 AM, Jeff King wrote:
> On Tue, Oct 07, 2014 at 01:25:54PM +0200, Michael Haggerty wrote:
>
>>> +static void object_array_release_entry(struct object_array_entry *ent)
>>> +{
>>> + if (ent->name != object_array_slopbuf)
>>> + free(ent->name);
>>> +}
>>> +
>>
>> Would it be a little safer to set ent->name to NULL or to
>> object_array_slopbuf after freeing the memory, to prevent accidents?
>
> I considered that, but what about the other parts of object_array_entry?
> Should we NULL the object context pointers, too?
>
> The intent of this function is freeing memory, not clearing it for sane
> reuse. I think I'd be more in favor of a comment clarifying that. It is
> a static function used only internally by the object-array code.
I guess the name reminded me of strbuf_release(), which returns the
strbuf to its newly-initialized state (contrary to what api-strbuf.txt
says, I just noticed). You're right that your function does no such
thing, so it is self-consistent for it not to set ent->name to NULL.
But maybe its name could be chosen better? Let's see if there is a
consensus naming policy for functions that free resources. I grepped for
short functions calling free() and visually inspected a bunch of them.
Functions *_release():
* strbuf_release(), range_set_release(), and diff_ranges_release()
completely reinitialize their arguments
* window_release() appears not to
Functions *_clear() and clear_*():
* All *_clear() functions that I looked at (e.g., argv_array_clear(),
clear_image(), credential_clear(), clear_exclude_list(),
signature_check_clear(), and clear_prio_queue()) completely reinitialize
their arguments
Functions *_free() and free_*():
* Almost all of these free their arguments plus anything that their
arguments point at.
* Confusingly, free_ref_list() and free_pathspec() don't free their
arguments, but rather only the things that their arguments points at.
(Perhaps they should be renamed.)
So while three out of four *_release() functions completely reinitialize
their arguments, there is one that doesn't. And I couldn't find enough
other functions that just free referenced memory without reinitializing
their whole argument to establish a naming pattern. So I guess your
function name is OK too.
So forget I said anything :-)
Michael
--
Michael Haggerty
mhagger@xxxxxxxxxxxx
--
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