Re: [patch rfc] Eliminate extract_address

[Andrew Cagney <ac131313 at redhat dot com>]

> On May 15, 12:49pm, Andrew Cagney wrote:
>
>
> > > First, the return types are different.  extract_address() returns
> > > CORE_ADDR while extract_unsigned_integer returns ULONGEST.  If
> > > we were to encounter a scenario where this is a problem, it's easier
> > > to fix a wrapper (extract_address()) instead of the myriad places in
> > > the code which presently call extract_address().  (This point is
> > > probably moot because I suspect we already have a lot of code which
> > > assumes that CORE_ADDR may be interchanged with LONGEST or ULONGEST
> > > anyway.)
>
> > sizeof(CORE_ADDR) <= sizeof(ULONGEST) so this isn't a problem.
>
>
> Do we have a gdb_assert() somewhere to ensure that this is the case?
> (This could happen at initialization time...)

Magic in "defs.h" does it.  An assert wouldn't hurt.

> > > Second, having function calls to extract_address() provides
> > > information to the reader that you don't get by having calls to
> > > extract_unsigned_integer().  It tells the reader that we're expecting
> > > to get an address and not an integer.  This really helps when someone
> > > reading gdb's code is wondering about what the thing is that's being
> > > extracted.
>
> > The extract_address function doesn't extract an address, it extracts an 
> > unsigned integer.
> > On the MIPS, extract_address needs to sign extend.  On the d10v, extract 
> > address needs to know the address space.
>
>
> Yes, I understand that.  Doing the substitution you propose will make
> it more difficult to make the correct fix (of using extract_typed_address)
> at a later time.
>
>
> > If the code needs to extract an address it can use extract_typed_address 
> > which corectly handles all these cases.
>
>
> Yes.
>
>
> > Is it a good thing?  It eliminates a lie.
>
>
> At the expense of making the code marginally less comprehensible and
> making it more difficult to identify the potential cases where
> extract_typed_address() should be used instead.

I think it makes it more comprehensible - it is now very clear exactly 
how the value is being obtained.  The ``extract_address'' function gives 
the misleading impression that it is correctly extracting an address, 
and that (per MIPS and d10v) isn't the case.

It also takes away the assumption that extract_address can, some how, be 
made cross architecture.

> Or have all of those cases already been identified?  If so, then I
> withdraw my objection.  (Though I still like having "address" in the
> function name to help to document what it is that's being extracted.)

It tinkers with the following:

- ada/jv-* where things are pretty broken

- dwarf2 which is extracting/assuming an an unsigned integer

- unsigned_pointer_to_address making its implementation consistent with 
signed_pointer_to_address

- solib* where it is now (worryingly) clear what the code is doing.

- stack.c where it's printing out an integer value

After that, it's all target dependant code.

Andrew