Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [patch] python prompt additions at first prompt.
@ 2011-07-30 22:20 Matt Rice
  2011-08-01  9:51 ` Phil Muldoon
  2011-08-03 18:08 ` Tom Tromey
  0 siblings, 2 replies; 20+ messages in thread
From: Matt Rice @ 2011-07-30 22:20 UTC (permalink / raw)
  To: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 1653 bytes --]

little bug with the new python prompt stuff, the prompt_hook doesn't
take effect until the second prompt.

I couldn't for the life of me figure out how to do a testcase for this...
using gdb.mi/mi-async.exp as an example of using -ex it still didn't
want to work,
there was also the need to match the very first prompt.
and command line python stuff seems to go awry of how skip_python_tests works.
sorry.

before:
gdb/gdb -quiet -ex 'set prompt (foo) ' -ex 'python def prompt(x):
return "(bar) "' -ex 'python gdb.prompt_hook = prompt'
(foo)
(bar) quit

~/git/gdb/gdb/gdb -quiet -ex 'python def prompt(x): return "(bar) "'
-ex 'python gdb.prompt_hook = prompt'
(gdb)
(bar) quit


after:
gdb/gdb -quiet -ex 'set prompt (foo) ' -ex 'python def prompt(x):
return "(bar) "' -ex 'python gdb.prompt_hook = prompt'
(bar)
(bar) quit

gdb/gdb -quiet -ex 'python def prompt(x): return "(bar) "' -ex 'python
gdb.prompt_hook = prompt'
(bar)
(bar) quit

ran the above things with --enable-tui --disable-tui plus gdb --interpreter=tui
and ran the testsuite with --enable-tui and --disable-tui.
didn't see anything but what looked like tests that fail randomly.

I think it's ok anyways, the 'if (async_command_editing_p)' case in
display_gdb_prompt seems to cover it,
with the addition of rl_callback_handler_remove() which afaict seemed
ok to call before a handler is installed

otherwise, there is a workaround
calling gdb.execute("set prompt " + prompt("foo"))

Thanks.

2011-07-30  Matt Rice  <ratmice@gmail.com>

        * event-top.c (cli_command_loop): Remove code redundant to
        display_gdb_prompt.
        * tui/tui-interp.c (tui_command_loop): Ditto.

[-- Attachment #2: foo.diff --]
[-- Type: text/x-patch, Size: 2351 bytes --]

diff --git a/gdb/event-top.c b/gdb/event-top.c
index 37882728..fb0478f 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -185,27 +185,7 @@ rl_callback_read_char_wrapper (gdb_client_data client_data)
 void
 cli_command_loop (void)
 {
-  /* If we are using readline, set things up and display the first
-     prompt, otherwise just print the prompt.  */
-  if (async_command_editing_p)
-    {
-      int length;
-      char *a_prompt;
-      char *gdb_prompt = get_prompt (0);
-
-      /* Tell readline what the prompt to display is and what function
-         it will need to call after a whole line is read.  This also
-         displays the first prompt.  */
-      length = strlen (get_prefix (0))
-	+ strlen (gdb_prompt) + strlen (get_suffix(0)) + 1;
-      a_prompt = (char *) alloca (length);
-      strcpy (a_prompt, get_prefix (0));
-      strcat (a_prompt, gdb_prompt);
-      strcat (a_prompt, get_suffix (0));
-      rl_callback_handler_install (a_prompt, input_handler);
-    }
-  else
-    display_gdb_prompt (0);
+  display_gdb_prompt (0);
 
   /* Now it's time to start the event loop.  */
   start_event_loop ();
diff --git a/gdb/tui/tui-interp.c b/gdb/tui/tui-interp.c
index 919d1ac..9e34412 100644
--- a/gdb/tui/tui-interp.c
+++ b/gdb/tui/tui-interp.c
@@ -139,27 +139,7 @@ tui_exec (void *data, const char *command_str)
 static void
 tui_command_loop (void *data)
 {
-  /* If we are using readline, set things up and display the first
-     prompt, otherwise just print the prompt.  */
-  if (async_command_editing_p)
-    {
-      int length;
-      char *a_prompt;
-      char *gdb_prompt = get_prompt (0);
-
-      /* Tell readline what the prompt to display is and what function
-         it will need to call after a whole line is read. This also
-         displays the first prompt.  */
-      length = strlen (get_prefix (0))
-	+ strlen (gdb_prompt) + strlen (get_suffix (0)) + 1;
-      a_prompt = (char *) alloca (length);
-      strcpy (a_prompt, get_prefix (0));
-      strcat (a_prompt, gdb_prompt);
-      strcat (a_prompt, get_suffix (0));
-      rl_callback_handler_install (a_prompt, input_handler);
-    }
-  else
-    display_gdb_prompt (0);
+  display_gdb_prompt (0);
 
   /* Loop until there is nothing to do. This is the entry point to the
      event loop engine. gdb_do_one_event, called via catch_errors()

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch] python prompt additions at first prompt.
  2011-07-30 22:20 [patch] python prompt additions at first prompt Matt Rice
@ 2011-08-01  9:51 ` Phil Muldoon
  2011-08-01 14:08   ` Matt Rice
  2011-08-03 18:08 ` Tom Tromey
  1 sibling, 1 reply; 20+ messages in thread
From: Phil Muldoon @ 2011-08-01  9:51 UTC (permalink / raw)
  To: Matt Rice; +Cc: gdb-patches

Matt Rice <ratmice@gmail.com> writes:

> little bug with the new python prompt stuff, the prompt_hook doesn't
> take effect until the second prompt.

Thanks for doing this.  This looks fine to me, apart from one question.

For tests, there are prompt tests in the python.exp file for prompt
substitutions.  For passing arguments to GDB at start-up, see usage of
$GDBFLAGS in the testsuite.


> -     prompt, otherwise just print the prompt.  */
> -  if (async_command_editing_p)
> -    {
> -      int length;
> -      char *a_prompt;
> -      char *gdb_prompt = get_prompt (0);
> -
> -      /* Tell readline what the prompt to display is and what function
> -         it will need to call after a whole line is read.  This also
> -         displays the first prompt.  */
> -      length = strlen (get_prefix (0))
> -	+ strlen (gdb_prompt) + strlen (get_suffix(0)) + 1;
> -      a_prompt = (char *) alloca (length);
> -      strcpy (a_prompt, get_prefix (0));
> -      strcat (a_prompt, gdb_prompt);
> -      strcat (a_prompt, get_suffix (0));
> -      rl_callback_handler_install (a_prompt, input_handler);
> -    }
> -  else
> -    display_gdb_prompt (0);
> +  display_gdb_prompt (0);


display_gdb_prompt removes any handler that is present, and then
installs a handler.  As this is the first prompt, the above code does
not (because there could not have been a handler installed).  I think
attempting to remove a handler that does not exist just results in a
NOOP, but it is worth checking.  I know very little about readline,
however.  So my question is: with GDB's copy of readline, is this okay?
  
Cheers

Phil


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch] python prompt additions at first prompt.
  2011-08-01  9:51 ` Phil Muldoon
@ 2011-08-01 14:08   ` Matt Rice
  2011-08-01 14:13     ` Phil Muldoon
  0 siblings, 1 reply; 20+ messages in thread
From: Matt Rice @ 2011-08-01 14:08 UTC (permalink / raw)
  To: pmuldoon; +Cc: gdb-patches

On Mon, Aug 1, 2011 at 2:50 AM, Phil Muldoon <pmuldoon@redhat.com> wrote:
> Matt Rice <ratmice@gmail.com> writes:
>
>> little bug with the new python prompt stuff, the prompt_hook doesn't
>> take effect until the second prompt.
>
> Thanks for doing this.  This looks fine to me, apart from one question.
>
> For tests, there are prompt tests in the python.exp file for prompt
> substitutions.  For passing arguments to GDB at start-up, see usage of
> $GDBFLAGS in the testsuite.

I'd tried that, but didn't have much luck,
i'll give it another shot.

>> -     prompt, otherwise just print the prompt.  */
>> -  if (async_command_editing_p)
>> -    {
>> -      int length;
>> -      char *a_prompt;
>> -      char *gdb_prompt = get_prompt (0);
>> -
>> -      /* Tell readline what the prompt to display is and what function
>> -         it will need to call after a whole line is read.  This also
>> -         displays the first prompt.  */
>> -      length = strlen (get_prefix (0))
>> -     + strlen (gdb_prompt) + strlen (get_suffix(0)) + 1;
>> -      a_prompt = (char *) alloca (length);
>> -      strcpy (a_prompt, get_prefix (0));
>> -      strcat (a_prompt, gdb_prompt);
>> -      strcat (a_prompt, get_suffix (0));
>> -      rl_callback_handler_install (a_prompt, input_handler);
>> -    }
>> -  else
>> -    display_gdb_prompt (0);
>> +  display_gdb_prompt (0);
>
>
> display_gdb_prompt removes any handler that is present, and then
> installs a handler.  As this is the first prompt, the above code does
> not (because there could not have been a handler installed).  I think
> attempting to remove a handler that does not exist just results in a
> NOOP, but it is worth checking.  I know very little about readline,
> however.  So my question is: with GDB's copy of readline, is this okay?

Yes, the effect of it seems to turn zero's into zero's.
by my reading and valgrind, it seems fine,

the documentation doesn't specify one way or the other though.

If it is deemed not OK, it needs to be handled in display_gdb_prompt regardless,
because gdb -ex 'set editing off' ' followed by a 'set editing on', at
the gdb prompt, results in 2 calls to _remove before the callback is
installed
from display_gdb_prompt, and change_line_handler.


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch] python prompt additions at first prompt.
  2011-08-01 14:08   ` Matt Rice
@ 2011-08-01 14:13     ` Phil Muldoon
  2011-08-01 17:44       ` Matt Rice
  0 siblings, 1 reply; 20+ messages in thread
From: Phil Muldoon @ 2011-08-01 14:13 UTC (permalink / raw)
  To: Matt Rice; +Cc: gdb-patches

Matt Rice <ratmice@gmail.com> writes:

> On Mon, Aug 1, 2011 at 2:50 AM, Phil Muldoon <pmuldoon@redhat.com> wrote:
>> Matt Rice <ratmice@gmail.com> writes:
>>
>>> little bug with the new python prompt stuff, the prompt_hook doesn't
>>> take effect until the second prompt.
>>
>> Thanks for doing this.  This looks fine to me, apart from one question.
>>
>> For tests, there are prompt tests in the python.exp file for prompt
>> substitutions.  For passing arguments to GDB at start-up, see usage of
>> $GDBFLAGS in the testsuite.
>
> I'd tried that, but didn't have much luck,
> i'll give it another shot.


Look at gdb.base/args.exp

You may not be declaring GDBFLAGS as global.


>> display_gdb_prompt removes any handler that is present, and then
>> installs a handler.  As this is the first prompt, the above code does
>> not (because there could not have been a handler installed).  I think
>> attempting to remove a handler that does not exist just results in a
>> NOOP, but it is worth checking.  I know very little about readline,
>> however.  So my question is: with GDB's copy of readline, is this okay?
>
> Yes, the effect of it seems to turn zero's into zero's.
> by my reading and valgrind, it seems fine,
>
> the documentation doesn't specify one way or the other though.
>
> If it is deemed not OK, it needs to be handled in display_gdb_prompt regardless,
> because gdb -ex 'set editing off' ' followed by a 'set editing on', at
> the gdb prompt, results in 2 calls to _remove before the callback is
> installed
> from display_gdb_prompt, and change_line_handler.

I'm fine with this, display_gdb_prompt has been around since 1999 I
think, and it has not failed in those years.  I think you did a
reasonable analysis.

Cheers,

Phil


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch] python prompt additions at first prompt.
  2011-08-01 14:13     ` Phil Muldoon
@ 2011-08-01 17:44       ` Matt Rice
  2011-08-02  9:07         ` Phil Muldoon
  0 siblings, 1 reply; 20+ messages in thread
From: Matt Rice @ 2011-08-01 17:44 UTC (permalink / raw)
  To: pmuldoon; +Cc: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 791 bytes --]

On Mon, Aug 1, 2011 at 7:13 AM, Phil Muldoon <pmuldoon@redhat.com> wrote:
> Matt Rice <ratmice@gmail.com> writes:
>
>> On Mon, Aug 1, 2011 at 2:50 AM, Phil Muldoon <pmuldoon@redhat.com> wrote:
>>> Matt Rice <ratmice@gmail.com> writes:
>>>
>>>> little bug with the new python prompt stuff, the prompt_hook doesn't
>>>> take effect until the second prompt.
>>>
>>> Thanks for doing this.  This looks fine to me, apart from one question.
>>>
>>> For tests, there are prompt tests in the python.exp file for prompt
>>> substitutions.  For passing arguments to GDB at start-up, see usage of
>>> $GDBFLAGS in the testsuite.

Think it was the quoting of $gdb_prompt which had me foiled.

2011-08-01  Matt Rice  <ratmice@gmail.com>

        * gdb.python/py-prompt.exp: New file.

[-- Attachment #2: testsuite.diff --]
[-- Type: text/x-patch, Size: 2078 bytes --]

diff --git a/gdb/testsuite/gdb.python/py-prompt.exp b/gdb/testsuite/gdb.python/py-prompt.exp
new file mode 100644
index 0000000..fc15931
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-prompt.exp
@@ -0,0 +1,60 @@
+# Copyright (C) 2009, 2010, 2011 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This file is part of the GDB testsuite.  It tests the mechanism
+# for defining new GDB commands in Python.
+
+if $tracelevel then {
+    strace $tracelevel
+}
+
+load_lib gdb-python.exp
+
+# Start with a fresh gdb.
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+
+# Skip all tests if Python scripting is not enabled.
+if { [skip_python_tests] } { continue }
+gdb_exit
+
+global GDBFLAGS
+set saved_gdbflags $GDBFLAGS
+set GDBFLAGS [concat $GDBFLAGS " -ex \"python def foo(x): return \'(Foo) \'\""]
+set GDBFLAGS [concat $GDBFLAGS " -ex \"python gdb.prompt_hook = foo\""]
+
+set tmp_gdbflags $GDBFLAGS
+set saved_gdb_prompt $gdb_prompt
+
+global gdb_prompt
+set gdb_prompt "\[(\]Foo\[)\]"
+
+
+# The following tests are strange in that we are testing the first prompt
+# gdb_start will fail/timeout if does not receive the '(foo) ' prompt.
+# otherwise no 'passing' test is performed.
+set GDBFLAGS [concat $tmp_gdbflags " -ex \"set editing on\""]
+gdb_start
+gdb_exit
+
+set GDBFLAGS [concat $tmp_gdbflags " -ex \"set editing off\""]
+gdb_start
+gdb_exit
+
+set GDBFLAGS $saved_gdbflags
+set gdb_prompt $saved_gdb_prompt
+return 0

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch] python prompt additions at first prompt.
  2011-08-01 17:44       ` Matt Rice
@ 2011-08-02  9:07         ` Phil Muldoon
  2011-08-02 17:59           ` Matt Rice
  0 siblings, 1 reply; 20+ messages in thread
From: Phil Muldoon @ 2011-08-02  9:07 UTC (permalink / raw)
  To: Matt Rice; +Cc: gdb-patches

Matt Rice <ratmice@gmail.com> writes:

> On Mon, Aug 1, 2011 at 7:13 AM, Phil Muldoon <pmuldoon@redhat.com> wrote:

> Think it was the quoting of $gdb_prompt which had me foiled.
>
> 2011-08-01  Matt Rice  <ratmice@gmail.com>
>
>         * gdb.python/py-prompt.exp: New file.

Thanks.

> +++ b/gdb/testsuite/gdb.python/py-prompt.exp
> @@ -0,0 +1,60 @@
> +# Copyright (C) 2009, 2010, 2011 Free Software Foundation, Inc.
> +

This is a new file, so we just need 2011.  

Also, the other prompt tests are in python.exp, so this standalone test
is kind of an anomaly.  What do you think about combining the python.exp
prompt tests into this test file?  If you think that is ok, then you can
just submit another consolidation patch after this one has been approved
and checked in.


> +set GDBFLAGS [concat $GDBFLAGS " -ex \"python def foo(x): return \'(Foo) \'\""]
> +set GDBFLAGS [concat $GDBFLAGS " -ex \"python gdb.prompt_hook = foo\""]
> +
> +set tmp_gdbflags $GDBFLAGS
> +set saved_gdb_prompt $gdb_prompt
> +
> +global gdb_prompt
> +set gdb_prompt "\[(\]Foo\[)\]"
> +
> +
> +# The following tests are strange in that we are testing the first prompt
> +# gdb_start will fail/timeout if does not receive the '(foo) ' prompt.
> +# otherwise no 'passing' test is performed.
> +set GDBFLAGS [concat $tmp_gdbflags " -ex \"set editing on\""]
> +gdb_start
> +gdb_exit

While this does test the prompt at startup, without a PASS/FAIL how can
we track if it has regressed?  I think it will just timeout, which is
not optimal.

If Tom (or any other maintainer) is ok with it, I guess I am too.  If
there is a method to utilise a formal PASS/FAIL into the test I would
prefer that.

Thanks again for doing this.

Cheers

Phil


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch] python prompt additions at first prompt.
  2011-08-02  9:07         ` Phil Muldoon
@ 2011-08-02 17:59           ` Matt Rice
  2011-08-02 20:37             ` Phil Muldoon
  0 siblings, 1 reply; 20+ messages in thread
From: Matt Rice @ 2011-08-02 17:59 UTC (permalink / raw)
  To: pmuldoon; +Cc: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 1373 bytes --]

On Tue, Aug 2, 2011 at 2:07 AM, Phil Muldoon <pmuldoon@redhat.com> wrote:
> Matt Rice <ratmice@gmail.com> writes:
>
>> On Mon, Aug 1, 2011 at 7:13 AM, Phil Muldoon <pmuldoon@redhat.com> wrote:
>
>> Think it was the quoting of $gdb_prompt which had me foiled.
>>
>> 2011-08-01  Matt Rice  <ratmice@gmail.com>
>>
>>         * gdb.python/py-prompt.exp: New file.
>
> Thanks.
>
>> +++ b/gdb/testsuite/gdb.python/py-prompt.exp
>> @@ -0,0 +1,60 @@
>> +# Copyright (C) 2009, 2010, 2011 Free Software Foundation, Inc.
>> +
>
> This is a new file, so we just need 2011.

Oops,

> Also, the other prompt tests are in python.exp, so this standalone test
> is kind of an anomaly.  What do you think about combining the python.exp
> prompt tests into this test file?  If you think that is ok, then you can
> just submit another consolidation patch after this one has been approved
> and checked in.

sure don't mind at all,

with all the gdb invocations and global variables in this test
and python.exp being shared between many tests I didn't want to ugly it up.

> If there is a method to utilise a formal PASS/FAIL into the test I would
> prefer that.

I managed to finagle this into working with a modified gdb_start.
I stuck it in lib because i imagine it has to be for the baseboard override
to work, even though its only used by this test?

[-- Attachment #2: testsuite.diff --]
[-- Type: text/x-patch, Size: 5395 bytes --]

diff --git a/gdb/testsuite/gdb.python/py-prompt.exp b/gdb/testsuite/gdb.python/py-prompt.exp
new file mode 100644
index 0000000..1189bc3
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-prompt.exp
@@ -0,0 +1,75 @@
+# Copyright (C) 2011 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This file is part of the GDB testsuite.  It tests the mechanism
+# for defining the prompt in Python.
+
+if $tracelevel then {
+    strace $tracelevel
+}
+
+load_lib gdb-python.exp
+load_lib prompt.exp
+
+# Start with a fresh gdb.
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+
+# Skip all tests if Python scripting is not enabled.
+if { [skip_python_tests] } { continue }
+gdb_exit
+
+global GDBFLAGS
+set saved_gdbflags $GDBFLAGS
+set GDBFLAGS [concat $GDBFLAGS " -ex \"set height 0\""]
+set GDBFLAGS [concat $GDBFLAGS " -ex \"set width 0\""]
+set GDBFLAGS [concat $GDBFLAGS " -ex \"python i = 0\""]
+set prompt_func "python def foo(x): global i; i += 1; return \'(Foo) \'"
+set GDBFLAGS [concat $GDBFLAGS " -ex \"$prompt_func\""]
+set GDBFLAGS [concat $GDBFLAGS " -ex \"python gdb.prompt_hook=foo\""]
+
+set tmp_gdbflags $GDBFLAGS
+set saved_gdb_prompt $gdb_prompt
+set gdb_prompt_fail "\[(\]gdb\[)\]"
+
+global gdb_prompt
+# Does not include the space at the end of the prompt.
+# gdb_test expects it not to be there.
+set gdb_prompt "\[(\]Foo\[)\]"
+
+set GDBFLAGS [concat $tmp_gdbflags " -ex \"set editing on\""]
+prompt_gdb_start
+gdb_test "python x = i; print gdb.execute(\"show prompt\", to_string = True)" \
+	 ".*prompt is \"$gdb_prompt \".*" \
+	 "show prompt gets the correct result"
+gdb_test "python print x, i" "1 2" \
+	 "retrieving the prompt causes no extra prompt_hook calls"
+gdb_exit
+
+
+set GDBFLAGS [concat $tmp_gdbflags " -ex \"set editing off\""]
+prompt_gdb_start
+gdb_test "python x = i; print gdb.execute(\"show prompt\", to_string = True)" \
+	 ".*prompt is \"$gdb_prompt \".*" \
+	 "show prompt gets the correct result 2"
+gdb_test "python print x, i" "1 2" \
+	 "retrieving the prompt causes no extra prompt_hook calls 2"
+gdb_exit
+
+set GDBFLAGS $saved_gdbflags
+set gdb_prompt $saved_gdb_prompt
+return 0
diff --git a/gdb/testsuite/lib/prompt.exp b/gdb/testsuite/lib/prompt.exp
new file mode 100644
index 0000000..84186cc
--- /dev/null
+++ b/gdb/testsuite/lib/prompt.exp
@@ -0,0 +1,86 @@
+# Copyright (C) 2011 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Specialized subroutines for launching gdb and testing the very first prompt.
+
+
+#
+# start gdb -- start gdb running, prompt procedure
+# this procedure differs from the default in that you must pass 'set height 0',
+# and 'set width 0', yourself in GDBFLAGS, and it has a gdb_prompt_fail variable,
+#
+# uses pass if it sees $gdb_prompt, and fail if it sees $gdb_prompt_fail.
+#
+proc default_prompt_gdb_start { } {
+    global verbose
+    global GDB
+    global INTERNAL_GDBFLAGS GDBFLAGS
+    global gdb_prompt
+    global gdb_prompt_fail
+    global timeout
+    global gdb_spawn_id;
+
+    gdb_stop_suppressing_tests;
+
+    verbose "Spawning $GDB $INTERNAL_GDBFLAGS $GDBFLAGS"
+
+    if [info exists gdb_spawn_id] {
+	return 0;
+    }
+
+    if ![is_remote host] {
+	if { [which $GDB] == 0 } then {
+	    perror "$GDB does not exist."
+	    exit 1
+	}
+    }
+    set res [remote_spawn host "$GDB $INTERNAL_GDBFLAGS $GDBFLAGS [host_info gdb_opts]"];
+    if { $res < 0 || $res == "" } {
+	perror "Spawning $GDB failed."
+	return 1;
+    }
+    gdb_expect 360 {
+	-re "\[\r\n\]$gdb_prompt_fail $" {
+	    fail "GDB initializing first prompt"
+	}
+	-re "\[\r\n\]$gdb_prompt $" {
+	    pass "GDB initializing first prompt"
+	}
+	-re "$gdb_prompt $"	{
+	    perror "GDB never initialized."
+	    return -1
+	}
+	-re "$gdb_prompt_fail $"	{
+	    perror "GDB never initialized."
+	    return -1
+	}
+	timeout	{
+	    perror "(timeout) GDB never initialized after 10 seconds."
+	    remote_close host;
+	    return -1
+	}
+    }
+    set gdb_spawn_id -1;
+    return 0;
+}
+
+#
+# Overridable function. You can override this function in your
+# baseboard file.
+#
+proc prompt_gdb_start { } {
+    default_prompt_gdb_start
+}
+

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch] python prompt additions at first prompt.
  2011-08-02 17:59           ` Matt Rice
@ 2011-08-02 20:37             ` Phil Muldoon
  0 siblings, 0 replies; 20+ messages in thread
From: Phil Muldoon @ 2011-08-02 20:37 UTC (permalink / raw)
  To: Matt Rice; +Cc: gdb-patches

Matt Rice <ratmice@gmail.com> writes:

> I managed to finagle this into working with a modified gdb_start.
> I stuck it in lib because i imagine it has to be for the baseboard override
> to work, even though its only used by this test?


Thanks for persevering and your work.  This looks OK to me.  Please
remember to wait on a maintainer review.

Cheers,

Phil


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch] python prompt additions at first prompt.
  2011-07-30 22:20 [patch] python prompt additions at first prompt Matt Rice
  2011-08-01  9:51 ` Phil Muldoon
@ 2011-08-03 18:08 ` Tom Tromey
  2011-08-09  0:20   ` Matt Rice
  1 sibling, 1 reply; 20+ messages in thread
From: Tom Tromey @ 2011-08-03 18:08 UTC (permalink / raw)
  To: Matt Rice; +Cc: gdb-patches

>>>>> "Matt" == Matt Rice <ratmice@gmail.com> writes:

Matt> I think it's ok anyways, the 'if (async_command_editing_p)' case in
Matt> display_gdb_prompt seems to cover it,
Matt> with the addition of rl_callback_handler_remove() which afaict seemed
Matt> ok to call before a handler is installed

It is really unclear to me whether or not this patch is ok.

display_gdb_prompt can early exit in a couple of cases:

1:

  if (!current_interp_display_prompt_p ())
    return;

(I don't think this one can happen here.)

2:

  if (sync_execution && is_running (inferior_ptid))
    {
[...]
      rl_callback_handler_remove ();
      return;

This seems like it could be possible, maybe with attach + cont from the
command line?

If that one cannot be taken, then I agree the patch is correct.
Could you try this?

Tom


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch] python prompt additions at first prompt.
  2011-08-03 18:08 ` Tom Tromey
@ 2011-08-09  0:20   ` Matt Rice
  2011-08-09  0:25     ` Matt Rice
  2011-08-10 15:21     ` Tom Tromey
  0 siblings, 2 replies; 20+ messages in thread
From: Matt Rice @ 2011-08-09  0:20 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On Wed, Aug 3, 2011 at 11:07 AM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Matt" == Matt Rice <ratmice@gmail.com> writes:
>
> Matt> I think it's ok anyways, the 'if (async_command_editing_p)' case in
> Matt> display_gdb_prompt seems to cover it,
> Matt> with the addition of rl_callback_handler_remove() which afaict seemed
> Matt> ok to call before a handler is installed
>
> It is really unclear to me whether or not this patch is ok.
>
> display_gdb_prompt can early exit in a couple of cases:
>
> 1:
>
>  if (!current_interp_display_prompt_p ())
>    return;
>
> (I don't think this one can happen here.)
>
> 2:
>
>  if (sync_execution && is_running (inferior_ptid))
>    {
> [...]
>      rl_callback_handler_remove ();
>      return;
>
> This seems like it could be possible, maybe with attach + cont from the
> command line?

argh, thanks for noticing this should have seen it.
yes it is possible,

./gdb/gdb -ex 'set target-async on' -ex 'attach 26359' -ex 'continue&'

I have yet to fully unfurl the state machine, but in this case,
display_gdb_prompt
now gets called twice, first hitting this sync_execution && is_running
and returning, secondly without returning early, and hitting
async_editing.   This leads to one prompt being displayed.

further, trying this same case without my patch leads me to believe
that the original code is not correct either.
precisely because it does call rl_callback_handler_install, debugging
gdb gives an infinite loop of these.
not debugging gdb you seem to wind up back at the shell needing to 'fg' gdb.

Program received signal SIGTTOU, Stopped (tty output).
0x000000394acd7f18 in tcsetattr () from /lib64/libc.so.6
(gdb) bt
#0  0x000000394acd7f18 in tcsetattr () from /lib64/libc.so.6
#1  0x00000000006c16db in _set_tty_settings (tty=0,
tiop=0x7fffffffdcf0) at rltty.c:476
#2  0x00000000006c1706 in set_tty_settings (tty=0,
tiop=0x7fffffffdcf0) at rltty.c:490
#3  0x00000000006c1a71 in rl_prep_terminal (meta_flag=1) at rltty.c:653
#4  0x00000000006d436e in _rl_callback_newline () at callback.c:82
#5  0x00000000006d43d3 in rl_callback_handler_install
(prompt=0x7fffffffdd70 "",
    linefunc=0x5b6236 <command_line_handler>) at callback.c:102
#6  0x00000000004fa86c in tui_command_loop (data=0x0) at ./tui/tui-interp.c:159

the patch at least suffers from one issue that the first 'current'
argument to gdb.prompt_hook
is both inconsistent and unsatisfactory.
None, or "" empty string with patch: (normal vs target-async/attach .. etc)
rather than the "(gdb) " it is without the patch/should be.
(by putting the observer mechanism before the first rl_callback_handler_install)

So, I guess neither the current patch or adding the observer calls to
the existing code
is correct and we at least need to split up prompt displaying from the
setting of the prompt parameter?

> If that one cannot be taken, then I agree the patch is correct.
> Could you try this?
>
> Tom
>


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch] python prompt additions at first prompt.
  2011-08-09  0:20   ` Matt Rice
@ 2011-08-09  0:25     ` Matt Rice
  2011-08-10 15:21     ` Tom Tromey
  1 sibling, 0 replies; 20+ messages in thread
From: Matt Rice @ 2011-08-09  0:25 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On Mon, Aug 8, 2011 at 5:20 PM, Matt Rice <ratmice@gmail.com> wrote:

> further, trying this same case without my patch leads me to believe
> that the original code is not correct either.
> precisely because it does call rl_callback_handler_install, debugging
> gdb gives an infinite loop of these.
> not debugging gdb you seem to wind up back at the shell needing to 'fg' gdb.

should have looked first, this seems to be PR 10720


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch] python prompt additions at first prompt.
  2011-08-09  0:20   ` Matt Rice
  2011-08-09  0:25     ` Matt Rice
@ 2011-08-10 15:21     ` Tom Tromey
  2011-08-11 12:03       ` Matt Rice
  1 sibling, 1 reply; 20+ messages in thread
From: Tom Tromey @ 2011-08-10 15:21 UTC (permalink / raw)
  To: Matt Rice; +Cc: gdb-patches

Matt> So, I guess neither the current patch or adding the observer calls
Matt> to the existing code is correct and we at least need to split up
Matt> prompt displaying from the setting of the prompt parameter?

I'm afraid I don't have an answer for you.

Tom


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch] python prompt additions at first prompt.
  2011-08-10 15:21     ` Tom Tromey
@ 2011-08-11 12:03       ` Matt Rice
  2011-08-12 13:10         ` Matt Rice
  0 siblings, 1 reply; 20+ messages in thread
From: Matt Rice @ 2011-08-11 12:03 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 3451 bytes --]

On Wed, Aug 10, 2011 at 8:20 AM, Tom Tromey <tromey@redhat.com> wrote:
> Matt> So, I guess neither the current patch or adding the observer calls
> Matt> to the existing code is correct and we at least need to split up
> Matt> prompt displaying from the setting of the prompt parameter?
>
> I'm afraid I don't have an answer for you.

thats alright, updated patch, modifications from the previous patch include
on the code side we need to avoid calling the observer/prompt_hook
when sync_execution is enabled,
wrt tests check for double prompting in lib/prompt.exp, and add
sync_execution tests,
and tests for the prompt_hook function's argument.

if we don't check sync_execution when calling the prompt_hook:
in the sync_execution && is_running case this leads to prompt_hook
being called for a prompt that is never displayed (giving the
prompt_hook an empty string argument from async_disable_stdin).

in the sync_execution && !is_running case the fallthrough can lead to
a 'double prompt', if the prompt_hook overrides the empty prompt that
async_disable_stdin sets, then it prompts again in the future.

there is a concerted effort (see below for examples), that we force
the prompt to return early, avoiding readline stuff, and then
re-calling it in the future, when the signal handling won't get in the
way, the first prompt does not seem to be different in regard to this
than any other, thus the cli_command_loop call to display_gdb_prompt,
returns early, then later in fetch_inferior_event, we call it again
without sync_execution set, and its only then that we get the first
prompt and setup readline.

doing readline stuff in cli_command_loop (in the case of
sync_execution) was causing 10720.
because sync_execution is set during command handling which happens
before the first prompt.
and it wasn't testing that flag.

thus, I don't think returning early is a problem here, i believe it is
working as intended, though this entire mechanism does seem complex
and fragile... anyhow that is my understanding of it, thanks for your
patience.

infrun.c:fetch_inferior_event (~2815)
  /* If the inferior was in sync execution mode, and now isn't,
     restore the prompt.  */
  if (was_sync && !sync_execution)
    display_gdb_prompt (0);

inf-loop.c:inferior_event_handler (~58)
          async_enable_stdin ();
          display_gdb_prompt (0);

inf-loop.c:inferior_event_handler (~73)
      /* The call to async_enable_stdin below resets 'sync_execution'.
         However, if sync_execution is 1 now, we also need to show the
         prompt below, so save the current value.  */
      was_sync = sync_execution;
      async_enable_stdin ();

P.S. I have seen some core dumps on exit in the sync_execution
versions of these tests, both with and without the patch.
along with random stopping after 'continue&', i'm not very familiar
with target-async so i'm not exactly sure what to make of it.

no new failures (not even a spurious one, weird), all new tests pass.

2011-08-11  Matt Rice  <ratmice@gmail.com>

        * event-top.c (cli_command_loop): Replace readline setup with
        direct call to display_gdb_prompt.
        (display_gdb_prompt): Do not call observer mechanism during
        synchronous execution.

2011-08-11  Matt Rice  <ratmice@gmail.com>

        * lib/prompt.exp: New file for testing the first prompt.
        * gdb.python/py-prompt.exp: Ditto.
        * gdb.python/py-prompt.c: Ditto (copy of ext-attach.c).

[-- Attachment #2: foo.diff --]
[-- Type: text/x-patch, Size: 10731 bytes --]

diff --git a/gdb/event-top.c b/gdb/event-top.c
index 37882728..0210600 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -185,27 +185,7 @@ rl_callback_read_char_wrapper (gdb_client_data client_data)
 void
 cli_command_loop (void)
 {
-  /* If we are using readline, set things up and display the first
-     prompt, otherwise just print the prompt.  */
-  if (async_command_editing_p)
-    {
-      int length;
-      char *a_prompt;
-      char *gdb_prompt = get_prompt (0);
-
-      /* Tell readline what the prompt to display is and what function
-         it will need to call after a whole line is read.  This also
-         displays the first prompt.  */
-      length = strlen (get_prefix (0))
-	+ strlen (gdb_prompt) + strlen (get_suffix(0)) + 1;
-      a_prompt = (char *) alloca (length);
-      strcpy (a_prompt, get_prefix (0));
-      strcat (a_prompt, gdb_prompt);
-      strcat (a_prompt, get_suffix (0));
-      rl_callback_handler_install (a_prompt, input_handler);
-    }
-  else
-    display_gdb_prompt (0);
+  display_gdb_prompt (0);
 
   /* Now it's time to start the event loop.  */
   start_event_loop ();
@@ -273,7 +253,7 @@ display_gdb_prompt (char *new_prompt)
      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)
+  if (! sync_execution && ! new_prompt)
     {
       char *post_gdb_prompt = NULL;
       char *pre_gdb_prompt = xstrdup (get_prompt (0));
diff --git a/gdb/testsuite/gdb.python/py-prompt.c b/gdb/testsuite/gdb.python/py-prompt.c
new file mode 100644
index 0000000..8d84f09
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-prompt.c
@@ -0,0 +1,31 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2007, 2009, 2010, 2011 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* This program is intended to be started outside of gdb, and then
+   attached to by gdb.  It loops for a while, but not forever.  */
+
+#include <unistd.h>
+
+int main ()
+{
+  int i;
+
+  for (i = 0; i < 120; i++)
+    sleep (1);
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.python/py-prompt.exp b/gdb/testsuite/gdb.python/py-prompt.exp
new file mode 100644
index 0000000..c1ca105
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-prompt.exp
@@ -0,0 +1,131 @@
+# Copyright (C) 2011 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This file is part of the GDB testsuite.  It tests the mechanism
+# for defining the prompt in Python.
+
+if $tracelevel then {
+    strace $tracelevel
+}
+
+set testfile "py-prompt"
+set srcfile ${testfile}.c
+set binfile ${objdir}/${subdir}/${testfile}
+
+
+load_lib gdb-python.exp
+load_lib prompt.exp
+
+# Start with a fresh gdb.
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+
+# Skip all tests if Python scripting is not enabled.
+if { [skip_python_tests] } { continue }
+gdb_exit
+
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
+    untested py-prompt.exp
+    return -1
+}
+
+global GDBFLAGS
+set saved_gdbflags $GDBFLAGS
+set GDBFLAGS [concat $GDBFLAGS " -ex \"set height 0\""]
+set GDBFLAGS [concat $GDBFLAGS " -ex \"set width 0\""]
+set GDBFLAGS [concat $GDBFLAGS " -ex \"python p = list()\""]
+set prompt_func "python def foo(x): global p; p.append(x);  return \'(Foo) \'"
+set GDBFLAGS [concat $GDBFLAGS " -ex \"$prompt_func\""]
+set GDBFLAGS [concat $GDBFLAGS " -ex \"python gdb.prompt_hook=foo\""]
+
+set tmp_gdbflags $GDBFLAGS
+set gdb_prompt_fail $gdb_prompt
+
+global gdb_prompt
+# Does not include the space at the end of the prompt.
+# gdb_test expects it not to be there.
+set gdb_prompt "\[(\]Foo\[)\]"
+
+set GDBFLAGS [concat $tmp_gdbflags " -ex \"set editing on\""]
+prompt_gdb_start
+gdb_test "python x = len(p); print gdb.execute(\"show prompt\", to_string = True)" \
+	 ".*prompt is \"$gdb_prompt \".*" \
+	 "show prompt gets the correct result"
+gdb_test "python print x, len(p)" "1 2" \
+	 "retrieving the prompt causes no extra prompt_hook calls"
+gdb_test "python print \"'\" + str(p\[0\]) + \"'\"" "'$gdb_prompt_fail '" \
+	 "prompt_hook argument is default prompt."
+gdb_exit
+
+
+set GDBFLAGS [concat $tmp_gdbflags " -ex \"set editing off\""]
+prompt_gdb_start
+gdb_test "python x = len(p); print gdb.execute(\"show prompt\", to_string = True)" \
+	 ".*prompt is \"$gdb_prompt \".*" \
+	 "show prompt gets the correct result 2"
+gdb_test "python print x, len(p)" "1 2" \
+	 "retrieving the prompt causes no extra prompt_hook calls 2"
+gdb_test "python print \"'\" + str(p\[0\]) + \"'\"" "'$gdb_prompt_fail '" \
+	 "prompt_hook argument is default prompt. 2"
+gdb_exit
+
+# Start the program running and then wait for a bit, to be sure
+# that it can be attached to.
+set testpid [eval exec $binfile &]
+exec sleep 2
+if { [istarget "*-*-cygwin*"] } {
+    # testpid is the Cygwin PID, GDB uses the Windows PID, which might be
+    # different due to the way fork/exec works.
+    set testpid [ exec ps -e | gawk "{ if (\$1 == $testpid) print \$4; }" ]
+}
+
+set GDBFLAGS [concat $tmp_gdbflags " -ex \"set target-async on\""]
+set GDBFLAGS [concat $GDBFLAGS " -ex \"set pagination off\""]
+set GDBFLAGS [concat $GDBFLAGS " -ex \"set editing on\""]
+set GDBFLAGS [concat $GDBFLAGS " -ex \"attach $testpid\""]
+set GDBFLAGS [concat $GDBFLAGS " -ex \"continue&\""]
+
+# sync_execution = 1 is_running = 1
+prompt_gdb_start
+gdb_test "python x = len(p); print gdb.execute(\"show prompt\", to_string = True)" \
+	 ".*prompt is \"$gdb_prompt \".*" \
+	 "show prompt gets the correct result 3"
+gdb_test "python print x, len(p)" "1 2" \
+	 "retrieving the prompt causes no extra prompt_hook calls 3"
+gdb_test "python print \"'\" + str(p\[0\]) + \"'\"" "'$gdb_prompt_fail '" \
+	 "prompt_hook argument is default prompt. 3"
+gdb_exit
+
+set GDBFLAGS [concat $tmp_gdbflags " -ex \"set target-async on\""]
+set GDBFLAGS [concat $GDBFLAGS " -ex \"set pagination off\""]
+set GDBFLAGS [concat $GDBFLAGS " -ex \"set editing on\""]
+set GDBFLAGS [concat $GDBFLAGS " -ex \"attach $testpid\""]
+set GDBFLAGS [concat $GDBFLAGS " -ex \"interrupt\""]
+
+# sync_execution = 1 is_running = 0
+prompt_gdb_start
+gdb_test "python x = len(p); print gdb.execute(\"show prompt\", to_string = True)" \
+	 ".*prompt is \"$gdb_prompt \".*" \
+	 "show prompt gets the correct result 4"
+gdb_test "python print x, len(p)" "1 2" \
+	 "retrieving the prompt causes no extra prompt_hook calls 4"
+gdb_test "python print \"'\" + str(p\[0\]) + \"'\"" "'$gdb_prompt_fail '" \
+	 "prompt_hook argument is default prompt. 4"
+gdb_exit
+
+set GDBFLAGS $saved_gdbflags
+return 0
diff --git a/gdb/testsuite/lib/prompt.exp b/gdb/testsuite/lib/prompt.exp
new file mode 100644
index 0000000..727bbb3
--- /dev/null
+++ b/gdb/testsuite/lib/prompt.exp
@@ -0,0 +1,92 @@
+# Copyright (C) 2011 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Specialized subroutines for launching gdb and testing the very first prompt.
+
+
+#
+# start gdb -- start gdb running, prompt procedure
+# this procedure differs from the default in that you must pass 'set height 0',
+# and 'set width 0', yourself in GDBFLAGS, and it has a gdb_prompt_fail variable,
+#
+# uses pass if it sees $gdb_prompt, and fail if it sees $gdb_prompt_fail.
+#
+proc default_prompt_gdb_start { } {
+    global verbose
+    global GDB
+    global INTERNAL_GDBFLAGS GDBFLAGS
+    global gdb_prompt
+    global gdb_prompt_fail
+    global timeout
+    global gdb_spawn_id;
+
+    gdb_stop_suppressing_tests;
+
+    verbose "Spawning $GDB $INTERNAL_GDBFLAGS $GDBFLAGS"
+
+    if [info exists gdb_spawn_id] {
+	return 0;
+    }
+
+    if ![is_remote host] {
+	if { [which $GDB] == 0 } then {
+	    perror "$GDB does not exist."
+	    exit 1
+	}
+    }
+    set res [remote_spawn host "$GDB $INTERNAL_GDBFLAGS $GDBFLAGS [host_info gdb_opts]"];
+    if { $res < 0 || $res == "" } {
+	perror "Spawning $GDB failed."
+	return 1;
+    }
+    gdb_expect 360 {
+	-re ".*$gdb_prompt_fail.*$gdb_prompt_fail.*" {
+	    fail "double prompted fail prompt"
+	}
+	-re ".*$gdb_prompt.*$gdb_prompt.*" {
+	    fail "double prompted"
+	}
+	-re "\[\r\n\]$gdb_prompt_fail $" {
+	    fail "GDB initializing first prompt"
+	}
+	-re "\[\r\n\]$gdb_prompt $" {
+	    pass "GDB initializing first prompt"
+	}
+	-re "$gdb_prompt $"	{
+	    perror "GDB never initialized."
+	    return -1
+	}
+	-re "$gdb_prompt_fail $"	{
+	    perror "GDB never initialized."
+	    return -1
+	}
+	timeout	{
+	    perror "(timeout) GDB never initialized after 10 seconds."
+	    remote_close host;
+	    return -1
+	}
+    }
+    set gdb_spawn_id -1;
+    return 0;
+}
+
+#
+# Overridable function. You can override this function in your
+# baseboard file.
+#
+proc prompt_gdb_start { } {
+    default_prompt_gdb_start
+}
+

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch] python prompt additions at first prompt.
  2011-08-11 12:03       ` Matt Rice
@ 2011-08-12 13:10         ` Matt Rice
  2011-08-12 14:44           ` Pedro Alves
  0 siblings, 1 reply; 20+ messages in thread
From: Matt Rice @ 2011-08-12 13:10 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 2166 bytes --]

sorry, think i'm done now...
this patch is intended to be functionally equivalent to the one i sent
yesterday wrt prompt bugs,
I thought (too late), that the sync_execution && !is_running behavior
of 'display an empty prompt followed by an actual prompt' is odd, and
since it happens only in obscure cases
is easily missed.

it seems clearer to return early in all sync_execution cases, and
limit the potential for introducing the double prompting type of bug.
I haven't been able to find any problems with this approach.

2011-08-12  Matt Rice  <ratmice@gmail.com>

        * event-top.c (cli_command_loop): Call display_gdb_prompt
        unconditionally.
        (display_gdb_prompt): Exit early if sync_execution is set.
        Move observer calls after early exits.
        Remove extra semi-colon.
        (async_enable_stdin): Don't pop an empty prompt.
        (async_disable_stdin): Don't push an empty prompt.


more information on PR 10720, which I'd had random difficulties
reproducing, I managed to figure out why, the process being attached
to must be on the same tty as the gdb process to reproduce it.
Haven't been able to reproduce that in the testsuite though.

thus the following gdb command lines on the various PID tty's the
buggy gdb process which gets Stopped is frobbed.

$ ~/tests/test &
[10] 18413
$ tty
/dev/pts/0
$ ~/tests/test  &
[3] 18415
$ tty
/dev/pts/4
$ gdb/gdb -quiet -ex 'set target-async on' -ex 'attach 18415' -ex 'continue&'
Attaching to process 18415
Continuing.

[4]+  Stopped                 gdb/gdb -quiet -ex 'set target-async on'
-ex 'attach 18415' -ex 'continue&'

$ gdb/gdb -quiet -ex 'set target-async on' -ex 'attach 18413' -ex 'continue&'
Attaching to process 18413
Continuing.

Program received signal SIGSTOP, Stopped (signal).
0x00400525 in ?? ()
Reading symbols from /home/ratmice/tests/test...done.
Reading symbols from /lib64/libc.so.6...(no debugging symbols found)...done.
Loaded symbols for /lib64/libc.so.6
Reading symbols from /lib64/ld-linux-x86-64.so.2...(no debugging
symbols found)...done.
Loaded symbols for /lib64/ld-linux-x86-64.so.2
main (argc=1, argv=0x7fff67d42c68) at test.c:39
39	    ;
(gdb)

[-- Attachment #2: bar.diff --]
[-- Type: text/x-patch, Size: 3991 bytes --]

diff --git a/gdb/event-top.c b/gdb/event-top.c
index 37882728..29a57f1 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -185,27 +185,7 @@ rl_callback_read_char_wrapper (gdb_client_data client_data)
 void
 cli_command_loop (void)
 {
-  /* If we are using readline, set things up and display the first
-     prompt, otherwise just print the prompt.  */
-  if (async_command_editing_p)
-    {
-      int length;
-      char *a_prompt;
-      char *gdb_prompt = get_prompt (0);
-
-      /* Tell readline what the prompt to display is and what function
-         it will need to call after a whole line is read.  This also
-         displays the first prompt.  */
-      length = strlen (get_prefix (0))
-	+ strlen (gdb_prompt) + strlen (get_suffix(0)) + 1;
-      a_prompt = (char *) alloca (length);
-      strcpy (a_prompt, get_prefix (0));
-      strcat (a_prompt, gdb_prompt);
-      strcat (a_prompt, get_suffix (0));
-      rl_callback_handler_install (a_prompt, input_handler);
-    }
-  else
-    display_gdb_prompt (0);
+  display_gdb_prompt (0);
 
   /* Now it's time to start the event loop.  */
   start_event_loop ();
@@ -269,26 +249,7 @@ 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 (0));
-
-      observer_notify_before_prompt (pre_gdb_prompt);
-      post_gdb_prompt = get_prompt (0);
-
-      /* 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))
+  if (sync_execution)
     {
       /* This is to trick readline into not trying to display the
          prompt.  Even though we display the prompt using this
@@ -305,10 +266,31 @@ display_gdb_prompt (char *new_prompt)
          between the calls to the above two functions.
          Calling rl_callback_handler_remove(), does the job.  */
 
-      rl_callback_handler_remove ();
+      if (is_running (inferior_ptid))
+        rl_callback_handler_remove ();
+
       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 (0));
+
+      observer_notify_before_prompt (pre_gdb_prompt);
+      post_gdb_prompt = get_prompt (0);
+
+      /* 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 the observer changed the prompt, ACTUAL_GDB_PROMPT will not be
      NULL.  Otherwise, either copy the existing prompt, or set it to
      NEW_PROMPT.  */
@@ -331,7 +313,7 @@ display_gdb_prompt (char *new_prompt)
 	  strcat (actual_gdb_prompt, get_suffix (0));
 	}
       else
-	actual_gdb_prompt = new_prompt;;
+	actual_gdb_prompt = new_prompt;
     }
 
   if (async_command_editing_p)
@@ -473,7 +455,6 @@ async_enable_stdin (void)
 	 sync_execution.  Current target_terminal_ours() implementations
 	 check for sync_execution before switching the terminal.  */
       target_terminal_ours ();
-      pop_prompt ();
       sync_execution = 0;
     }
 }
@@ -487,7 +468,6 @@ async_disable_stdin (void)
   if (!sync_execution)
     {
       sync_execution = 1;
-      push_prompt ("", "", "");
     }
 }
 \f

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch] python prompt additions at first prompt.
  2011-08-12 13:10         ` Matt Rice
@ 2011-08-12 14:44           ` Pedro Alves
  2011-08-12 15:07             ` Matt Rice
  0 siblings, 1 reply; 20+ messages in thread
From: Pedro Alves @ 2011-08-12 14:44 UTC (permalink / raw)
  To: gdb-patches; +Cc: Matt Rice, Tom Tromey

On Friday 12 August 2011 14:10:17, Matt Rice wrote:
> it seems clearer to return early in all sync_execution cases, and
> limit the potential for introducing the double prompting type of bug.
> I haven't been able to find any problems with this approach.

I think this breaks the pagination prompt in async mode.  Try setting
a breakpoint in a loop, with a command list that just continues:

(gdb) b inloop
(gdb) commands 
Type commands for breakpoint(s) 2, one per line.
End with a line saying just "end".
>c
>end
(gdb) c

-- 
Pedro Alves


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch] python prompt additions at first prompt.
  2011-08-12 14:44           ` Pedro Alves
@ 2011-08-12 15:07             ` Matt Rice
  2011-08-29 16:23               ` Matt Rice
  0 siblings, 1 reply; 20+ messages in thread
From: Matt Rice @ 2011-08-12 15:07 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, Tom Tromey

On Fri, Aug 12, 2011 at 7:44 AM, Pedro Alves <pedro@codesourcery.com> wrote:
> On Friday 12 August 2011 14:10:17, Matt Rice wrote:
>> it seems clearer to return early in all sync_execution cases, and
>> limit the potential for introducing the double prompting type of bug.
>> I haven't been able to find any problems with this approach.
>
> I think this breaks the pagination prompt in async mode.  Try setting
> a breakpoint in a loop, with a command list that just continues:
>
> (gdb) b inloop
> (gdb) commands
> Type commands for breakpoint(s) 2, one per line.
> End with a line saying just "end".
>>c
>>end
> (gdb) c

Yeah it does, thank you for pointing this out.


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch] python prompt additions at first prompt.
  2011-08-12 15:07             ` Matt Rice
@ 2011-08-29 16:23               ` Matt Rice
  2011-08-29 16:39                 ` Pedro Alves
  2011-09-02 14:31                 ` Pedro Alves
  0 siblings, 2 replies; 20+ messages in thread
From: Matt Rice @ 2011-08-29 16:23 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, Tom Tromey

[-- Attachment #1: Type: text/plain, Size: 1498 bytes --]

On Fri, Aug 12, 2011 at 8:07 AM, Matt Rice <ratmice@gmail.com> wrote:
> On Fri, Aug 12, 2011 at 7:44 AM, Pedro Alves <pedro@codesourcery.com> wrote:
>> On Friday 12 August 2011 14:10:17, Matt Rice wrote:
>>> it seems clearer to return early in all sync_execution cases, and
>>> limit the potential for introducing the double prompting type of bug.
>>> I haven't been able to find any problems with this approach.
>>
>> I think this breaks the pagination prompt in async mode.  Try setting
>> a breakpoint in a loop, with a command list that just continues:
>>
>> (gdb) b inloop
>> (gdb) commands
>> Type commands for breakpoint(s) 2, one per line.
>> End with a line saying just "end".
>>>c
>>>end
>> (gdb) c
>
> Yeah it does, thank you for pointing this out.
>

Here is a version of my previous patch, which doesn't suffer from this failure.
I have added  comments explaining that we really do want to display an
empty prompt in this case.

hopefully this clears up any confusion...

2011-08-28  Matt Rice  <ratmice@gmail.com>

        * event-top.c (cli_command_loop): Replace readline setup with
        direct call to display_gdb_prompt.
        (display_gdb_prompt): Do not call observer mechanism during
        synchronous execution.

 2011-08-28  Matt Rice  <ratmice@gmail.com>

        * lib/prompt.exp: New file for testing the first prompt.
        * gdb.python/py-prompt.exp: Ditto.
        * gdb.python/py-prompt.c: Ditto (copy of ext-attach.c).

[-- Attachment #2: foo.diff --]
[-- Type: text/x-patch, Size: 11518 bytes --]

diff --git a/gdb/event-top.c b/gdb/event-top.c
index 37882728..6496988 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -185,27 +185,7 @@ rl_callback_read_char_wrapper (gdb_client_data client_data)
 void
 cli_command_loop (void)
 {
-  /* If we are using readline, set things up and display the first
-     prompt, otherwise just print the prompt.  */
-  if (async_command_editing_p)
-    {
-      int length;
-      char *a_prompt;
-      char *gdb_prompt = get_prompt (0);
-
-      /* Tell readline what the prompt to display is and what function
-         it will need to call after a whole line is read.  This also
-         displays the first prompt.  */
-      length = strlen (get_prefix (0))
-	+ strlen (gdb_prompt) + strlen (get_suffix(0)) + 1;
-      a_prompt = (char *) alloca (length);
-      strcpy (a_prompt, get_prefix (0));
-      strcat (a_prompt, gdb_prompt);
-      strcat (a_prompt, get_suffix (0));
-      rl_callback_handler_install (a_prompt, input_handler);
-    }
-  else
-    display_gdb_prompt (0);
+  display_gdb_prompt (0);
 
   /* Now it's time to start the event loop.  */
   start_event_loop ();
@@ -272,8 +252,11 @@ display_gdb_prompt (char *new_prompt)
   /* 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)
+     a temporary prompt, IE, displayed but not set.  Do not call
+     observers for a prompt change if sync_execution is set, it will
+     call us again with sync_execution not set when it wants to
+     display an actual prompt.  */
+  if (! sync_execution && ! new_prompt)
     {
       char *post_gdb_prompt = NULL;
       char *pre_gdb_prompt = xstrdup (get_prompt (0));
@@ -288,6 +271,10 @@ display_gdb_prompt (char *new_prompt)
       xfree (pre_gdb_prompt);
     }
 
+  /* In the sync_execution && !is_running case we need to display the prompt
+     event though it may be "" to avoid a double prompt, while installing the
+     callback handlers, in the async_editing_p case for pagination,
+     So fall through.  */
   if (sync_execution && is_running (inferior_ptid))
     {
       /* This is to trick readline into not trying to display the
diff --git a/gdb/testsuite/gdb.python/py-prompt.c b/gdb/testsuite/gdb.python/py-prompt.c
new file mode 100644
index 0000000..8d84f09
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-prompt.c
@@ -0,0 +1,31 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2007, 2009, 2010, 2011 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* This program is intended to be started outside of gdb, and then
+   attached to by gdb.  It loops for a while, but not forever.  */
+
+#include <unistd.h>
+
+int main ()
+{
+  int i;
+
+  for (i = 0; i < 120; i++)
+    sleep (1);
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.python/py-prompt.exp b/gdb/testsuite/gdb.python/py-prompt.exp
new file mode 100644
index 0000000..c1ca105
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-prompt.exp
@@ -0,0 +1,131 @@
+# Copyright (C) 2011 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This file is part of the GDB testsuite.  It tests the mechanism
+# for defining the prompt in Python.
+
+if $tracelevel then {
+    strace $tracelevel
+}
+
+set testfile "py-prompt"
+set srcfile ${testfile}.c
+set binfile ${objdir}/${subdir}/${testfile}
+
+
+load_lib gdb-python.exp
+load_lib prompt.exp
+
+# Start with a fresh gdb.
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+
+# Skip all tests if Python scripting is not enabled.
+if { [skip_python_tests] } { continue }
+gdb_exit
+
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
+    untested py-prompt.exp
+    return -1
+}
+
+global GDBFLAGS
+set saved_gdbflags $GDBFLAGS
+set GDBFLAGS [concat $GDBFLAGS " -ex \"set height 0\""]
+set GDBFLAGS [concat $GDBFLAGS " -ex \"set width 0\""]
+set GDBFLAGS [concat $GDBFLAGS " -ex \"python p = list()\""]
+set prompt_func "python def foo(x): global p; p.append(x);  return \'(Foo) \'"
+set GDBFLAGS [concat $GDBFLAGS " -ex \"$prompt_func\""]
+set GDBFLAGS [concat $GDBFLAGS " -ex \"python gdb.prompt_hook=foo\""]
+
+set tmp_gdbflags $GDBFLAGS
+set gdb_prompt_fail $gdb_prompt
+
+global gdb_prompt
+# Does not include the space at the end of the prompt.
+# gdb_test expects it not to be there.
+set gdb_prompt "\[(\]Foo\[)\]"
+
+set GDBFLAGS [concat $tmp_gdbflags " -ex \"set editing on\""]
+prompt_gdb_start
+gdb_test "python x = len(p); print gdb.execute(\"show prompt\", to_string = True)" \
+	 ".*prompt is \"$gdb_prompt \".*" \
+	 "show prompt gets the correct result"
+gdb_test "python print x, len(p)" "1 2" \
+	 "retrieving the prompt causes no extra prompt_hook calls"
+gdb_test "python print \"'\" + str(p\[0\]) + \"'\"" "'$gdb_prompt_fail '" \
+	 "prompt_hook argument is default prompt."
+gdb_exit
+
+
+set GDBFLAGS [concat $tmp_gdbflags " -ex \"set editing off\""]
+prompt_gdb_start
+gdb_test "python x = len(p); print gdb.execute(\"show prompt\", to_string = True)" \
+	 ".*prompt is \"$gdb_prompt \".*" \
+	 "show prompt gets the correct result 2"
+gdb_test "python print x, len(p)" "1 2" \
+	 "retrieving the prompt causes no extra prompt_hook calls 2"
+gdb_test "python print \"'\" + str(p\[0\]) + \"'\"" "'$gdb_prompt_fail '" \
+	 "prompt_hook argument is default prompt. 2"
+gdb_exit
+
+# Start the program running and then wait for a bit, to be sure
+# that it can be attached to.
+set testpid [eval exec $binfile &]
+exec sleep 2
+if { [istarget "*-*-cygwin*"] } {
+    # testpid is the Cygwin PID, GDB uses the Windows PID, which might be
+    # different due to the way fork/exec works.
+    set testpid [ exec ps -e | gawk "{ if (\$1 == $testpid) print \$4; }" ]
+}
+
+set GDBFLAGS [concat $tmp_gdbflags " -ex \"set target-async on\""]
+set GDBFLAGS [concat $GDBFLAGS " -ex \"set pagination off\""]
+set GDBFLAGS [concat $GDBFLAGS " -ex \"set editing on\""]
+set GDBFLAGS [concat $GDBFLAGS " -ex \"attach $testpid\""]
+set GDBFLAGS [concat $GDBFLAGS " -ex \"continue&\""]
+
+# sync_execution = 1 is_running = 1
+prompt_gdb_start
+gdb_test "python x = len(p); print gdb.execute(\"show prompt\", to_string = True)" \
+	 ".*prompt is \"$gdb_prompt \".*" \
+	 "show prompt gets the correct result 3"
+gdb_test "python print x, len(p)" "1 2" \
+	 "retrieving the prompt causes no extra prompt_hook calls 3"
+gdb_test "python print \"'\" + str(p\[0\]) + \"'\"" "'$gdb_prompt_fail '" \
+	 "prompt_hook argument is default prompt. 3"
+gdb_exit
+
+set GDBFLAGS [concat $tmp_gdbflags " -ex \"set target-async on\""]
+set GDBFLAGS [concat $GDBFLAGS " -ex \"set pagination off\""]
+set GDBFLAGS [concat $GDBFLAGS " -ex \"set editing on\""]
+set GDBFLAGS [concat $GDBFLAGS " -ex \"attach $testpid\""]
+set GDBFLAGS [concat $GDBFLAGS " -ex \"interrupt\""]
+
+# sync_execution = 1 is_running = 0
+prompt_gdb_start
+gdb_test "python x = len(p); print gdb.execute(\"show prompt\", to_string = True)" \
+	 ".*prompt is \"$gdb_prompt \".*" \
+	 "show prompt gets the correct result 4"
+gdb_test "python print x, len(p)" "1 2" \
+	 "retrieving the prompt causes no extra prompt_hook calls 4"
+gdb_test "python print \"'\" + str(p\[0\]) + \"'\"" "'$gdb_prompt_fail '" \
+	 "prompt_hook argument is default prompt. 4"
+gdb_exit
+
+set GDBFLAGS $saved_gdbflags
+return 0
diff --git a/gdb/testsuite/lib/prompt.exp b/gdb/testsuite/lib/prompt.exp
new file mode 100644
index 0000000..727bbb3
--- /dev/null
+++ b/gdb/testsuite/lib/prompt.exp
@@ -0,0 +1,92 @@
+# Copyright (C) 2011 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Specialized subroutines for launching gdb and testing the very first prompt.
+
+
+#
+# start gdb -- start gdb running, prompt procedure
+# this procedure differs from the default in that you must pass 'set height 0',
+# and 'set width 0', yourself in GDBFLAGS, and it has a gdb_prompt_fail variable,
+#
+# uses pass if it sees $gdb_prompt, and fail if it sees $gdb_prompt_fail.
+#
+proc default_prompt_gdb_start { } {
+    global verbose
+    global GDB
+    global INTERNAL_GDBFLAGS GDBFLAGS
+    global gdb_prompt
+    global gdb_prompt_fail
+    global timeout
+    global gdb_spawn_id;
+
+    gdb_stop_suppressing_tests;
+
+    verbose "Spawning $GDB $INTERNAL_GDBFLAGS $GDBFLAGS"
+
+    if [info exists gdb_spawn_id] {
+	return 0;
+    }
+
+    if ![is_remote host] {
+	if { [which $GDB] == 0 } then {
+	    perror "$GDB does not exist."
+	    exit 1
+	}
+    }
+    set res [remote_spawn host "$GDB $INTERNAL_GDBFLAGS $GDBFLAGS [host_info gdb_opts]"];
+    if { $res < 0 || $res == "" } {
+	perror "Spawning $GDB failed."
+	return 1;
+    }
+    gdb_expect 360 {
+	-re ".*$gdb_prompt_fail.*$gdb_prompt_fail.*" {
+	    fail "double prompted fail prompt"
+	}
+	-re ".*$gdb_prompt.*$gdb_prompt.*" {
+	    fail "double prompted"
+	}
+	-re "\[\r\n\]$gdb_prompt_fail $" {
+	    fail "GDB initializing first prompt"
+	}
+	-re "\[\r\n\]$gdb_prompt $" {
+	    pass "GDB initializing first prompt"
+	}
+	-re "$gdb_prompt $"	{
+	    perror "GDB never initialized."
+	    return -1
+	}
+	-re "$gdb_prompt_fail $"	{
+	    perror "GDB never initialized."
+	    return -1
+	}
+	timeout	{
+	    perror "(timeout) GDB never initialized after 10 seconds."
+	    remote_close host;
+	    return -1
+	}
+    }
+    set gdb_spawn_id -1;
+    return 0;
+}
+
+#
+# Overridable function. You can override this function in your
+# baseboard file.
+#
+proc prompt_gdb_start { } {
+    default_prompt_gdb_start
+}
+

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch] python prompt additions at first prompt.
  2011-08-29 16:23               ` Matt Rice
@ 2011-08-29 16:39                 ` Pedro Alves
  2011-09-02 14:31                 ` Pedro Alves
  1 sibling, 0 replies; 20+ messages in thread
From: Pedro Alves @ 2011-08-29 16:39 UTC (permalink / raw)
  To: gdb-patches; +Cc: Matt Rice, Tom Tromey

On Monday 29 August 2011 17:22:48, Matt Rice wrote:
> On Fri, Aug 12, 2011 at 8:07 AM, Matt Rice <ratmice@gmail.com> wrote:
> > On Fri, Aug 12, 2011 at 7:44 AM, Pedro Alves <pedro@codesourcery.com> wrote:
> >> On Friday 12 August 2011 14:10:17, Matt Rice wrote:
> >>> it seems clearer to return early in all sync_execution cases, and
> >>> limit the potential for introducing the double prompting type of bug.
> >>> I haven't been able to find any problems with this approach.
> >>
> >> I think this breaks the pagination prompt in async mode.  Try setting
> >> a breakpoint in a loop, with a command list that just continues:
> >>
> >> (gdb) b inloop
> >> (gdb) commands
> >> Type commands for breakpoint(s) 2, one per line.
> >> End with a line saying just "end".
> >>>c
> >>>end
> >> (gdb) c
> >
> > Yeah it does, thank you for pointing this out.
> >
> 

> Here is a version of my previous patch, which doesn't suffer from this failure.
> I have added  comments explaining that we really do want to display an
> empty prompt in this case.
> 
> hopefully this clears up any confusion...

Thanks.  I'd like to take a look at this.  Let me clear a few other pending
async things first, and I'll get back on this.

-- 
Pedro Alves


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch] python prompt additions at first prompt.
  2011-08-29 16:23               ` Matt Rice
  2011-08-29 16:39                 ` Pedro Alves
@ 2011-09-02 14:31                 ` Pedro Alves
  2011-09-02 21:41                   ` Matt Rice
  1 sibling, 1 reply; 20+ messages in thread
From: Pedro Alves @ 2011-09-02 14:31 UTC (permalink / raw)
  To: gdb-patches; +Cc: Matt Rice, Tom Tromey

Hi Matt,

Sorry for the delay.

On Monday 29 August 2011 17:22:48, Matt Rice wrote:
> +     event though it may be "" to avoid a double prompt, while installing the

Typo event -> even.

This version looks good to me.  Thanks!

-- 
Pedro Alves


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch] python prompt additions at first prompt.
  2011-09-02 14:31                 ` Pedro Alves
@ 2011-09-02 21:41                   ` Matt Rice
  0 siblings, 0 replies; 20+ messages in thread
From: Matt Rice @ 2011-09-02 21:41 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, Tom Tromey

[-- Attachment #1: Type: text/plain, Size: 180 bytes --]

On Fri, Sep 2, 2011 at 6:52 AM, Pedro Alves <pedro@codesourcery.com> wrote:
> Typo event -> even.
>
> This version looks good to me.  Thanks!

Thanks, committed as attached.

[-- Attachment #2: foo.diff --]
[-- Type: text/x-patch, Size: 11517 bytes --]

diff --git a/gdb/event-top.c b/gdb/event-top.c
index 37882728..09bd89f 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -185,27 +185,7 @@ rl_callback_read_char_wrapper (gdb_client_data client_data)
 void
 cli_command_loop (void)
 {
-  /* If we are using readline, set things up and display the first
-     prompt, otherwise just print the prompt.  */
-  if (async_command_editing_p)
-    {
-      int length;
-      char *a_prompt;
-      char *gdb_prompt = get_prompt (0);
-
-      /* Tell readline what the prompt to display is and what function
-         it will need to call after a whole line is read.  This also
-         displays the first prompt.  */
-      length = strlen (get_prefix (0))
-	+ strlen (gdb_prompt) + strlen (get_suffix(0)) + 1;
-      a_prompt = (char *) alloca (length);
-      strcpy (a_prompt, get_prefix (0));
-      strcat (a_prompt, gdb_prompt);
-      strcat (a_prompt, get_suffix (0));
-      rl_callback_handler_install (a_prompt, input_handler);
-    }
-  else
-    display_gdb_prompt (0);
+  display_gdb_prompt (0);
 
   /* Now it's time to start the event loop.  */
   start_event_loop ();
@@ -272,8 +252,11 @@ display_gdb_prompt (char *new_prompt)
   /* 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)
+     a temporary prompt, IE, displayed but not set.  Do not call
+     observers for a prompt change if sync_execution is set, it will
+     call us again with sync_execution not set when it wants to
+     display an actual prompt.  */
+  if (! sync_execution && ! new_prompt)
     {
       char *post_gdb_prompt = NULL;
       char *pre_gdb_prompt = xstrdup (get_prompt (0));
@@ -288,6 +271,10 @@ display_gdb_prompt (char *new_prompt)
       xfree (pre_gdb_prompt);
     }
 
+  /* In the sync_execution && !is_running case we need to display the prompt
+     even though it may be "" to avoid a double prompt, while installing the
+     callback handlers, in the async_editing_p case for pagination,
+     So fall through.  */
   if (sync_execution && is_running (inferior_ptid))
     {
       /* This is to trick readline into not trying to display the
diff --git a/gdb/testsuite/gdb.python/py-prompt.c b/gdb/testsuite/gdb.python/py-prompt.c
new file mode 100644
index 0000000..8d84f09
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-prompt.c
@@ -0,0 +1,31 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2007, 2009, 2010, 2011 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* This program is intended to be started outside of gdb, and then
+   attached to by gdb.  It loops for a while, but not forever.  */
+
+#include <unistd.h>
+
+int main ()
+{
+  int i;
+
+  for (i = 0; i < 120; i++)
+    sleep (1);
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.python/py-prompt.exp b/gdb/testsuite/gdb.python/py-prompt.exp
new file mode 100644
index 0000000..c1ca105
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-prompt.exp
@@ -0,0 +1,131 @@
+# Copyright (C) 2011 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This file is part of the GDB testsuite.  It tests the mechanism
+# for defining the prompt in Python.
+
+if $tracelevel then {
+    strace $tracelevel
+}
+
+set testfile "py-prompt"
+set srcfile ${testfile}.c
+set binfile ${objdir}/${subdir}/${testfile}
+
+
+load_lib gdb-python.exp
+load_lib prompt.exp
+
+# Start with a fresh gdb.
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+
+# Skip all tests if Python scripting is not enabled.
+if { [skip_python_tests] } { continue }
+gdb_exit
+
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
+    untested py-prompt.exp
+    return -1
+}
+
+global GDBFLAGS
+set saved_gdbflags $GDBFLAGS
+set GDBFLAGS [concat $GDBFLAGS " -ex \"set height 0\""]
+set GDBFLAGS [concat $GDBFLAGS " -ex \"set width 0\""]
+set GDBFLAGS [concat $GDBFLAGS " -ex \"python p = list()\""]
+set prompt_func "python def foo(x): global p; p.append(x);  return \'(Foo) \'"
+set GDBFLAGS [concat $GDBFLAGS " -ex \"$prompt_func\""]
+set GDBFLAGS [concat $GDBFLAGS " -ex \"python gdb.prompt_hook=foo\""]
+
+set tmp_gdbflags $GDBFLAGS
+set gdb_prompt_fail $gdb_prompt
+
+global gdb_prompt
+# Does not include the space at the end of the prompt.
+# gdb_test expects it not to be there.
+set gdb_prompt "\[(\]Foo\[)\]"
+
+set GDBFLAGS [concat $tmp_gdbflags " -ex \"set editing on\""]
+prompt_gdb_start
+gdb_test "python x = len(p); print gdb.execute(\"show prompt\", to_string = True)" \
+	 ".*prompt is \"$gdb_prompt \".*" \
+	 "show prompt gets the correct result"
+gdb_test "python print x, len(p)" "1 2" \
+	 "retrieving the prompt causes no extra prompt_hook calls"
+gdb_test "python print \"'\" + str(p\[0\]) + \"'\"" "'$gdb_prompt_fail '" \
+	 "prompt_hook argument is default prompt."
+gdb_exit
+
+
+set GDBFLAGS [concat $tmp_gdbflags " -ex \"set editing off\""]
+prompt_gdb_start
+gdb_test "python x = len(p); print gdb.execute(\"show prompt\", to_string = True)" \
+	 ".*prompt is \"$gdb_prompt \".*" \
+	 "show prompt gets the correct result 2"
+gdb_test "python print x, len(p)" "1 2" \
+	 "retrieving the prompt causes no extra prompt_hook calls 2"
+gdb_test "python print \"'\" + str(p\[0\]) + \"'\"" "'$gdb_prompt_fail '" \
+	 "prompt_hook argument is default prompt. 2"
+gdb_exit
+
+# Start the program running and then wait for a bit, to be sure
+# that it can be attached to.
+set testpid [eval exec $binfile &]
+exec sleep 2
+if { [istarget "*-*-cygwin*"] } {
+    # testpid is the Cygwin PID, GDB uses the Windows PID, which might be
+    # different due to the way fork/exec works.
+    set testpid [ exec ps -e | gawk "{ if (\$1 == $testpid) print \$4; }" ]
+}
+
+set GDBFLAGS [concat $tmp_gdbflags " -ex \"set target-async on\""]
+set GDBFLAGS [concat $GDBFLAGS " -ex \"set pagination off\""]
+set GDBFLAGS [concat $GDBFLAGS " -ex \"set editing on\""]
+set GDBFLAGS [concat $GDBFLAGS " -ex \"attach $testpid\""]
+set GDBFLAGS [concat $GDBFLAGS " -ex \"continue&\""]
+
+# sync_execution = 1 is_running = 1
+prompt_gdb_start
+gdb_test "python x = len(p); print gdb.execute(\"show prompt\", to_string = True)" \
+	 ".*prompt is \"$gdb_prompt \".*" \
+	 "show prompt gets the correct result 3"
+gdb_test "python print x, len(p)" "1 2" \
+	 "retrieving the prompt causes no extra prompt_hook calls 3"
+gdb_test "python print \"'\" + str(p\[0\]) + \"'\"" "'$gdb_prompt_fail '" \
+	 "prompt_hook argument is default prompt. 3"
+gdb_exit
+
+set GDBFLAGS [concat $tmp_gdbflags " -ex \"set target-async on\""]
+set GDBFLAGS [concat $GDBFLAGS " -ex \"set pagination off\""]
+set GDBFLAGS [concat $GDBFLAGS " -ex \"set editing on\""]
+set GDBFLAGS [concat $GDBFLAGS " -ex \"attach $testpid\""]
+set GDBFLAGS [concat $GDBFLAGS " -ex \"interrupt\""]
+
+# sync_execution = 1 is_running = 0
+prompt_gdb_start
+gdb_test "python x = len(p); print gdb.execute(\"show prompt\", to_string = True)" \
+	 ".*prompt is \"$gdb_prompt \".*" \
+	 "show prompt gets the correct result 4"
+gdb_test "python print x, len(p)" "1 2" \
+	 "retrieving the prompt causes no extra prompt_hook calls 4"
+gdb_test "python print \"'\" + str(p\[0\]) + \"'\"" "'$gdb_prompt_fail '" \
+	 "prompt_hook argument is default prompt. 4"
+gdb_exit
+
+set GDBFLAGS $saved_gdbflags
+return 0
diff --git a/gdb/testsuite/lib/prompt.exp b/gdb/testsuite/lib/prompt.exp
new file mode 100644
index 0000000..727bbb3
--- /dev/null
+++ b/gdb/testsuite/lib/prompt.exp
@@ -0,0 +1,92 @@
+# Copyright (C) 2011 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Specialized subroutines for launching gdb and testing the very first prompt.
+
+
+#
+# start gdb -- start gdb running, prompt procedure
+# this procedure differs from the default in that you must pass 'set height 0',
+# and 'set width 0', yourself in GDBFLAGS, and it has a gdb_prompt_fail variable,
+#
+# uses pass if it sees $gdb_prompt, and fail if it sees $gdb_prompt_fail.
+#
+proc default_prompt_gdb_start { } {
+    global verbose
+    global GDB
+    global INTERNAL_GDBFLAGS GDBFLAGS
+    global gdb_prompt
+    global gdb_prompt_fail
+    global timeout
+    global gdb_spawn_id;
+
+    gdb_stop_suppressing_tests;
+
+    verbose "Spawning $GDB $INTERNAL_GDBFLAGS $GDBFLAGS"
+
+    if [info exists gdb_spawn_id] {
+	return 0;
+    }
+
+    if ![is_remote host] {
+	if { [which $GDB] == 0 } then {
+	    perror "$GDB does not exist."
+	    exit 1
+	}
+    }
+    set res [remote_spawn host "$GDB $INTERNAL_GDBFLAGS $GDBFLAGS [host_info gdb_opts]"];
+    if { $res < 0 || $res == "" } {
+	perror "Spawning $GDB failed."
+	return 1;
+    }
+    gdb_expect 360 {
+	-re ".*$gdb_prompt_fail.*$gdb_prompt_fail.*" {
+	    fail "double prompted fail prompt"
+	}
+	-re ".*$gdb_prompt.*$gdb_prompt.*" {
+	    fail "double prompted"
+	}
+	-re "\[\r\n\]$gdb_prompt_fail $" {
+	    fail "GDB initializing first prompt"
+	}
+	-re "\[\r\n\]$gdb_prompt $" {
+	    pass "GDB initializing first prompt"
+	}
+	-re "$gdb_prompt $"	{
+	    perror "GDB never initialized."
+	    return -1
+	}
+	-re "$gdb_prompt_fail $"	{
+	    perror "GDB never initialized."
+	    return -1
+	}
+	timeout	{
+	    perror "(timeout) GDB never initialized after 10 seconds."
+	    remote_close host;
+	    return -1
+	}
+    }
+    set gdb_spawn_id -1;
+    return 0;
+}
+
+#
+# Overridable function. You can override this function in your
+# baseboard file.
+#
+proc prompt_gdb_start { } {
+    default_prompt_gdb_start
+}
+

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2011-09-02 21:22 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-07-30 22:20 [patch] python prompt additions at first prompt Matt Rice
2011-08-01  9:51 ` Phil Muldoon
2011-08-01 14:08   ` Matt Rice
2011-08-01 14:13     ` Phil Muldoon
2011-08-01 17:44       ` Matt Rice
2011-08-02  9:07         ` Phil Muldoon
2011-08-02 17:59           ` Matt Rice
2011-08-02 20:37             ` Phil Muldoon
2011-08-03 18:08 ` Tom Tromey
2011-08-09  0:20   ` Matt Rice
2011-08-09  0:25     ` Matt Rice
2011-08-10 15:21     ` Tom Tromey
2011-08-11 12:03       ` Matt Rice
2011-08-12 13:10         ` Matt Rice
2011-08-12 14:44           ` Pedro Alves
2011-08-12 15:07             ` Matt Rice
2011-08-29 16:23               ` Matt Rice
2011-08-29 16:39                 ` Pedro Alves
2011-09-02 14:31                 ` Pedro Alves
2011-09-02 21:41                   ` Matt Rice

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox