git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Phil Hord <hordp@cisco.com>
Date
Jul 7, 2011, 03:55 UTC
Message-ID
<4E152E31.4020105@cisco.com>
In-Reply-To
<7vaacrhtp3.fsf@alter.siamese.dyndns.org>
On 07/06/2011 07:31 PM, Junio C Hamano wrote:
Show 60 quoted lines
> 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
Previous: Junio C HamanoNext: Tay Ray Chuan
Message 10 of 16 in “prepare for histogram diff”
  1. 0/4 prepare for histogram diffTay Ray Chuan, Jul 6, 2011
  2. 1/4 xdiff/xprepare: use memset()Tay Ray Chuan, Jul 6, 2011
  3. 2/4 xdiff/xprepare: refactor abort cleanupsTay Ray Chuan, Jul 6, 2011
  4. 3/4 xdiff/xpatience: factor out fall-back-diff functionTay Ray Chuan, Jul 6, 2011
  5. 4/4 t4033-diff-patience: factor out testsTay Ray Chuan, Jul 6, 2011
  6. Junio C HamanoJul 6, 2011
  7. Junio C HamanoJul 6, 2011
  8. Tay Ray ChuanJul 6, 2011
  9. Junio C HamanoJul 6, 2011
  10. Phil HordJul 7, 2011
  11. Tay Ray ChuanJul 7, 2011
  12. 0/4 prepare for histogram diffTay Ray Chuan, Jul 7, 2011
  13. 1/4 xdiff/xprepare: use memset()Tay Ray Chuan, Jul 7, 2011
  14. 2/4 xdiff/xprepare: refactor abort cleanupsTay Ray Chuan, Jul 7, 2011
  15. 3/4 xdiff/xpatience: factor out fall-back-diff functionTay Ray Chuan, Jul 7, 2011
  16. 4/4 t4033-diff-patience: factor out testsTay Ray Chuan, Jul 7, 2011

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.