Re: [rfc] [01/05] Get rid of current_gdbarch in gdbarch.{c,h,sh}

["Ulrich Weigand" <uweigand@de.ibm.com>]

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