-----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 muller@ics.u-strasbg.fr
- 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
-
Pierre Muller