[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] Style change and some factoring
From: |
Joel E. Denny |
Subject: |
Re: [PATCH] Style change and some factoring |
Date: |
Mon, 27 Jul 2009 12:39:41 -0400 (EDT) |
User-agent: |
Alpine 1.00 (DEB 882 2007-12-20) |
Hi Alex.
On Fri, 24 Jul 2009, Alex Rozenman wrote:
> Thank you for the testing. I fixed the problem by the attached patch.
Thanks for doing that.
> Please
> review my changes in "symbol_list_free". I was forced to "inline" the
> LIST_FREE macro in order not to traverse the list twice. Let me know if you
> have objections.
> void
> symbol_list_free (symbol_list *list)
> {
> - LIST_FREE (symbol_list, list);
> + symbol_list *node, *next;
> + for (node = list; node; node = next)
> + {
> + next = node->next;
> + if (node->named_ref)
> + named_ref_free (node->named_ref);
> + free (node);
> + }
> }
Seems fine.
> commit 1e20ad112fc43f3d6adb3cc26be69ebffb14e9f6
> Author: Alex Rozenman <address@hidden>
> Date: Fri Jul 24 21:04:16 2009 +0300
>
> Fix some memory leaks.
> * src/named-ref.c: Add a pointer check (named_ref_free).
> * src/scan-code.l: New function (variant_table_free). Called in
> code_scanner_free.
> * src/symlist.c: Call to named_ref_free (symbol_list_free).
In your git log entries, leave a blank line after the first line. That
way, commands like git-format-patch can correctly extract an email subject
line. In ChangeLog, we usually omit the blank line as you've done.
> void
> named_ref_free (named_ref *r)
> {
> - free (r);
> + if (r)
> + free (r);
> }
The `if' is redundant. free (NULL) does nothing.
> +static void
> +variant_table_free ()
> +{
> + if (variant_table)
> + free (variant_table);
Likewise.