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

Derrick Stolee [off-list ref] writes:

quoted

quoted

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.