From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 22215 invoked by alias); 3 Feb 2009 00:23:46 -0000 Received: (qmail 22207 invoked by uid 22791); 3 Feb 2009 00:23:45 -0000 X-SWARE-Spam-Status: No, hits=-2.4 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 03 Feb 2009 00:23:41 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 4B2162A9610; Mon, 2 Feb 2009 19:23:39 -0500 (EST) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id W44TNwrsEHIB; Mon, 2 Feb 2009 19:23:39 -0500 (EST) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id B3E9C2A95FF; Mon, 2 Feb 2009 19:23:38 -0500 (EST) Received: by joel.gnat.com (Postfix, from userid 1000) id 2841FE7ACD; Mon, 2 Feb 2009 16:23:36 -0800 (PST) Date: Tue, 03 Feb 2009 00:23:00 -0000 From: Joel Brobecker To: Thiago Jung Bauermann Cc: Daniel Jacobowitz , gdb-patches ml Subject: Re: [RFA] Add la_getstr member to language_defn Message-ID: <20090203002336.GB3964@adacore.com> References: <1227417278.28256.183.camel@localhost.localdomain> <20081123161013.GA15069@caradoc.them.org> <1227490821.8533.25.camel@hotblack.bauerhaus> <20081124022858.GA19331@caradoc.them.org> <1227551659.28256.225.camel@localhost.localdomain> <20081124202146.GA1991@caradoc.them.org> <1227564549.28256.248.camel@localhost.localdomain> <1230949603.8380.143.camel@localhost.localdomain> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1230949603.8380.143.camel@localhost.localdomain> User-Agent: Mutt/1.4.2.2i Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2009-02/txt/msg00043.txt.bz2 > The patch doesn't apply anymore. This is the same patch, refreshed > against HEAD as of Dec 28th. Ok? Just a few comments and questions in addition to Tom's comments... > + if ((TYPE_NFIELDS (type) == 1) > + && TYPE_CODE (TYPE_FIELD_TYPE (type, 0)) == TYPE_CODE_RANGE) The extra parens around "TYPE_NFIELDS (type) == 1" shouldn't be necessary, right? In this case, it's pretty harmless, but a little bit below, this really starts making it hard to read a condition... > + if (((VALUE_LVAL (value) == not_lval) > + || (VALUE_LVAL (value) == lval_internalvar)) && (fetchlimit != UINT_MAX)) Can this be formatted as follow: if ((VALUE_LVAL (value) == not_lval || VALUE_LVAL (value) == lval_internalvar) && fetchlimit != UINT_MAX) ? (assuming my reading is correct!) The reason I like my suggestion is because there are less parentheses, so it's easier to match them without using my favorite editor; also, the formatting makes it clear at which level the || and the && operators are. > + if ((TYPE_CODE (element_type) != TYPE_CODE_INT) > + && (TYPE_CODE (element_type) != TYPE_CODE_CHAR)) Same here. > @@ -511,6 +626,7 @@ const struct language_defn minimal_language_defn = > c_language_arch_info, > default_print_array_index, > default_pass_by_reference, > + default_get_string, > LANG_MAGIC > }; I was wondering if it wouldn't be more useful to use the c_get_string function as the default rather than the default_get_string stub. What do you guys think? Granted, I know that there are cases for Ada where this isn't going to be enough, but I also know that it's going to handle some cases fine. I don't know how other languages such as Fortran or Pascal encode their strings, but chances are that if we call this function with a value whose type is an array of chars/ints or a char/int pointer, the c_get_string value would probably work. As for the Ada implementation, it's probably going to look like this: if (value_type is char/int_array or char/int_pointer) { /* A string a-la-C: Call the C get_string routine... */ c_get_string (...); return; } /* Ada-specific strings handled here. */ So setting the la_get_string method to c_get_string would be a good starting point for Ada. The implementation is robust enough that we'd get an error if the value type is not handled properly, which is exactly what the default_get_string routine always does. -- Joel