Re: [patch rfc] Eliminate extract_address

[Andrew Cagney <ac131313@redhat.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