[RFC] Discard local stdin events while an attached inferior is running
Commit Message
This patch attempts to work around an annoying typo that I (and others,
I assume) commonly make while debugging an attached (not forked)
process. That is, I sometimes type "continue\n\n" instead of
"continue\n" when I want to resume the inferior. By doing so, the
inferior gets resumed and the extraneous "\n" gets buffered. But once
GDB regains control, the extraneous "\n" gets read and so an empty
command line is submitted to readline. This is shorthand for running
the previous command again. So effectively typing "continue\n\n" will
run "continue" twice. Running "continue" twice instead of once can have
irreversible consequences to the debugging session and thus is pretty
annoying to deal with.
To work around this kind of typo, this patch installs a dummy event
handler that discards all local stdin events that occur when the
attached inferior is (synchronously) running.
The obvious side-effect of this patch is that dexterous users can no
longer "queue" commands while an attached inferior is running, e.g.
type "continue\nprint foo" with the intention of "print foo" being run
as soon as GDB regains control. I personally don't make use of this
ability very much.
Is this typo a common occurrence for anyone else? Should this kind of
typo be worked around? Any thoughts on the this particular workaround,
or on other potential workarounds?
gdb/ChangeLog:
* event-top.h (stdin_discard_event_handler): Declare.
* event-top.c (stdin_discard_event_handler): Define.
* linux-nat.c: Include "top.h".
(linux_nat_terminal_inferior): Use stdin_discard_event_handler.
---
gdb/event-top.c | 8 ++++++++
gdb/event-top.h | 6 ++++++
gdb/linux-nat.c | 20 ++++++++++++++++++++
3 files changed, 34 insertions(+)
Comments
> From: Patrick Palka <patrick@parcs.ath.cx>
> Cc: Patrick Palka <patrick@parcs.ath.cx>
> Date: Tue, 2 Jun 2015 09:39:18 -0400
>
> This patch attempts to work around an annoying typo that I (and others,
> I assume) commonly make while debugging an attached (not forked)
> process. That is, I sometimes type "continue\n\n" instead of
> "continue\n" when I want to resume the inferior. By doing so, the
> inferior gets resumed and the extraneous "\n" gets buffered. But once
> GDB regains control, the extraneous "\n" gets read and so an empty
> command line is submitted to readline. This is shorthand for running
> the previous command again. So effectively typing "continue\n\n" will
> run "continue" twice. Running "continue" twice instead of once can have
> irreversible consequences to the debugging session and thus is pretty
> annoying to deal with.
>
> To work around this kind of typo, this patch installs a dummy event
> handler that discards all local stdin events that occur when the
> attached inferior is (synchronously) running.
>
> The obvious side-effect of this patch is that dexterous users can no
> longer "queue" commands while an attached inferior is running, e.g.
> type "continue\nprint foo" with the intention of "print foo" being run
> as soon as GDB regains control. I personally don't make use of this
> ability very much.
IMO, this incompatible change of behavior should at least have an
option to get the old behavior back, and that option should possibly
be off by default.
In any case, there should be a NEWS entry and a patch for the manual
which describe this unusual feature.
> Is this typo a common occurrence for anyone else?
Not common, but sometimes. However, I sometimes type ahead future
commands without waiting for the prompt, for example, when I know I'll
need a lot of CR presses.
On Tue, Jun 2, 2015 at 11:10 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Patrick Palka <patrick@parcs.ath.cx>
>> Cc: Patrick Palka <patrick@parcs.ath.cx>
>> Date: Tue, 2 Jun 2015 09:39:18 -0400
>>
>> This patch attempts to work around an annoying typo that I (and others,
>> I assume) commonly make while debugging an attached (not forked)
>> process. That is, I sometimes type "continue\n\n" instead of
>> "continue\n" when I want to resume the inferior. By doing so, the
>> inferior gets resumed and the extraneous "\n" gets buffered. But once
>> GDB regains control, the extraneous "\n" gets read and so an empty
>> command line is submitted to readline. This is shorthand for running
>> the previous command again. So effectively typing "continue\n\n" will
>> run "continue" twice. Running "continue" twice instead of once can have
>> irreversible consequences to the debugging session and thus is pretty
>> annoying to deal with.
>>
>> To work around this kind of typo, this patch installs a dummy event
>> handler that discards all local stdin events that occur when the
>> attached inferior is (synchronously) running.
>>
>> The obvious side-effect of this patch is that dexterous users can no
>> longer "queue" commands while an attached inferior is running, e.g.
>> type "continue\nprint foo" with the intention of "print foo" being run
>> as soon as GDB regains control. I personally don't make use of this
>> ability very much.
>
> IMO, this incompatible change of behavior should at least have an
> option to get the old behavior back, and that option should possibly
> be off by default.
>
> In any case, there should be a NEWS entry and a patch for the manual
> which describe this unusual feature.
Will do, if there is enough support for this change to go forward.
>
>> Is this typo a common occurrence for anyone else?
>
> Not common, but sometimes. However, I sometimes type ahead future
> commands without waiting for the prompt, for example, when I know I'll
> need a lot of CR presses.
Yeah, unfortunately it is impossible to determine whether a key press
was a typo or not :(
An alternative solution is to provide an option to disable the
repeat-command shorthand altogether. It is slightly unintuitive,
typo-prone, and hardly an improvement over the well-known ^P<Enter>
key combination.
Patrick Palka <patrick@parcs.ath.cx> writes:
> Is this typo a common occurrence for anyone else? Should this kind of
> typo be worked around? Any thoughts on the this particular workaround,
> or on other potential workarounds?
Use dont_repeat instead.
Andreas.
On Tue, Jun 2, 2015 at 11:49 AM, Andreas Schwab <schwab@linux-m68k.org> wrote:
> Patrick Palka <patrick@parcs.ath.cx> writes:
>
>> Is this typo a common occurrence for anyone else? Should this kind of
>> typo be worked around? Any thoughts on the this particular workaround,
>> or on other potential workarounds?
>
> Use dont_repeat instead.
I wouldn't mind marking the "continue" command as dont_repeat. One can
always repeat the last command with ^P<Enter> anyway. The shorthand
only saves one character.
>
> Andreas.
>
> --
> Andreas Schwab, schwab@linux-m68k.org
> GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
> "And now for something completely different."
@@ -434,6 +434,14 @@ stdin_event_handler (int error, gdb_client_data client_data)
}
}
+/* See event-top.h. */
+
+void
+stdin_discard_event_handler (int error, gdb_client_data client_data)
+{
+ (void) fgetc (instream);
+}
+
/* Re-enable stdin after the end of an execution command in
synchronous mode, or after an error from the target, and we aborted
the exec operation. */
@@ -47,6 +47,12 @@ extern void handle_sigterm (int sig);
extern void gdb_readline2 (void *client_data);
extern void async_request_quit (void *arg);
extern void stdin_event_handler (int error, void *client_data);
+
+/* An event handler that reads and discards one byte of data
+ from INSTREAM (usually stdin). */
+
+extern void stdin_discard_event_handler (int error, void *client_data);
+
extern void async_disable_stdin (void);
extern void async_enable_stdin (void);
@@ -66,6 +66,7 @@
#include "target-descriptions.h"
#include "filestuff.h"
#include "objfiles.h"
+#include "top.h"
#ifndef SPUFS_MAGIC
#define SPUFS_MAGIC 0x23c9b64e
@@ -4635,6 +4636,25 @@ linux_nat_terminal_inferior (struct target_ops *self)
delete_file_handler (input_fd);
async_terminal_is_ours = 0;
+
+ /* Discard any local stdin events that occur while an attached inferior is
+ (synchronously) running. These events would otherwise eventually get
+ passed to the command line once GDB regains control. This is problematic
+ because submitting an empty command line causes the previous command to
+ run again, so accidentally typing "continue\n\n" would perform "continue"
+ twice. By discarding local stdin events while an attached inferior is
+ active, typing "continue\n\n" will only perform "continue" once because
+ the 2nd "\n" will now get discarded, saving some headache for the clumsy
+ user.
+
+ This event handler will get replaced by the regular stdin event handler
+ will in the next call to target_terminal_ours(). */
+
+ if (current_inferior ()->attach_flag
+ && instream == stdin
+ && ISATTY (instream))
+ add_file_handler (input_fd, stdin_discard_event_handler, 0);
+
set_sigint_trap ();
}