From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5367 invoked by alias); 8 Nov 2007 13:18:12 -0000 Received: (qmail 5355 invoked by uid 22791); 8 Nov 2007 13:18:11 -0000 X-Spam-Check-By: sourceware.org Received: from mtagate7.de.ibm.com (HELO mtagate7.de.ibm.com) (195.212.29.156) by sourceware.org (qpsmtpd/0.31) with ESMTP; Thu, 08 Nov 2007 13:18:05 +0000 Received: from d12nrmr1607.megacenter.de.ibm.com (d12nrmr1607.megacenter.de.ibm.com [9.149.167.49]) by mtagate7.de.ibm.com (8.13.8/8.13.8) with ESMTP id lA8DI1Wj240192 for ; Thu, 8 Nov 2007 13:18:01 GMT Received: from d12av02.megacenter.de.ibm.com (d12av02.megacenter.de.ibm.com [9.149.165.228]) by d12nrmr1607.megacenter.de.ibm.com (8.13.8/8.13.8/NCO v8.6) with ESMTP id lA8DI1fR2347258 for ; Thu, 8 Nov 2007 14:18:01 +0100 Received: from d12av02.megacenter.de.ibm.com (loopback [127.0.0.1]) by d12av02.megacenter.de.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id lA8DI1bT012763 for ; Thu, 8 Nov 2007 14:18:01 +0100 Received: from tuxmaker.boeblingen.de.ibm.com (tuxmaker.boeblingen.de.ibm.com [9.152.85.9]) by d12av02.megacenter.de.ibm.com (8.12.11.20060308/8.12.11) with SMTP id lA8DI1q5012756; Thu, 8 Nov 2007 14:18:01 +0100 Message-Id: <200711081318.lA8DI1q5012756@d12av02.megacenter.de.ibm.com> Received: by tuxmaker.boeblingen.de.ibm.com (sSMTP sendmail emulation); Thu, 8 Nov 2007 14:18:01 +0100 Subject: Re: [rfc] [01/05] Get rid of current_gdbarch in gdbarch.{c,h,sh} To: deuling@de.ibm.com (Markus Deuling) Date: Thu, 08 Nov 2007 13:18:00 -0000 From: "Ulrich Weigand" Cc: gdb-patches@sourceware.org (GDB Patches), drow@false.org (Daniel Jacobowitz) In-Reply-To: <4731D20A.7020506@de.ibm.com> from "Markus Deuling" at Nov 07, 2007 03:56:10 PM X-Mailer: ELM [version 2.5 PL2] MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit 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: 2007-11/txt/msg00164.txt.bz2 Markus Deuling wrote. > Daniel Jacobowitz schrieb: > > On Wed, Nov 07, 2007 at 12:11:00PM +0100, Markus Deuling wrote: > >> architecture. This ensures that the new architectures initial > >> values are not influenced by the previous architecture. Once > >> everything is parameterised with gdbarch, this will go away. */ > >> - struct gdbarch *current_gdbarch; > >> + struct gdbarch *gdbarch; > > > > Please read the comment above this variable :-) > > > > Hm, > > will gdbarch_alloc go away? I thought every target uses gdbarch_alloc to allocate a basic > gdbarch structure and then it overwrites every necessary callback to fit to its architecture. > > This patch just changes the name of current_gdbarch to gdbarch. For gdbarch_alloc current_gdbarch > is a local variable invisible to the rest. Its not the global current_gdbarch what this patch changes. > > For me its a bit confusing to have a global current_gdbarch and a local one. Of course it is a hack to have the local current_gdbarch shadowing the global one -- that is why the comment *says* it is a hack and should go away at some time in future ("once everything is parameterised with gdbarch"). However, as your patch *implements* what the comment describes -- as of right now, there are no macros implicitly using current_gdbarch any more, and thus there is no need for the local current_gdbarch hack -- it does not make any sense to leave the comment unchanged once your patch is in. I would suggest you simply remove this comment as part of your patch. Also, there is another comment that needs to be updated: # Variable declarations can refer to ``current_gdbarch'' which # will contain the current architecture. Care should be # taken. (at the description of "postdefault" in the loop immediately preceding function_list). In general, having stale comments in the code can be very confusing for developers working on that code in the future, so when you work on a patch, making sure comments are added/removed/changed as appropriate is just as important as making sure the code changes themselves are correct ... Finally, I do think the change in inself is correct (provided the comments are adapted). In fact, the very same should be done to the other two functions in gdbarch.sh that use the "local current_gdbarch variable" hack, verify_gdbarch and gdbarch_dump. Bye, Ulrich -- Dr. Ulrich Weigand GNU Toolchain for Linux on System z and Cell BE Ulrich.Weigand@de.ibm.com