Re: [RFA] fix "server" command prefix handling (unexpected confirmation queries)

[Joel Brobecker <brobecker@adacore.com>]

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

> > gdb/ChangeLog:
> > 
> >         * event-top.c (handle_line_of_input): Set server_command.
> > 
> > gdb/testsuite/ChangeLog:
> > 
> >         * gdb.base/server-del-break.c: New file.
> >         * gdb.base/server-del-break.exp: New file.
> > 
> > Tested on x86_64-linux, no regression.
> > OK to push to master?
> 
> LGTM, with a couple easy tweaks to the testcase.  See below.

Thanks, Pedro. I added the comments to the testcase and also fixed
the oversight of not using standard_testfile + prepare_for_testing
(it shows I am getting rusty, which is no big surprise :-/).

Attached is what I ended up pushing. I can make further adjustments
if needed.

Thanks again!
-- 
Joel

[-- Attachment #2: 0001-fix-server-command-prefix-handling-unexpected-confir.patch --]
[-- Type: text/x-diff, Size: 6499 bytes --]

From 9937536c23ae005422cec23d38a6b7f6fa8e1f48 Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Mon, 11 Dec 2017 23:51:29 -0500
Subject: [PATCH] fix "server" command prefix handling (unexpected confirmation
 queries)

The "server" command prefix no longer turns confirmation queries off.
We can reproduce this with any program by tring to delete all breakpoints,
for instance:

    (gdb) break main
    Breakpoint 1 at 0x40049b: file /[...]/break-fun-addr1.c, line 21.
    (gdb) server delete breakpoints
    Delete all breakpoints? (y or n)

GDB should not be asking "Delete all breakpoints? (y or n)", but
instead just delete all breakpoints without asking for confirmation.

Looking at utils.c::defaulted_query gives a glimpse of how this feature
is expected to work:

  /* Automatically answer the default value if the user did not want
     prompts or the command was issued with the server prefix.  */
  if (!confirm || server_command)
    return def_value;

So, it relies on the server_command global to be set when the "server "
command prefix is used, which is no longer the case since the following
commit:

    commit b69d38afdea34e4fecab5ea47ffe1e594e0b6233
    Date:   Wed Mar 9 18:25:00 2016 +0000
    Subject: Command line input handling TLC

The patch was simplifying the handling for the command line, and
I believe there was just a small oversight of removing the setting
of the server_command global.

This patch restores that, and adds a testcase to make sure we test
that feature.

gdb/ChangeLog:

        * event-top.c (handle_line_of_input): Set server_command.

gdb/testsuite/ChangeLog:

        * gdb.base/server-del-break.c: New file.
        * gdb.base/server-del-break.exp: New file.

Tested on x86_64-linux, no regression.
---
 gdb/ChangeLog                               |  4 +++
 gdb/event-top.c                             |  3 ++-
 gdb/testsuite/ChangeLog                     |  5 ++++
 gdb/testsuite/gdb.base/server-del-break.c   | 22 +++++++++++++++++
 gdb/testsuite/gdb.base/server-del-break.exp | 38 +++++++++++++++++++++++++++++
 5 files changed, 71 insertions(+), 1 deletion(-)
 create mode 100644 gdb/testsuite/gdb.base/server-del-break.c
 create mode 100644 gdb/testsuite/gdb.base/server-del-break.exp

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 201a9f3506..9fcf0c8d54 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,7 @@
+2017-12-13  Joel Brobecker  <brobecker@adacore.com>
+
+	* event-top.c (handle_line_of_input): Set server_command.
+
 2017-12-12  Stafford Horne  <shorne@gmail.com>
 	    Stefan Wallentowitz  <stefan@wallentowitz.de>
 	    Stefan Kristiansson  <stefan.kristiansson@saunalahti.fi>
diff --git a/gdb/event-top.c b/gdb/event-top.c
index 89936965a9..33e0ac4882 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -669,7 +669,8 @@ handle_line_of_input (struct buffer *cmd_line_buffer,
     }
 
 #define SERVER_COMMAND_PREFIX "server "
-  if (startswith (cmd, SERVER_COMMAND_PREFIX))
+  server_command = startswith (cmd, SERVER_COMMAND_PREFIX);
+  if (server_command)
     {
       /* Note that we don't set `saved_command_line'.  Between this
          and the check in dont_repeat, this insures that repeating
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index c17bff78df..c4d8ecf35b 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2017-12-13  Joel Brobecker  <brobecker@adacore.com>
+
+	* gdb.base/server-del-break.c: New file.
+	* gdb.base/server-del-break.exp: New file.
+
 2017-12-12  Stafford Horne  <shorne@gmail.com>
 
 	* gdb.xml/tdesc-regs.exp: Add or1k support.
diff --git a/gdb/testsuite/gdb.base/server-del-break.c b/gdb/testsuite/gdb.base/server-del-break.c
new file mode 100644
index 0000000000..2fee6961d5
--- /dev/null
+++ b/gdb/testsuite/gdb.base/server-del-break.c
@@ -0,0 +1,22 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2017 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/>.  */
+
+int
+main (void)
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/server-del-break.exp b/gdb/testsuite/gdb.base/server-del-break.exp
new file mode 100644
index 0000000000..0933834ebe
--- /dev/null
+++ b/gdb/testsuite/gdb.base/server-del-break.exp
@@ -0,0 +1,38 @@
+# Copyright 2017 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/>.
+
+# The purpose of this testcase is to verify that using the "server"
+# command prefix turns queries (confirmation requests, for instance)
+# off.  In this particular testcase, we use the "delete breakpoints"
+# command to demonstrate the behavior.
+
+standard_testfile
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
+    return -1
+}
+
+gdb_test "break main" \
+         "Breakpoint.*at.* file .*$srcfile, line .*"
+
+# Try deleting all breakpoints, using the "server" command prefix.
+# Prefixing the "delete breakpoints" with "server" should turn
+# the confirmation request ("Delete all breakpoints? (y or n)")
+# off, hence we expect the operation to be executed without output.
+gdb_test_no_output "server delete breakpoints"
+
+# Double-check that the all breakpoints were in fact deleted.
+gdb_test "info break" \
+         "No breakpoints or watchpoints."
-- 
2.11.0