Re: [patch] Cleanup init_raw_breakpoint

[Pedro Alves <pedro at codesourcery dot com>]

On Thursday 10 November 2011 07:55:30, Yao Qi wrote:
> Hi,
> portion of init_raw_breakpoint is quite similar to
> add_location_to_breakpoint, except the former is to "set"
> bp_location while the later is to "append" bp_location.  Beside this
> similarity, looks like there is an inconsistency between two
> parts about "adjust bp's address prior to allocating the
> location" introduced in this patch,
> 
>   [rfc] Set a breakpoint's type before adjusting its address
>   http://sourceware.org/ml/gdb-patches/2007-05/msg00039.html
> 
> So it is a good idea to move the code into one function
> (add_location_to_breakpoint) to remove duplicated code and
> inconsistency.  This is what this patch is trying to do.

Makes sense.

> -  for (tmp = &(b->loc); *tmp != NULL; tmp = &((*tmp)->next))
> -    ;
> -  *tmp = loc;
...
> +
> +  if (b->loc == NULL)
> +    b->loc = loc;

I don't understand the need for the explicit NULL check.

When b->loc is NULL, this:

     for (tmp = &(b->loc); *tmp != NULL; tmp = &((*tmp)->next))
      ;
     *tmp = loc;

is the same as `b->loc = loc'?

> +  else
> +    {
> +      struct bp_location **tmp;
> +
> +      for (tmp = &(b->loc); *tmp != NULL; tmp = &((*tmp)->next))
> +       ;
> +      *tmp = loc;
> +    }
> +

-- 
Pedro Alves