This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch rfc] Eliminate extract_address


Kevin?

The other option would be deprecate it, but I'd prefer not to as when pratical elimination is always better than deprecation.

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






Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]