Re: [PATCH 033/238] [misc.] breakpoint.c: -Wshadow fix

[Pedro Alves <pedro at codesourcery dot com>]

On Monday 05 December 2011 11:25:23, Joel Brobecker wrote:
> > Cause:
> >        Variable in the inner scope shadowed by variable from outer
> >        one(defined at the beginning of function.
> [...]
> > diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> > index 1a4974c..69a8782 100644
> > --- a/gdb/breakpoint.c
> > +++ b/gdb/breakpoint.c
> > @@ -10667,8 +10667,8 @@ update_global_location_list (int should_insert)
> >      {
> >        /* ALL_BP_LOCATIONS bp_location has LOC->OWNER always
> >  	 non-NULL.  */
> > -      struct breakpoint *b = loc->owner;
> >        struct bp_location **loc_first_p;
> > +      b = loc->owner;
> >  
> >        if (b->enable_state == bp_disabled
> >  	  || b->enable_state == bp_call_disabled
> 
> I looked at it, and it looks fine from a functional point of view.
> However, I'd rather have Jan or Pedro, who have modified this function
> more often than I have, to weigh in.

Looks fine to me.

> Personally, I'm not keen on the fact that a global variable is reused
> in the context of a local loop. So I would rather rename the local
> variable inside the loop rather than delete the local variable,
> and reuse the global one.  It makes for a bigger patch, but I think
> it's better in the end.

I'd agree if e.g., `b' was a passed in function argument, that had
some special meaning to the function.  But `b' in this function
is really just a temporary variable helper for the
ALL_BREAKPOINTS loops.

-- 
Pedro Alves