From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4318 invoked by alias); 13 Sep 2010 11:44:42 -0000 Received: (qmail 4307 invoked by uid 22791); 13 Sep 2010 11:44:40 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=BAYES_00 X-Spam-Check-By: sourceware.org Received: from eu1sys200aog115.obsmtp.com (HELO eu1sys200aog115.obsmtp.com) (207.126.144.139) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 13 Sep 2010 11:44:33 +0000 Received: from source ([167.4.1.35]) (using TLSv1) by eu1sys200aob115.postini.com ([207.126.147.11]) with SMTP ID DSNKTI4OnOdjKMtiZBGwAX8DlGTkNuXV33LU@postini.com; Mon, 13 Sep 2010 11:44:32 UTC Received: from zeta.dmz-us.st.com (ns4.st.com [167.4.80.115]) by beta.dmz-us.st.com (STMicroelectronics) with ESMTP id E1DE0F4; Mon, 13 Sep 2010 11:41:12 +0000 (GMT) Received: from Webmail-eu.st.com (safex1hubcas1.st.com [10.75.90.14]) by zeta.dmz-us.st.com (STMicroelectronics) with ESMTP id 44A58313; Mon, 13 Sep 2010 11:44:25 +0000 (GMT) Received: from SAFEX1MAIL2.st.com ([10.75.90.4]) by SAFEX1HUBCAS1.st.com ([10.75.90.14]) with mapi; Mon, 13 Sep 2010 13:44:14 +0200 From: Serge CHATROUX To: Tom Tromey Cc: "gdb-patches@sourceware.org" Date: Mon, 13 Sep 2010 17:41:00 -0000 Subject: RE: GDB 7.2 - Patch proposal for the use of GDB/Python scripts on MinGW Message-ID: References: <4C8DE078.4090309@st.com> In-Reply-To: <4C8DE078.4090309@st.com> x-cr-hashedpuzzle: Bmw= Aexl Dt9z D+Ff FcGT Idtm I18e KbX3 OBIM PC60 PmO+ P4QB QDGq Qj4W Sh3N UQDX;2;ZwBkAGIALQBwAGEAdABjAGgAZQBzAEAAcwBvAHUAcgBjAGUAdwBhAHIAZQAuAG8AcgBnADsAdAByAG8AbQBlAHkAQAByAGUAZABoAGEAdAAuAGMAbwBtAA==;Sosha1_v1;7;{F92EB491-B546-4E05-82FC-5B40435FE4F8};cwBlAHIAZwBlAC4AYwBoAGEAdAByAG8AdQB4AEAAcwB0AC4AYwBvAG0A;Mon, 13 Sep 2010 11:44:09 GMT;UgBFADoAIABHAEQAQgAgADcALgAyACAALQAgAFAAYQB0AGMAaAAgAHAAcgBvAHAAbwBzAGEAbAAgAGYAbwByACAAdABoAGUAIAB1AHMAZQAgAG8AZgAgAEcARABCAC8AUAB5AHQAaABvAG4AIABzAGMAcgBpAHAAdABzACAAbwBuACAATQBpAG4ARwBXAA== x-cr-puzzleid: {F92EB491-B546-4E05-82FC-5B40435FE4F8} Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 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: 2010-09/txt/msg00246.txt.bz2 > 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/CON= TRIBUTE for various other things. I will split the patch into severals. I also replied (directly) to Daniel Jacobowitz comments and wait its feedba= ck. > 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 d= llimport function and can not be copied at compilation time in a static var= iable. > Serge> + For these 3 files, I modify the source code to initialize t= he 'tp_new' fields to '0' in the static variables and to affect the proper = 'PyType_GenericNew' value in the 'gdbpy_initialize_commands', 'gdbpy_initia= lize_frames' and 'gdbpy_initialize_functions'. > Serge> + */ > Serge> + cmdpy_object_type.tp_new =3D 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 i= t 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 g= enerate 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 cod= ing standards cover this. OK. > Serge> + PyObject* PyFileObject =3D PyFile_FromString( (char*) file, = "r"); > > The spacing is different from our standard. It should look like: >=20 > PyObject *file_object =3D 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 ma= intainers. It could be great to have their feedback. > Serge> + /* Do not enable python scripting if the PYTHONHOME variable i= s undefined */ > Serge> + #ifdef HAVE_PYTHON > Serge> + if (getenv( "PYTHONHOME") =3D=3D NULL) > Serge> + { >=20 > This doesn't seem correct to me. >=20 > It definitely isn't ok for the Linux distro case. There, Python comes wi= th the system and basically nobody sets PYTHONHOME. >=20 > It isn't clear to me that it is always correct even on MinGW. Couldn't s= omeone ship a pre-built GDB plus a pre-built Python in the same tree, and e= xpect it to work properly? > > What is the failure mode here? GDB finds the python DLL but still someho= w 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.=20 - At gdb compilation time, the Python library was embed (linked with libpyt= hon2.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 v= alid Python environment.=20 Regards