Re: [PATCH v2 2/4] xdiff/xprepare: refactor abort cleanups

[Phil Hord <hordp@cisco.com>]

On 07/06/2011 07:31 PM, Junio C Hamano wrote:
> Tay Ray Chuan <rctay89@gmail.com> writes:
>
>> On Thu, Jul 7, 2011 at 1:57 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Tay Ray Chuan <rctay89@gmail.com> writes:
>>>
>>>> Group free()'s that are called when a malloc() fails in
>>>> xdl_prepare_ctx(), making for more readable code.
>>> Nicer, but I wonder if we can get away with a single label that lets us
>>> abort with freeing whatever we have allocated by making sure that the
>>> various pointer fields and variables are initialized to NULL, which may
>>> lead to even more readable code.
>> Pardon my dullness. Do you mean to check if the various fields are set
>> to NULL upon allocation, and free()'ing them if so?
> What I meant was that, instead of doing this:
>
>     func() {
> 	if (somefunc(&A, ...) < 0)
> 		goto failA;
>         ... do something ...
> 	B = someotheralloc();
>         if (!B)
>         	goto failB;
>         ... do something ...
> 	C = yetanotheralloc();
>         if (!C)
>         	goto failC;
> 	... do things using A, B, C ...
>
> 	return 0;
>     failC:
>     	free(B);
>     failB:
> 	free(A);
>     failA:
>         return -1;
>     }
>
> it would be easier to follow if you did:
>
>     func() {
> 	A = B = C = NULL;
> 	if (somefunc(&A, ...) < 0)
> 		goto fail;
>         ... do something ...
> 	B = someotheralloc();
>         if (!B)
>         	goto fail;
>         ... do something ...
> 	C = yetanotheralloc();
>         if (!C)
>         	goto fail;
> 	... do things using A, B, C ...
>
>     fail:
>         free(B);
>     	free(A);
>         return -1;
>     }
>
> Especially when you have more than 2 such fail labels.


Because 'free(NULL)' is handled sanely (as a no-op).
http://pubs.opengroup.org/onlinepubs/009695399/functions/free.html

I haven't looked, but I assume xdl_cha_free does the same thing.  I only
assume this because Junio seems to imply it, though.

Phil