Re: [PATCH v1 4/7] mv: check if <destination> is a SKIP_WORKTREE_DIR

[Junio C Hamano <gitster@pobox.com>]

Derrick Stolee <derrickstolee@github.com> writes:

>>> Good that you are freeing this here. You might also want to set it to NULL
>>> just in case.
>> 
>> I was using the `FREE_AND_NULL()` macro, but I wasn't sure since other
>> places in 'git-mv' only use `free()`. Though I think it is better to
>> `FREE_AND_NULL()`.
>
> free() is generally the way to go if it is clear that the variable
> is about to go out-of-scope and could not possibly be referenced
> again. Since there is a lot more of the current code block to go,
> nulling the variable is good defensive programming.

NULLing it out is better when a potential misuse of the pointer
after it got freed will be caught by dereferencing NULL.

There however are pointer members of structures wher they represent
optional data.  Access to such a member goes like so:

	if (structure->optinal_member)
		do_things(structure->optional_member);

When you are done using such a structure and clearing it, after
releasing the resource held by the member, it is better to leave it
dangling than assigning NULL to it.  If somebody reuses that
structure and the control enters a codepath like the above one to
use the "optional" pointer, uncleared dangling pointer will likely
be caught at runtime; setting it to NULL will paper over it.  We've
seen many bugs caused by a premature releasing of a member that was
hidden exactly by such a use of FREE_AND_NULL() few relases ago.

Thanks.