This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
Re: [patch rfc] Eliminate extract_address
On May 21, 12:19pm, Andrew Cagney wrote:
> Kevin?
>
> The other option would be deprecate it, but I'd prefer not to as when
> pratical elimination is always better than deprecation.
Yes, I agree with this sentiment.
If you are confident that those of us reading the code will be able to
determine that it is actually an address that's being extracted, then
I have no further objections. If there's any place where it's unclear,
then I suggest the addition of a comment.
Kevin
> Andrew
>
>
> >> 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
> >
> >
> >
>
>-- End of excerpt from Andrew Cagney