From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 32714 invoked by alias); 18 Jul 2008 19:50:33 -0000 Received: (qmail 32703 invoked by uid 22791); 18 Jul 2008 19:50:33 -0000 X-Spam-Check-By: sourceware.org Received: from NaN.false.org (HELO nan.false.org) (208.75.86.248) by sourceware.org (qpsmtpd/0.31) with ESMTP; Fri, 18 Jul 2008 19:50:13 +0000 Received: from nan.false.org (localhost [127.0.0.1]) by nan.false.org (Postfix) with ESMTP id 75F2A982C3; Fri, 18 Jul 2008 19:50:11 +0000 (GMT) Received: from caradoc.them.org (22.svnf5.xdsl.nauticom.net [209.195.183.55]) by nan.false.org (Postfix) with ESMTP id 0790A98015; Fri, 18 Jul 2008 19:50:10 +0000 (GMT) Received: from drow by caradoc.them.org with local (Exim 4.69) (envelope-from ) id 1KJvxy-0005X6-7F; Fri, 18 Jul 2008 15:50:10 -0400 Date: Fri, 18 Jul 2008 19:50:00 -0000 From: Daniel Jacobowitz To: Thiago Jung Bauermann Cc: Tom Tromey , gdb-patches ml Subject: Re: [RFA] Re: [RFC][patch 1/9] initial Python support Message-ID: <20080718195010.GA14356@caradoc.them.org> Mail-Followup-To: Thiago Jung Bauermann , Tom Tromey , gdb-patches ml References: <20080429155212.444237503@br.ibm.com> <20080429155304.288626880@br.ibm.com> <20080528205921.GA2969@caradoc.them.org> <20080615181833.uxmo25mg0kko40kw@imap.linux.ibm.com> <1216107418.14956.27.camel@localhost.localdomain> <1216245620.12209.18.camel@localhost.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1216245620.12209.18.camel@localhost.localdomain> User-Agent: Mutt/1.5.17 (2008-05-11) X-IsSubscribed: yes 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: 2008-07/txt/msg00367.txt.bz2 On Wed, Jul 16, 2008 at 07:00:20PM -0300, Thiago Jung Bauermann wrote: > +# > +# python sub directory definitons > +# > +SUBDIR_PYTHON_OBS = \ > + python-utils.o > +SUBDIR_PYTHON_SRCS = \ > + python/python.c \ > + python/utils.c Can we call it python-utils.c or something else, instead? IMO having different naming for the source file and object is very confusing. > @@ -1283,7 +1303,11 @@ test-cp-name-parser$(EXEEXT): test-cp-name-parser.o $(LIBIBERTY) > # duplicates. Files in the gdb/ directory can end up appearing in > # COMMON_OBS (as a .o file) and CONFIG_SRCS (as a .c file). > > -INIT_FILES = $(COMMON_OBS) $(TSOBS) $(CONFIG_SRCS) > +# NOTE: bauermann/2008-06-15: python.c needs to be explicitly included > +# because the generation of init.c does not work for .c files which > +# result in differently named objects (i.e., python/python -> python.o). > + > +INIT_FILES = $(COMMON_OBS) $(TSOBS) $(CONFIG_SRCS) python/python.c This comment doesn't make sense. python/python.c is already included in INIT_FILES, because of CONFIG_SRCS. > +# Flags needed to compile Python code (taken from python-config --cflags) > +PYTHON_CFLAGS=-fno-strict-aliasing -DNDEBUG -fwrapv The output of python-config will vary by platform (that's the point of having a helper program). So, I'm afraid we have to run python-config if we need the cflags. Or else we could add these flags unconditionally, if they work with the current compiler. Consider non-GCC. > @@ -538,6 +560,17 @@ execute_control_command (struct command_line *cmd) > return ret; > } > > +/* Like execute_control_command, but first set > + suppress_next_print_command_trace. */ > + > +enum command_control_type > +execute_control_command_suppressed (struct command_line *cmd) > +{ > + suppress_next_print_command_trace = 1; > + return execute_control_command (cmd); > +} > + > + > /* "while" command support. Executes a body of statements while the > loop condition is nonzero. */ > This is an unclear name - suppressed what? Maybe execute_control_command_untraced is more accurate. > diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo > index 2920179..b1419a7 100644 > --- a/gdb/doc/gdb.texinfo > +++ b/gdb/doc/gdb.texinfo > @@ -165,6 +165,7 @@ software in general. We will miss him. > * Emacs:: Using @value{GDBN} under @sc{gnu} Emacs > * GDB/MI:: @value{GDBN}'s Machine Interface. > * Annotations:: @value{GDBN}'s annotation interface. > +* Python:: Scripting @value{GDBN} using Python. > > * GDB Bugs:: Reporting bugs in @value{GDBN} > We should ask Eli to look at the manual. > +@kindex set python-stack > +By default, @value{GDBN} will print a stack trace when an error occurs > +in a Python script. This can be controlled using @code{set python-stack}. Please expand this; see e.g. Print Settings for the usual way we document commands. In particular it should say what the values are/mean. Also, perhaps this should use a prefix command, like "set python print-stack"? There should probably be an explicit table item for the "python" command, too. > +/* Python 2.4 doesn't include stdint.h soon enough to get {u,}intptr_t > + needed by pyport.h. */ > +#ifdef HAVE_STDINT_H > +#include > +#endif You can include stdint.h unconditionally now; gnulib provides it. > +/* /usr/include/features.h on linux systems will define _POSIX_C_SOURCE > + if it sees _GNU_SOURCE (which config.h will define). > + pyconfig.h defines _POSIX_C_SOURCE to a different value than > + /usr/include/features.h does causing compilation to fail. > + To work around this, undef _POSIX_C_SOURCE before we include Python.h. */ > +#undef _POSIX_C_SOURCE Odd. For me, pyconfig.h and features.h define it to the same thing. Anyway, this is gross but acceptable. > +static PyMethodDef GdbMethods[] = > +{ > + { "execute", execute_gdb_command, METH_VARARGS, > + "Execute a gdb command" }, > + { "show", get_show_variable, METH_VARARGS, > + "Return a gdb setting's value" }, > + > + { "write", gdbpy_write, METH_VARARGS, > + "Write a string using gdb's filtered stream." }, > + { "flush", gdbpy_flush, METH_NOARGS, > + "Flush gdb's filtered stdout stream." }, > + > + {NULL, NULL, 0, NULL} > +}; I think that these are now "gdb commands", effectively. So, sorry, but there's a bit more documentation you get to write :-) No new undocumented public interfaces. > + /* Create a couple objects which are used for Python's stdout and > + stderr. */ > + PyRun_SimpleString ("\ > +import sys\n\ > +class GdbOutputFile:\n\ > + def close(self):\n\ > + # Do nothing.\n\ > + return None\n\ > +\n\ > + def isatty(self):\n\ > + return False\n\ > +\n\ > + def write(self, s):\n\ > + gdb.write(s)\n\ > +\n\ > + def writelines(self, iterable):\n\ > + for line in iterable:\n\ > + self.write(line)\n\ > +\n\ > + def flush(self):\n\ > + gdb.flush()\n\ > +\n\ > +sys.stderr = GdbOutputFile()\n\ > +sys.stdout = GdbOutputFile()\n\ This is OK for now, but it is likely to grow. There's code to convert a text file into a C string, in features/ (used for XML files); maybe we should internalize a couple of Python source files that way? Alternatively we could install Python source files to a known path in the build / install directories; that lets vendors precompile the files for faster startup. Anyway, that's a future note, not a current problem. > +/* Initialize dummy Python code. */ > + > +void > +_initialize_python (void) > +{ > + add_com ("python", class_obscure, python_command, _("\ > +Evaluate a Python command.\n\ > +\n\ > +Python scripting is not supported in this copy of GDB.\n\ > +This command is only a placeholder.")); > + > + add_setshow_boolean_cmd ("python-stack", class_maintenance, > + &gdbpy_should_print_stack, _("\ > +Set Python stack printing."), _("\ > +Show Python stack printing."), _("\ > +Enables printing of Python stack traces."), > + NULL, NULL, > + &maintenance_set_cmdlist, > + &maintenance_show_cmdlist); > + > +} If the list of python configuration variables grows, this will get out of hand; I suggest sharing the init code regardless of HAVE_PYTHON. This patch is enough for a first NEWS entry. Otherwise OK :-) -- Daniel Jacobowitz CodeSourcery