From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 12001 invoked by alias); 20 Jul 2011 11:33:44 -0000 Received: (qmail 11988 invoked by uid 22791); 20 Jul 2011 11:33:42 -0000 X-SWARE-Spam-Status: No, hits=-6.8 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,SPF_HELO_PASS,TW_CP X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 20 Jul 2011 11:33:26 +0000 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p6KBXQ22006337 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Wed, 20 Jul 2011 07:33:26 -0400 Received: from localhost.localdomain (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id p6KBXOap002732; Wed, 20 Jul 2011 07:33:25 -0400 From: Phil Muldoon To: Tom Tromey Cc: gdb-patches@sourceware.org Subject: Re: [patch] [python] Prompt substitution References: Reply-to: pmuldoon@redhat.com X-URL: http://www.redhat.com Date: Wed, 20 Jul 2011 13:05:00 -0000 In-Reply-To: (Tom Tromey's message of "Tue, 19 Jul 2011 14:55:01 -0600") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii 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: 2011-07/txt/msg00516.txt.bz2 Tom Tromey writes: >>>>>> "Phil" == Phil Muldoon writes: > > Phil> +@findex gdb.get_current_prompt > Phil> +@defun get_current_prompt > Phil> +Return the current @value{GDBN} prompt as a string. > Phil> +@end defun > > Is this different from gdb.parameter('prompt')? Nope, it is just a codified API to set the prompt, and one I used in the testsuite. We can get rid of it, I have no strong feelings. > Phil> +@findex gdb.set_current_prompt > Phil> +@defun set_current_prompt @r{[}new_prompt@r{]} > Phil> +Sets the @value{GDBN} prompt to @var{new_prompt}. @var{new_prompt} > Phil> +must be a string. This method has no return value. > Phil> +@end defun > > I'm on the fence about this. It would probably be better to have a > general facility for setting parameters. See above. If it is a problem, lets just remove it. The current attached patch has these hunks removed. > Phil> +@defop Operation {@value{GDBN}} prompt_hook [current_prompt] > Phil> +If @var{prompt_hook} exists, and is not set to @code{None}, > > ... really if it is callable. > > Phil> +The parameter @code{current_prompt} contains the current @value{GDBN} > Phil> +prompt. > > If the parameter is not optional, then it should not be surrounded in [] > in the @defop. Ok to both doc changes. > Phil> + observer_notify_before_prompt (pre_gdb_prompt); > Phil> + post_gdb_prompt = get_prompt (); > Phil> + > Phil> + /* If the observer changed the prompt, use that prompt overriding > Phil> + any new_prompt that was passed to this function. */ > Phil> + if (strcmp (pre_gdb_prompt, post_gdb_prompt) != 0) > Suppose an observer sets the prompt. > Won't this free the memory pointed to by pre_gdb_prompt? > Thus making this strcmp access invalid memory? Yes you are right. We should xstrdup pre_gdb_prompt and later free it. Sigh. Sorry for bothering you with such an elementary mistake. > Phil> + if (current_gdb_prompt == NULL) > Phil> + { > > When can this happen? Never, I'll remove it. > Phil> + current_prompt = PyString_FromString (current_gdb_prompt); > > Extra space after the '='. > > Phil> + if ((result != Py_None) && (! PyString_Check (result))) > > Too many parens. > > Phil> + const char *prompt = get_prompt (); > Phil> + if (prompt) > Phil> + return PyString_FromString (prompt); > > Likewise about whether this can happen. > > Phil> + { "flush", (PyCFunction)gdbpy_flush, METH_VARARGS | METH_KEYWORDS, > Phil> + "Flush gdb's filtered stdout stream." }, This is really weird. This hunk is not in my patch on disk, or in the git tree. Anyway, yes, I will get rid of it. > This is already in the code and shouldn't be in this patch. > > Phil> + PROMPT (0) = xstrdup (s); > > Extra space. Ok to this, and other formatting nits. Current patch attached. Cheers Phil -- diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index 3a3a9fb..5b26bbb 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -21080,6 +21080,22 @@ provided, it is decoded the way that @value{GDBN}'s inbuilt @code{break} or @code{edit} commands do (@pxref{Specify Location}). @end defun +@defop Operation {@value{GDBN}} prompt_hook current_prompt +If @var{prompt_hook} is callable, @value{GDBN} will call the method +assigned to this operation before a prompt is displayed by +@value{GDBN}. + +The parameter @code{current_prompt} contains the current @value{GDBN} +prompt. This method must return a Python string, or @code{None}. If +a string is returned, the @value{GDBN} prompt will be set to that +string. If @code{None} is returned, @value{GDBN} will continue to use +the current prompt. + +Some prompts cannot be substituted in @value{GDBN}. Secondary prompts +such as those used by readline for command input, and annotation +related prompts are prohibited from being changed. +@end defop + @node Exception Handling @subsubsection Exception Handling @cindex python exceptions diff --git a/gdb/doc/observer.texi b/gdb/doc/observer.texi index d8c3924..689b303 100644 --- a/gdb/doc/observer.texi +++ b/gdb/doc/observer.texi @@ -222,6 +222,11 @@ Bytes from @var{data} to @var{data} + @var{len} have been written to the current inferior at @var{addr}. @end deftypefun +@deftypefun void before_prompt (const char *@var{current_prompt}) +Called before a top-level prompt is displayed. @var{current_prompt} is +the current top-level prompt. +@end deftypefun + @deftypefun void test_notification (int @var{somearg}) This observer is used for internal testing. Do not use. See testsuite/gdb.gdb/observer.exp. diff --git a/gdb/event-top.c b/gdb/event-top.c index 72cbfdc..21fa425 100644 --- a/gdb/event-top.c +++ b/gdb/event-top.c @@ -33,6 +33,7 @@ #include "cli/cli-script.h" /* for reset_command_nest_depth */ #include "main.h" #include "gdbthread.h" +#include "observer.h" #include "continuations.h" #include "gdbcmd.h" /* for dont_repeat() */ @@ -258,7 +259,7 @@ void display_gdb_prompt (char *new_prompt) { int prompt_length = 0; - char *gdb_prompt = get_prompt (); + char *actual_gdb_prompt = NULL; /* Reset the nesting depth used when trace-commands is set. */ reset_command_nest_depth (); @@ -268,6 +269,25 @@ display_gdb_prompt (char *new_prompt) if (!current_interp_display_prompt_p ()) return; + /* Get the prompt before the observers are called as observer hook + functions may change the prompt. Do not call observers on an + explicit prompt change as passed to this function, as this forms + a temporary prompt, IE, displayed but not set. */ + if (! new_prompt) + { + char *post_gdb_prompt = NULL; + char *pre_gdb_prompt = xstrdup (get_prompt ()); + + observer_notify_before_prompt (pre_gdb_prompt); + post_gdb_prompt = get_prompt (); + + /* If the observer changed the prompt, use that prompt. */ + if (strcmp (pre_gdb_prompt, post_gdb_prompt) != 0) + actual_gdb_prompt = post_gdb_prompt; + + xfree (pre_gdb_prompt); + } + if (sync_execution && is_running (inferior_ptid)) { /* This is to trick readline into not trying to display the @@ -289,27 +309,35 @@ display_gdb_prompt (char *new_prompt) return; } - if (!new_prompt) + /* If the observer changed the prompt, ACTUAL_GDB_PROMPT will not be + NULL. Otherwise, either copy the existing prompt, or set it to + NEW_PROMPT. */ + if (! actual_gdb_prompt) { - /* Just use the top of the prompt stack. */ - prompt_length = strlen (PREFIX (0)) + - strlen (SUFFIX (0)) + - strlen (gdb_prompt) + 1; - - new_prompt = (char *) alloca (prompt_length); - - /* Prefix needs to have new line at end. */ - strcpy (new_prompt, PREFIX (0)); - strcat (new_prompt, gdb_prompt); - /* Suffix needs to have a new line at end and \032 \032 at - beginning. */ - strcat (new_prompt, SUFFIX (0)); + if (! new_prompt) + { + /* Just use the top of the prompt stack. */ + prompt_length = strlen (PREFIX (0)) + + strlen (SUFFIX (0)) + + strlen (get_prompt()) + 1; + + actual_gdb_prompt = (char *) alloca (prompt_length); + + /* Prefix needs to have new line at end. */ + strcpy (actual_gdb_prompt, PREFIX (0)); + strcat (actual_gdb_prompt, get_prompt()); + /* Suffix needs to have a new line at end and \032 \032 at + beginning. */ + strcat (actual_gdb_prompt, SUFFIX (0)); + } + else + actual_gdb_prompt = new_prompt;; } if (async_command_editing_p) { rl_callback_handler_remove (); - rl_callback_handler_install (new_prompt, input_handler); + rl_callback_handler_install (actual_gdb_prompt, input_handler); } /* new_prompt at this point can be the top of the stack or the one passed in. It can't be NULL. */ @@ -318,7 +346,7 @@ display_gdb_prompt (char *new_prompt) /* Don't use a _filtered function here. It causes the assumed character position to be off, since the newline we read from the user is not accounted for. */ - fputs_unfiltered (new_prompt, gdb_stdout); + fputs_unfiltered (actual_gdb_prompt, gdb_stdout); gdb_flush (gdb_stdout); } } diff --git a/gdb/python/python.c b/gdb/python/python.c index f15936d..e3a8d19 100644 --- a/gdb/python/python.c +++ b/gdb/python/python.c @@ -51,6 +51,7 @@ static int gdbpy_should_print_stack = 0; #include "version.h" #include "target.h" #include "gdbthread.h" +#include "observer.h" static PyMethodDef GdbMethods[]; @@ -682,6 +683,81 @@ gdbpy_initialize_events (void) } } + + +static void +before_prompt_hook (const char *current_gdb_prompt) +{ + struct cleanup *cleanup; + char *prompt = NULL; + + cleanup = ensure_python_env (get_current_arch (), current_language); + + if (PyObject_HasAttrString (gdb_module, "prompt_hook")) + { + PyObject *hook; + + hook = PyObject_GetAttrString (gdb_module, "prompt_hook"); + if (hook == NULL) + goto fail; + + if (PyCallable_Check (hook)) + { + PyObject *result; + PyObject *current_prompt; + + current_prompt = PyString_FromString (current_gdb_prompt); + if (current_prompt == NULL) + goto fail; + + result = PyObject_CallFunctionObjArgs (hook, current_prompt, NULL); + + Py_DECREF (current_prompt); + + if (result == NULL) + goto fail; + + make_cleanup_py_decref (result); + + /* Return type should be None, or a String. If it is None, + fall through, we will not set a prompt. If it is a + string, set PROMPT. Anything else, set an exception. */ + if (result != Py_None && ! PyString_Check (result)) + { + PyErr_Format (PyExc_RuntimeError, + _("Return from prompt_hook must " \ + "be either a Python string, or None")); + goto fail; + } + + if (result != Py_None) + { + prompt = python_string_to_host_string (result); + + if (prompt == NULL) + goto fail; + else + make_cleanup (xfree, prompt); + } + } + } + + /* If a prompt has been set, PROMPT will not be NULL. If it is + NULL, do not set the prompt. */ + if (prompt != NULL) + set_prompt (prompt); + + do_cleanups (cleanup); + return; + + fail: + gdbpy_print_stack (); + do_cleanups (cleanup); + return; +} + + + /* Printing. */ /* A python function to write a single string using gdb's filtered @@ -1134,6 +1210,8 @@ Enables or disables printing of Python stack traces."), gdbpy_initialize_exited_event (); gdbpy_initialize_thread_event (); + observer_attach_before_prompt (before_prompt_hook); + PyRun_SimpleString ("import gdb"); PyRun_SimpleString ("gdb.pretty_printers = []"); @@ -1236,6 +1314,8 @@ def GdbSetPythonDirectory (dir):\n\ \n\ # Install the default gdb.PYTHONDIR.\n\ GdbSetPythonDirectory (gdb.PYTHONDIR)\n\ +# Default prompt hook does nothing.\n\ +prompt_hook = None\n\ "); do_cleanups (cleanup); diff --git a/gdb/testsuite/gdb.python/python.exp b/gdb/testsuite/gdb.python/python.exp index 8906bbc..832afc0 100644 --- a/gdb/testsuite/gdb.python/python.exp +++ b/gdb/testsuite/gdb.python/python.exp @@ -193,3 +193,85 @@ gdb_py_test_silent_cmd "set python print-stack on" \ "Test print-backtrace set setting" 1 gdb_test "show python print-stack" \ "Whether Python stack will be printed on error is on.*" \ + +# Test prompt substituion + +gdb_py_test_multiple "prompt substitution" \ + "python" "" \ + "someCounter = 0" "" \ + "def prompt(current):" "" \ + " global someCounter" "" \ + " if (current == \"testfake \"):" "" \ + " return None" "" \ + " someCounter = someCounter + 1" "" \ + " return \"py prompt \" + str (someCounter) + \" \"" "" \ + "end" "" + +gdb_py_test_multiple "prompt substitution readline" \ + "python" "" \ + "pCounter = 0" "" \ + "def program_prompt(current):" "" \ + " global pCounter" "" \ + " if (current == \">\"):" "" \ + " pCounter = pCounter + 1" "" \ + " return \"python line \" + str (pCounter) + \": \"" "" \ + " return None" "" \ + "end" "" + +set newprompt "py prompt 1" +set newprompt2 "py prompt 2" +set testfake "testfake" + +gdb_test_multiple "python gdb.prompt_hook = prompt" "set the hook" { + -re "\[\r\n\]$newprompt $" { + pass "set hook" + } +} + +gdb_test_multiple "set prompt testfake " "set testfake prompt in GDB" { + -re "\[\r\n\]$testfake $" { + pass "set prompt testfake" + } +} + +gdb_test_multiple "show prompt" "show testfake prompt" { + -re "Gdb's prompt is \"$testfake \"..* $" { + pass "show prompt shows guarded prompt" + } +} + +gdb_test_multiple "set prompt blah " "set blah in GDB" { + -re "\[\r\n\]$newprompt2 $" { + pass "set prompt blah overriden" + } +} + +gdb_test_multiple "python gdb.prompt_hook = None" "Delete hook" { + -re "\[\r\n\]$newprompt2 $" { + pass "Delete old hook" + } +} + +gdb_test_multiple "set prompt $gdb_prompt " "set default prompt" { + -re "\[\r\n\]$gdb_prompt $" { + pass "set default prompt" + } +} + +gdb_test_multiple "python gdb.prompt_hook = program_prompt" "set the hook" { + -re "\[\r\n\]$gdb_prompt $" { + pass "set programming hook" + } +} + +gdb_test_multiple "python" "test we ignore substituion for seconday prompts" { + -re "\r\n>$" { + pass "readline secondary are not substituted" + } +} + +gdb_test_multiple "end" "end programming" { + -re "\[\r\n\]$gdb_prompt $" { + pass "end programming" + } +} diff --git a/gdb/top.c b/gdb/top.c index ecb91f2..80d094d 100644 --- a/gdb/top.c +++ b/gdb/top.c @@ -1133,14 +1133,21 @@ get_prompt (void) } void -set_prompt (char *s) +set_prompt (const char *s) { -/* ??rehrauer: I don't know why this fails, since it looks as though - assignments to prompt are wrapped in calls to xstrdup... - if (prompt != NULL) - xfree (prompt); - */ - PROMPT (0) = xstrdup (s); + /* If S == PROMPT then do not free it or set it. If we did + that, S (which points to PROMPT), would become garbage. */ + + if (s != PROMPT (0)) + { + xfree (PROMPT (0)); + PROMPT (0) = xstrdup (s); + + /* Also, free and set new_async_prompt so prompt changes sync up + with set/show prompt. */ + xfree (new_async_prompt); + new_async_prompt = xstrdup (PROMPT (0)); + } } diff --git a/gdb/top.h b/gdb/top.h index 4a6e8fb..24ec2f2 100644 --- a/gdb/top.h +++ b/gdb/top.h @@ -55,7 +55,7 @@ extern char *get_prompt (void); /* This function copies the specified string into the string that is used by gdb for its command prompt. */ -extern void set_prompt (char *); +extern void set_prompt (const char *); /* From random places. */ extern int readnow_symbol_files;