This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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 ...).  */

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.

> +  /* 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.

With these adjustments in, your patch is pre-approved (but please
remember to give it a round of testing, just in cases).

Thank you,
-- 
Joel


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]