From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 27595 invoked by alias); 11 Jan 2012 10:27:04 -0000 Received: (qmail 27585 invoked by uid 22791); 11 Jan 2012 10:27:01 -0000 X-SWARE-Spam-Status: No, hits=-2.1 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,TW_CP X-Spam-Check-By: sourceware.org Received: from mail-vw0-f41.google.com (HELO mail-vw0-f41.google.com) (209.85.212.41) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 11 Jan 2012 10:26:47 +0000 Received: by vbbfn1 with SMTP id fn1so444736vbb.0 for ; Wed, 11 Jan 2012 02:26:46 -0800 (PST) Received: by 10.52.18.144 with SMTP id w16mr3935244vdd.94.1326277606136; Wed, 11 Jan 2012 02:26:46 -0800 (PST) MIME-Version: 1.0 Received: by 10.220.3.130 with HTTP; Wed, 11 Jan 2012 02:26:25 -0800 (PST) In-Reply-To: References: From: Kevin Pouget Date: Wed, 11 Jan 2012 10:43:00 -0000 Message-ID: Subject: Re: Make the "python" command resemble the standard Python interpreter To: Khoo Yit Phang Cc: gdb-patches@sourceware.org Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable 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: 2012-01/txt/msg00348.txt.bz2 On Wed, Jan 11, 2012 at 1:18 AM, Khoo Yit Phang wrote: > Hi, > > I'd like to contribute a patch to improve the "python" command by making = it resemble the standard Python interpreter in behavior. > > For "python" with arguments, this prints the results of expressions, e.g.= , "python 1 + 2" will print "3" (previously, it will not print anything). > > For "python" without arguments, this uses Python's built-in interactive l= oop (PyRun_InteractiveLoop) so that individual statements/expressions are i= mmediately evaluated and results of expressions are printed. It also hooks = GDB's readline wrappers (command_line_input) to Python so that line editing= and history editing works. Additionally, Python's standard readline module= is stubbed out because it conflicts with GDB's use of readline. > > This patch depends on a patch to handle SIGINT that I previously submitte= d (http://sourceware.org/bugzilla/show_bug.cgi?id=3D13265) to handle SIGINT= in the interactive loop. > > Thanks! > > Yit > January 10, 2012 > Hello, it sounds interesting indeed, I dropped below a few comments. I'm not a maintainer, so that's nothing more than suggestions > gdb/ChangeLog: > > 2012-01-10 =A0Khoo Yit Phang > > Make the "python" command resemble the standard Python interpreter. > * Makefile.in (SUBDIR_PYTHON_OBS): Add py-gdb-readline.o. > (SUBDIR_PYTHON_SRCS): Add python/py-gdb-readline.c. > (py-gdb-readline.o): Add rule to compile python/py-gdb-readline.c. > * python/py-gdb-readline.c: New file. > * python/python-internal.h (gdbpy_initialize_gdb_readline): New > prototype. > * python/python.c (eval_python_command): New function. > (eval_python_from_control_command): Call eval_python_command instead > of PyRun_SimpleString. > (python_command): For "python" with arguments, call > eval_python_command. =A0For "python" without arguments, call > PyRun_InteractiveLoop if from_tty is true, other call > execute_control_command_untraced. > > diff --git a/gdb/Makefile.in b/gdb/Makefile.in > --- a/gdb/Makefile.in > +++ b/gdb/Makefile.in > @@ -270,6 +270,7 @@ > # > SUBDIR_PYTHON_OBS =3D \ > python.o \ > + py-gdb-readline.o \ > py-auto-load.o \ > py-block.o \ > py-bpevent.o \ > @@ -302,6 +303,7 @@ > > SUBDIR_PYTHON_SRCS =3D \ > python/python.c \ > + python/py-gdb-readline.c \ > python/py-auto-load.c \ > python/py-block.c \ > python/py-bpevent.c \ name should be kept in alphabetical order, please. The same applies for the entries below. > @@ -2019,6 +2021,10 @@ > $(COMPILE) $(PYTHON_CFLAGS) $(srcdir)/python/python.c > $(POSTCOMPILE) > > +py-gdb-readline.o: $(srcdir)/python/py-gdb-readline.c > + $(COMPILE) $(PYTHON_CFLAGS) $(srcdir)/python/py-gdb-readline.c > + $(POSTCOMPILE) > + > py-auto-load.o: $(srcdir)/python/py-auto-load.c > $(COMPILE) $(PYTHON_CFLAGS) $(srcdir)/python/py-auto-load.c > $(POSTCOMPILE) > diff --git a/gdb/python/py-gdb-readline.c b/gdb/python/py-gdb-readline.c > new file mode 100644 > --- /dev/null > +++ b/gdb/python/py-gdb-readline.c > @@ -0,0 +1,168 @@ > +/* Readline support for Python. > + > + =A0 Copyright (C) 2012 Free Software Foundation, Inc. > + > + =A0 This file is part of GDB. > + > + =A0 This program is free software; you can redistribute it and/or modify > + =A0 it under the terms of the GNU General Public License as published by > + =A0 the Free Software Foundation; either version 3 of the License, or > + =A0 (at your option) any later version. > + > + =A0 This program is distributed in the hope that it will be useful, > + =A0 but WITHOUT ANY WARRANTY; without even the implied warranty of > + =A0 MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. =A0See the > + =A0 GNU General Public License for more details. > + > + =A0 You should have received a copy of the GNU General Public License > + =A0 along with this program. =A0If not, see . =A0*/ > + > +#include "defs.h" > +#include "python-internal.h" > +#include "../exceptions.h" > +#include "../top.h" according to other py-*.c files, you don't need the '../' > +#include > +#include > + > +/* Python's readline module conflicts with GDB's use of readline > + =A0 since readline is not reentrant. To prevent conflicts, a dummy > + =A0 readline module is set up which simply fails for any readline > + =A0 method. */ Two spaces after the dots "reentrant. To prevent", "method. */", and the same in further comments > +/* TODO: Ideally, this module should implement a reentrant wrapper > + =A0 to readline, so that the readline module can be used without > + =A0 conflicting with GDB. */ I think TODO will not be accepted in the final patch. > +static PyObject * > +unimplemented_command (PyObject *self, PyObject *args, PyObject *kw) > +{ > + =A0PyErr_SetString (PyExc_NotImplementedError, > + =A0 "readline module disabled under GDB"); > + =A0return NULL; > +} > + > +#define UNIMPLEMENTED(cmd) \ > + =A0{ cmd, (PyCFunction)unimplemented_command, \ > + =A0 =A0METH_VARARGS | METH_KEYWORDS, "" } space after the '('. The same applies below, and for all the function calls: > function (...); > (cast) var; > +static PyMethodDef GdbReadlineMethods[] =3D > +{ > + =A0/* readline module methods as of Python 2.7.2. */ > + =A0UNIMPLEMENTED("add_history"), > + =A0UNIMPLEMENTED("clear_history"), > + =A0UNIMPLEMENTED("get_begidx"), > + =A0UNIMPLEMENTED("get_completer"), > + =A0UNIMPLEMENTED("get_completer_delims"), > + =A0UNIMPLEMENTED("get_completion_type"), > + =A0UNIMPLEMENTED("get_current_history_length"), > + =A0UNIMPLEMENTED("get_endidx"), > + =A0UNIMPLEMENTED("get_history_item"), > + =A0UNIMPLEMENTED("get_history_length"), > + =A0UNIMPLEMENTED("get_line_buffer"), > + =A0UNIMPLEMENTED("insert_text"), > + =A0UNIMPLEMENTED("parse_and_bind"), > + =A0UNIMPLEMENTED("read_history_file"), > + =A0UNIMPLEMENTED("read_init_file"), > + =A0UNIMPLEMENTED("redisplay"), > + =A0UNIMPLEMENTED("remove_history_item"), > + =A0UNIMPLEMENTED("replace_history_item"), > + =A0UNIMPLEMENTED("set_completer"), > + =A0UNIMPLEMENTED("set_completer_delims"), > + =A0UNIMPLEMENTED("set_completion_display_matches_hook"), > + =A0UNIMPLEMENTED("set_history_length"), > + =A0UNIMPLEMENTED("set_pre_input_hook"), > + =A0UNIMPLEMENTED("set_startup_hook"), > + =A0UNIMPLEMENTED("write_history_file"), > + =A0{ NULL, NULL, 0, NULL } > +}; > + > +/* Readline function suitable for PyOS_ReadlineFunctionPointer, which > + =A0 is used for Python's interactive parser and raw_input. In both > + =A0 cases, sys_stdin and sys_stdout are always stdin and stdout > + =A0 respectively, as far as I can tell; they are ignored and > + =A0 command_line_input is used instead. */ > + > +static char * > +gdbpy_readline_wrapper (FILE *sys_stdin, FILE *sys_stdout, > + char *prompt) I think that 'char *prompt' whould be aligned with FILE *sys_stdin' > +{ > + =A0int n; > + =A0char *p =3D NULL, *p_start, *p_end, *q; > + =A0volatile struct gdb_exception except; > + > + =A0TRY_CATCH (except, RETURN_MASK_ALL) > + =A0 =A0{ > + =A0 =A0 =A0struct cleanup *cleanup =3D gdbpy_suspend_sigint_handler (); new line between declarations and the code > + =A0 =A0 =A0p =3D command_line_input (prompt, 0, "python"); > + =A0 =A0 =A0do_cleanups (cleanup); > + =A0 =A0} I'm not sure about that, but isn't the clean up supposed to be executed even if an exception is thrown? it seems not to be the case here > + =A0/* Detect Ctrl-C and treat as KeyboardInterrupt. */ > + =A0if (except.reason =3D=3D RETURN_QUIT) > + =A0 =A0return NULL; > + > + =A0/* Handle errors by raising Python exceptions. */ > + =A0if (except.reason < 0) > + =A0 =A0{ > + =A0 =A0 =A0/* The thread state is nulled during gdbpy_readline_wrapper, > + with the original value saved in the following undocumented > + variable (see Python's Parser/myreadline.c and > + Modules/readline.c). */ comment lines should be aligned with "The thread" (or maybe my tabs are not displayed properly) > + =A0 =A0 =A0PyEval_RestoreThread (_PyOS_ReadlineTState); > + =A0 =A0 =A0gdbpy_convert_exception (except); > + =A0 =A0 =A0PyEval_SaveThread (); > + =A0 =A0 =A0return NULL; > + =A0 =A0} > + > + =A0/* Detect EOF (Ctrl-D). */ > + =A0if (p =3D=3D NULL) > + =A0 =A0{ > + =A0 =A0 =A0q =3D PyMem_Malloc (1); > + =A0 =A0 =A0if (q !=3D NULL) > + q[0] =3D '\0'; > + =A0 =A0 =A0return q; > + =A0 =A0} > + > + =A0n =3D strlen (p); > + > + =A0/* Detect "end" like process_next_line in cli/cli-script.c. */ > + =A0/* Strip trailing whitespace. =A0*/ > + =A0p_end =3D p + n; > + =A0while (p_end > p && (p_end[-1] =3D=3D ' ' || p_end[-1] =3D=3D '\t')) > + =A0 =A0p_end--; > + > + =A0/* Strip leading whitespace. =A0*/ > + =A0p_start =3D p; > + =A0while (p_start < p_end && (*p_start =3D=3D ' ' || *p_start =3D=3D '\= t')) > + =A0 =A0p_start++; maybe clu-utils.h remove_trailing_whitespace and skip_spaces already do the same? > + =A0/* Treat "end" as EOF. */ > + =A0if (p_end - p_start =3D=3D 3 && !strncmp (p_start, "end", 3)) > + =A0 =A0{ > + =A0 =A0 =A0q =3D PyMem_Malloc (1); > + =A0 =A0 =A0if (q !=3D NULL) > + q[0] =3D '\0'; > + =A0 =A0 =A0return q; > + =A0 =A0} > + > + =A0/* Copy the line to Python and return. */ > + =A0q =3D PyMem_Malloc (n + 2); > + =A0if (q !=3D NULL) > + =A0 =A0{ > + =A0 =A0 =A0strncpy (q, p, n); > + =A0 =A0 =A0q[n] =3D '\n'; > + =A0 =A0 =A0q[n + 1] =3D '\0'; > + =A0 =A0} > + =A0return q; > +} > + > +void > +gdbpy_initialize_gdb_readline (void) all functions should have a comment, but I'm not sure whether it should be in the C or H file > +{ > + =A0Py_InitModule3 ("readline", GdbReadlineMethods, > + =A0"GDB-provided dummy readline module to prevent conflicts with the st= andard readline module."); This line is too long, should be < 80 chars > + > + =A0PyOS_ReadlineFunctionPointer =3D gdbpy_readline_wrapper; > +} > + > diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h > --- a/gdb/python/python-internal.h > +++ b/gdb/python/python-internal.h > @@ -234,6 +234,7 @@ > struct symtab_and_line *sal_object_to_symtab_and_line (PyObject *obj); > struct frame_info *frame_object_to_frame_info (PyObject *frame_obj); > > +void gdbpy_initialize_gdb_readline (void); > void gdbpy_initialize_auto_load (void); > void gdbpy_initialize_values (void); > void gdbpy_initialize_frames (void); > diff --git a/gdb/python/python.c b/gdb/python/python.c > --- a/gdb/python/python.c > +++ b/gdb/python/python.c > @@ -279,6 +279,43 @@ > =A0 return script; > } > > +/* Evaluate a Python command like PyRun_SimpleString, but uses > + =A0 Py_single_input which prints the result of expressions if the input > + =A0 is from tty, and Py_file_input otherwise. */ > + > +static void > +eval_python_command (const char *command, int from_tty) > +{ > + =A0struct cleanup *cleanup; > + =A0PyObject *m, *d, *v; > + > + =A0cleanup =3D ensure_python_env (get_current_arch (), current_language= ); > + > + =A0m =3D PyImport_AddModule ("__main__"); > + =A0if (m =3D=3D NULL) > + =A0 =A0error (_("Error while executing Python code.")); > + > + =A0d =3D PyModule_GetDict (m); > + =A0v =3D PyRun_StringFlags (command, > + from_tty ? Py_single_input : Py_file_input, > + d, d, NULL); > + =A0if (v =3D=3D NULL) > + =A0 =A0{ > + =A0 =A0 =A0int interrupt =3D PyErr_ExceptionMatches (PyExc_KeyboardInte= rrupt); blank line > + =A0 =A0 =A0PyErr_Print (); > + =A0 =A0 =A0if (! interrupt) > + error (_("Error while executing Python code.")); > + =A0 =A0} > + =A0else > + =A0 =A0{ > + =A0 =A0 =A0Py_DECREF (v); > + =A0 =A0 =A0if (Py_FlushLine ()) > + PyErr_Clear (); > + =A0 =A0} > + > + =A0do_cleanups (cleanup); > +} > + > /* Take a command line structure representing a 'python' command, and > =A0 =A0evaluate its body using the Python interpreter. =A0*/ > > @@ -292,13 +329,10 @@ > =A0 if (cmd->body_count !=3D 1) > =A0 =A0 error (_("Invalid \"python\" block structure.")); > > - =A0cleanup =3D ensure_python_env (get_current_arch (), current_language= ); > + =A0script =3D compute_python_string (cmd->body_list[0]); > + =A0cleanup =3D make_cleanup (xfree, script); > > - =A0script =3D compute_python_string (cmd->body_list[0]); > - =A0ret =3D PyRun_SimpleString (script); > - =A0xfree (script); > - =A0if (ret) > - =A0 =A0error (_("Error while executing Python code.")); > + =A0eval_python_command (script, 0); > > =A0 do_cleanups (cleanup); > } > @@ -316,18 +350,26 @@ > =A0 while (arg && *arg && isspace (*arg)) > =A0 =A0 ++arg; > =A0 if (arg && *arg) > - =A0 =A0{ > - =A0 =A0 =A0ensure_python_env (get_current_arch (), current_language); > - > - =A0 =A0 =A0if (PyRun_SimpleString (arg)) > - error (_("Error while executing Python code.")); > - =A0 =A0} > + =A0 =A0eval_python_command (arg, from_tty); > =A0 else > =A0 =A0 { > - =A0 =A0 =A0struct command_line *l =3D get_command_line (python_control,= ""); > - > - =A0 =A0 =A0make_cleanup_free_command_lines (&l); > - =A0 =A0 =A0execute_control_command_untraced (l); > + =A0 =A0 =A0if (from_tty) > + { > + =A0int err; > + =A0ensure_python_env (get_current_arch (), current_language); > + =A0err =3D PyRun_InteractiveLoop(instream, ""); > + =A0dont_repeat (); > + =A0if (err && ! PyErr_ExceptionMatches (PyExc_KeyboardInterrupt)) > + =A0 =A0error(_("Error while executing Python code.")); > + } > + =A0 =A0 =A0else > + { > + =A0/* TODO: perhaps PyRun_FileExFlags can be used, which would > + =A0 =A0 simplify cli/cli-script.c. */ > + =A0struct command_line *l =3D get_command_line (python_control, ""); > + =A0make_cleanup_free_command_lines (&l); > + =A0execute_control_command_untraced (l); > + } > =A0 =A0 } > > =A0 do_cleanups (cleanup); > @@ -1280,6 +1322,7 @@ > =A0 gdbpy_gdberror_exc =3D PyErr_NewException ("gdb.GdbError", NULL, NULL= ); > =A0 PyModule_AddObject (gdb_module, "GdbError", gdbpy_gdberror_exc); > > + =A0gdbpy_initialize_gdb_readline (); > =A0 gdbpy_initialize_auto_load (); > =A0 gdbpy_initialize_values (); > =A0 gdbpy_initialize_frames (); and you can maybe add some regression tests, if there's a way to test it and update the 'python' documentation Cordially, Kevin