From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5681 invoked by alias); 26 Sep 2005 01:23:08 -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 5635 invoked by uid 22791); 26 Sep 2005 01:23:03 -0000 Received: from nevyn.them.org (HELO nevyn.them.org) (66.93.172.17) by sourceware.org (qpsmtpd/0.30-dev) with ESMTP; Mon, 26 Sep 2005 01:23:03 +0000 Received: from drow by nevyn.them.org with local (Exim 4.52) id 1EJhhr-0005pJ-Ou; Sun, 25 Sep 2005 21:22:59 -0400 Date: Mon, 26 Sep 2005 01:23:00 -0000 From: Daniel Jacobowitz To: Joel Brobecker Cc: Eli Zaretskii , gdb-patches@sources.redhat.com Subject: Re: [RFA] print arrays with indexes Message-ID: <20050926012259.GA22284@nevyn.them.org> Mail-Followup-To: Joel Brobecker , Eli Zaretskii , gdb-patches@sources.redhat.com References: <20050918054109.GD2496@adacore.com> <20050918191943.GA27191@nevyn.them.org> <20050920073058.GR2496@adacore.com> <20050920193132.GY2496@adacore.com> <20050920193339.GA28294@nevyn.them.org> <20050920193918.GB10186@adacore.com> <20050922164622.GF5841@adacore.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20050922164622.GF5841@adacore.com> User-Agent: Mutt/1.5.8i X-SW-Source: 2005-09/txt/msg00206.txt.bz2 On Thu, Sep 22, 2005 at 09:46:22AM -0700, Joel Brobecker wrote: > > This sounds good to me too. Perhaps a compromise between what Eli > > suggested and what you have been suggesting would work. How about > > we implement a on/off knob for now. Later on, if we understand better > > in which direction to use the threshold, then we can enhance the knob > > to include "auto", and then have a second knob. > > > > As long as we cross-reference each setting in the help text, I think > > users won't be confused. > > No objection to the above, so far... Fingers crossed. > > Would you know, it just occured to me that the above is what my last > patch actually implemented :). Assuming this intermediate approach is > ok, I'm resubmitting that patch again. > > Note that the patch was first submitted in: > > http://sources.redhat.com/ml/gdb-patches/2005-09/msg00047.html This looks OK to me, with documentation and tests. Minor comments: - Reading the diff it's apparent that there is some space vs tabs confusion in the Ada changes. I prefer tabs, but not violently so; however, I mostly prefer that code match its surroundings. Otherwise indentation in diffs looks very odd. > /* Called by various _val_print routines to print elements of an > array in the form ", , , ...". > > + Some languages such as Ada allow the user to specify arrays where > + the index of the first element is not zero. REAL_INDEX_OFFSET is > + the user-level index of the first element of the array. For many > + languages such as C or C++, it is always zero. > + Ada is by no means the only language with this feature - namely, Fortran. Can't you compute this value from the array type, instead of passing it in from Ada-specific code? I couldn't see any obvious reasons why not. > +/* Assuming TYPE is a simple, non-empty array type, compute the lower > + bound and the array index type. Save the low bound into LOW_BOUND > + if not NULL. Save the index type in INDEX_TYPE if not NULL. > + > + Return 1 if the operation was successful. Return zero otherwise, > + in which case the value of LOW_BOUND and INDEX_TYPE is undefined. */ s/undefined/unmodified/, since you rely on it below. -- Daniel Jacobowitz CodeSourcery, LLC