RE: GDB 7.2 - Patch proposal for the use of GDB/Python scripts on MinGW

[Serge CHATROUX <serge.chatroux@st.com>]

> Do you have a copyright assignment in place for gdb?
> If not, email me and we can get the process started.

Yes (I work with Christophe Lyon).

> It is generally better if you send separate patches for each thing.
> Also, we like submissions to include a ChangeLog entry... see src/gdb/CONTRIBUTE for various other things.

I will split the patch into severals.
I also replied (directly) to Daniel Jacobowitz comments and wait its feedback.

> Serge> + #ifdef __MINGW32__
> Serge> +   /*
> Serge> + 	 I have to modify some gdb python files because the files
> 'python-function.c', python-cmd.c and python-frame.c define some 'static PyTypeObject'.
> Serge> +      The field 'tp_new' of these static variables statically
> initialized with the python 'PyType_GenericNew' function.
> Serge> +      In Windows port of Python, this function is declared as a dllimport function and can not be copied at compilation time in a static variable.
> Serge> +      For these 3 files, I modify the source code to initialize the 'tp_new' fields to '0' in the static variables and to affect the proper 'PyType_GenericNew' value in the 'gdbpy_initialize_commands', 'gdbpy_initialize_frames' and 'gdbpy_initialize_functions'.
> Serge> +   */
> Serge> +   cmdpy_object_type.tp_new = PyType_GenericNew; #endif
>
> Just make your patch do this unconditionally on all hosts, with a little (one line -- not as long as what you have above) comment explaining where it is needed.  I think that is clearer than using #ifdef all over.

I can reduce the comment size.
I do not know how to avoid the '#if __MINGW32__' because this modification is not needed for other systems (Linux, cygwin).
It seems that Daniel Jacobowitz succeed in compiling gdb over MINGW without this modification.

> Serge> + #ifdef __MINGW32__
> Serge> +   {
> Serge> +     /* We can not use 'PyRun_SimpleFile' because Python may not be compiled using the same MSVCRT DLL than GDB
> Serge> +        so the FILE* stream will not be known in this DLL. This generate an error when the MSVCRT will try to lock the file handle. */
>
> These lines are too long.  In GDB we wrap at column 79 or so; the GNU coding standards cover this.

OK.

> Serge> +     PyObject* PyFileObject = PyFile_FromString( (char*) file, "r");
>
> The spacing is different from our standard.  It should look like:
> 
>      PyObject *file_object = PyFile_FromString ((char *) file, "r");
>
> Perhaps this code and some surrounding code should be refactored so that we can just avoid FILE* and use the same code path on all hosts.

It could be great. I don't know the name of the Python scripting support maintainers. It could be great to have their feedback.

> Serge> +   /* Do not enable python scripting if the PYTHONHOME variable is undefined */
> Serge> + #ifdef HAVE_PYTHON
> Serge> +   if (getenv( "PYTHONHOME") == NULL)
> Serge> +     {
> 
> This doesn't seem correct to me.
> 
> It definitely isn't ok for the Linux distro case.  There, Python comes with the system and basically nobody sets PYTHONHOME.
> 
> It isn't clear to me that it is always correct even on MinGW.  Couldn't someone ship a pre-built GDB plus a pre-built Python in the same tree, and expect it to work properly?
>
> What is the failure mode here?  GDB finds the python DLL but still somehow doesn't work?

I will remove this part of the patch ($PYTHONHOME detection...), it is not relevant for a standard version.
I set this feature to solve an issue that I had on Linux:
- I compiled Python support without the --enabled-shared support. 
- At gdb compilation time, the Python library was embed (linked with libpython2.6.a not the shared library).
When I use the gdb program on a Linux host on which Python is not installed or without defined PYTHONPATH variable, I got the following message:
	gdb
	'import site' failed; use -v for traceback
	GNU gdb (GDB) 7.2
	Copyright (C) 2010 Free Software Foundation, Inc.
By checking the PYTHONHOME variable, I tried to check that the user set a valid Python environment. 


Regards