Re: [PATCH 2/2] checkout: forbid "-B <branch>" from touching a branch used elsewhere

> >> +	if (opts->new_branch_force) {
> >> +		char *full_ref = xstrfmt("refs/heads/%s", opts->new_branch);
> >> +		die_if_switching_to_a_branch_in_use(opts, full_ref);
> >> +		free(full_ref);
> >
> > At the moment this is academic as neither of the test scripts changed
> > by this patch are leak free and so I don't think we need to worry
> > about it but it raises an interesting question about how we should
> > handle memory leaks when dying. Leaving the leak when dying means that
> > a test script that tests an expected failure will never be leak free
> > but using UNLEAK() would mean we miss a leak being introduced in the
> > successful case should the call to "free()" ever be removed.
> 
> Is there a leak here?  The piece of memory is pointed at by an on-stack
> variable full_ref when leak sanitizer starts scanning the heap and
> the stack just before the process exits due to die, so I do not see
> a reason to worry about this particular variable over all the other
> on stack variables we accumulated before the control reached this
> point of the code.

> Are you worried about optimizing compilers that behave more cleverly
> than their own good to somehow lose the on-stack reference to
> full_ref while calling die_if_switching_to_a_branch_in_use()?  We
> might need to squelch them with UNLEAK() but that does not mean we
> have to remove the free() we see above, and I suspect a more
> productive use of our time to solve that issue is ensure that our
> leak-sanitizing build will not triger such an unwanted optimization
> anyway.