From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 10007 invoked by alias); 22 May 2003 17:50:43 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 9321 invoked from network); 22 May 2003 17:50:25 -0000 Received: from unknown (HELO localhost.redhat.com) (207.219.125.131) by sources.redhat.com with SMTP; 22 May 2003 17:50:25 -0000 Received: from redhat.com (localhost [127.0.0.1]) by localhost.redhat.com (Postfix) with ESMTP id 0A4202B2F; Thu, 22 May 2003 13:50:18 -0400 (EDT) Message-ID: <3ECD0DD9.1080902@redhat.com> Date: Thu, 22 May 2003 17:50:00 -0000 From: Andrew Cagney User-Agent: Mozilla/5.0 (X11; U; NetBSD macppc; en-US; rv:1.0.2) Gecko/20030223 X-Accept-Language: en-us, en MIME-Version: 1.0 To: Kevin Buettner Cc: gdb-patches@sources.redhat.com Subject: Re: [patch rfc] Eliminate extract_address References: <3EC23225.4090605@redhat.com> <1030514164201.ZM9355@localhost.localdomain> <3EC3C50F.1060700@redhat.com> <1030515182039.ZM13780@localhost.localdomain> <3EC3E455.9080100@redhat.com> <3ECBA726.1020106@redhat.com> <1030521164111.ZM31391@localhost.localdomain> Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2003-05/txt/msg00428.txt.bz2 > 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. I'll do this (which means I'll re-do the change piece meal) and commit over comming days. Andrew >> 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 > > > >