From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 20870 invoked by alias); 21 May 2003 16:20:02 -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 20835 invoked from network); 21 May 2003 16:20:02 -0000 Received: from unknown (HELO localhost.redhat.com) (207.219.125.131) by sources.redhat.com with SMTP; 21 May 2003 16:20:02 -0000 Received: from redhat.com (localhost [127.0.0.1]) by localhost.redhat.com (Postfix) with ESMTP id 9A8852B5A; Wed, 21 May 2003 12:19:50 -0400 (EDT) Message-ID: <3ECBA726.1020106@redhat.com> Date: Wed, 21 May 2003 16:20: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> Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2003-05/txt/msg00386.txt.bz2 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 > > >