From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7957 invoked by alias); 26 Jun 2002 03:11:21 -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 7946 invoked from network); 26 Jun 2002 03:11:17 -0000 Received: from unknown (HELO localhost.redhat.com) (24.112.240.27) by sources.redhat.com with SMTP; 26 Jun 2002 03:11:17 -0000 Received: from cygnus.com (localhost [127.0.0.1]) by localhost.redhat.com (Postfix) with ESMTP id 0762B3C62; Tue, 25 Jun 2002 23:10:43 -0400 (EDT) Message-ID: <3D1930B2.9040708@cygnus.com> Date: Tue, 25 Jun 2002 20:11:00 -0000 From: Andrew Cagney User-Agent: Mozilla/5.0 (X11; U; NetBSD macppc; en-US; rv:1.0.0) Gecko/20020613 X-Accept-Language: en-us, en MIME-Version: 1.0 To: Grace Sainsbury Cc: gdb-patches@sources.redhat.com Subject: Re: [PATCH] monitor_ops regnames array References: <20020625170708.A4105@tomago.toronto.redhat.com> Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2002-06/txt/msg00525.txt.bz2 > With multi-arch NUM_REGS is not a constant, but monitor.c was treating > as a constant in determining the size of the numregs array. So I added > a function to monitor_ops that returns the register names, and doesn't > need NUM_REGS to be constant. Outch. > I only changed the rom files that are compiled for the m68k, but the > array is still there. Ok, I think having a functional interface will be better any way. > ok to commit? Yes, after the below tweaks: > -static char *abug_regnames[NUM_REGS] = > +static char * > +abug_regname (int index) > { > - "D0", "D1", "D2", "D3", "D4", "D5", "D6", "D7", > - "A0", "A1", "A2", "A3", "A4", "A5", "A6", "A7", > - "PC", > -}; > + static char *regnames[] = > + { > + "D0", "D1", "D2", "D3", "D4", "D5", "D6", "D7", > + "A0", "A1", "A2", "A3", "A4", "A5", "A6", "A7", > + "PC", > + }; I think the below should be: if (index >= (sizeof (regnames) / sizeof (regnames[0])) || ... > + if ((index >= sizeof (regnames)) || (index < 0) > + || index >= NUM_REGS) > + return NULL; > + else > + return regnames[index]; > Index: monitor.c > =================================================================== > RCS file: /cvs/src/src/gdb/monitor.c,v > retrieving revision 1.33 > diff -u -r1.33 monitor.c > --- monitor.c 18 Apr 2002 18:09:03 -0000 1.33 > +++ monitor.c 25 Jun 2002 20:30:08 -0000 > @@ -1183,7 +1183,10 @@ > zerobuf = alloca (MAX_REGISTER_RAW_SIZE); > memset (zerobuf, 0, MAX_REGISTER_RAW_SIZE); > > - name = current_monitor->regnames[regno]; > + if (current_monitor->regnames) Suggest testing regname first vis: if (current_monitor->regname != NULL) since that is the prefered interface. If, for some [strange] reason, both were present I think the function should be prefered. > + name = current_monitor->regnames[regno]; > + else > + name = current_monitor->regname (regno); > Index: monitor.h > =================================================================== > RCS file: /cvs/src/src/gdb/monitor.h,v > retrieving revision 1.8 > diff -u -r1.8 monitor.h > --- monitor.h 21 Oct 2001 19:20:30 -0000 1.8 > +++ monitor.h 25 Jun 2002 20:30:08 -0000 > @@ -116,6 +116,7 @@ > struct target_ops *target; /* target operations */ > int stopbits; /* number of stop bits */ Suggest a comment mentioning that ``regnames'' is ``depreacated'' and create a change-request [bug report] indicating that it should be zapped. > char **regnames; /* array of register names in ascii */ > + char *(*regname) (int index); /* function for dynamic regname array */ Is it possible to have (*regname)() return a ``const char *''? The fuctions would need updating. (One day, GDB will be compiled with -Wwrite-strings and the above would upset that). enjoy, Andrew