bison-patches
[Top][All Lists]
Advanced

[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.




reply via email to

[Prev in Thread] Current Thread [Next in Thread]