From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 10376 invoked by alias); 26 Sep 2007 19:39:54 -0000 Received: (qmail 10367 invoked by uid 22791); 26 Sep 2007 19:39:53 -0000 X-Spam-Check-By: sourceware.org Received: from ics.u-strasbg.fr (HELO ics.u-strasbg.fr) (130.79.112.250) by sourceware.org (qpsmtpd/0.31) with ESMTP; Wed, 26 Sep 2007 19:39:50 +0000 Received: from ICSMULLER (unknown [130.79.244.148]) by ics.u-strasbg.fr (Postfix) with ESMTP id 78C3618701D; Wed, 26 Sep 2007 21:44:42 +0200 (CEST) From: "Pierre Muller" To: "'Joel Brobecker'" Cc: , References: <003901c8004b$2f9a55d0$8ecf0170$@u-strasbg.fr> <20070926175805.GA6208@adacore.com> In-Reply-To: <20070926175805.GA6208@adacore.com> Subject: RE: [RFC] Handle GPC specific name for main function Date: Wed, 26 Sep 2007 19:39:00 -0000 Message-ID: <001501c80074$ff05ddc0$fd119940$@u-strasbg.fr> MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: 7bit X-Mailer: Microsoft Office Outlook 12.0 Content-Language: en-us 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: 2007-09/txt/msg00385.txt.bz2 > -----Original Message----- > From: Joel Brobecker [mailto:brobecker@adacore.com] > Sent: Wednesday, September 26, 2007 7:58 PM > To: Pierre Muller > Cc: gdb-patches@sourceware.org; gpc@gnu.de > Subject: Re: [RFC] Handle GPC specific name for main function > > Hello Pierre, > > > 2007-09-26 Pierre Muller > > * p-lang.h (pascal_main_name): New function. > > p-lang.c (GPC_MAIN_PROGRAM_NAME_1, > > GPC_MAIN_PROGRAM_NAME_2): New char array constants > > corresponding to the two minimal symbols used > > by GPC compiler. > > (pascal_main_name): Try to find minimal symbol > > corresponding to the entry of GPC compiled programs. > > symtab.c (find_main_name): Try to find > > pascal specific main name by calling pascal_main_name. > > This is mostly OK, except for the tiny comment formatting pointed out > by Eli. I have a few suggestions: > > > +/* The name of the symbol used by GPC compiler. */ > > I find this is too vague. I suggest something like: > > /* The name of the symbol that GPC uses as the name of the main > subprogram. There are several possibilities depending on > the version of GPC. */ > > I might even go one step further and put a comment for each entry. > Something like: > > /* The name of the symbol that GPC uses as the name of the main > subprogram (since version ...). */ Here, I really need feedback from GPC developers! > And then, for the other one say: > > /* Older versions of GPC (version ... and older) were using > a different name for the main subprogram. */ > > > +static const char GPC_MAIN_PROGRAM_NAME_1[] > > + = "pascal_main_program"; > > +static const char GPC_MAIN_PROGRAM_NAME_2[] > > + = "_p__M0_main_program"; > > Also, I think you might want to order them the opposite way. > The net effect is that you'll end up trying the newer symbol > first, and then fallback to the older symbol. When people > start using the newer compiler, GDB will need to do only one > symbol lookup to find a match. You are perfectly right, chances are higher that the program will be compiled by a recent or future GPC. > > + /* For GPC, main procedure s a special name. > > + . */ > > I think making the comment above a bit more expressive makes > this comment superfluous. > > > @@ -40,6 +40,7 @@ > > #include "filenames.h" /* for FILENAME_CMP */ > > #include "objc-lang.h" > > #include "ada-lang.h" > > +#include "p-lang.h" > > > > #include "hashtab.h" > > > > You also need to update Makefile.in, since you added a dependency. Funny, while trying to add this, I noticed that ${ada_lang_h} was twice in the dependency list, so I just replaced the second one with ${p_lang_h} > With these adjustments in, your patch is pre-approved (but please > remember to give it a round of testing, just in cases). Thanks, but my problem is that I am unable to run the testsuite, and anyhow gdb.pascal is not yet in CVS, and its mainly there where I expect a change. Pierre