From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 31963 invoked by alias); 5 Dec 2011 13:05:45 -0000 Received: (qmail 31953 invoked by uid 22791); 5 Dec 2011 13:05:44 -0000 X-SWARE-Spam-Status: No, hits=-1.8 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 05 Dec 2011 13:05:30 +0000 Received: from nat-ies.mentorg.com ([192.94.31.2] helo=EU1-MAIL.mgc.mentorg.com) by relay1.mentorg.com with esmtp id 1RXYEa-0000xq-Nl from pedro_alves@mentor.com ; Mon, 05 Dec 2011 05:05:28 -0800 Received: from scottsdale.localnet ([172.16.63.104]) by EU1-MAIL.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.1830); Mon, 5 Dec 2011 13:05:26 +0000 From: Pedro Alves To: Joel Brobecker Subject: Re: [PATCH 033/238] [misc.] breakpoint.c: -Wshadow fix Date: Mon, 05 Dec 2011 13:17:00 -0000 User-Agent: KMail/1.13.6 (Linux/2.6.38-13-generic; KDE/4.7.2; x86_64; ; ) Cc: Andrey Smirnov , Jan Kratochvil , gdb-patches@sourceware.org, tromey@redhat.com References: <1322493153-29512-1-git-send-email-andrew.smirnov@gmail.com> <20111205112523.GJ2777@adacore.com> In-Reply-To: <20111205112523.GJ2777@adacore.com> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201112051305.24880.pedro@codesourcery.com> X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2011-12/txt/msg00133.txt.bz2 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