public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Andrew Burgess <aburgess@redhat.com>
To: gdb-patches@sourceware.org
Cc: Andrew Burgess <aburgess@redhat.com>
Subject: [PATCH 3/3] gdb/record: minor clean, remove some unneeded arguments
Date: Mon, 15 Apr 2024 15:19:28 +0100	[thread overview]
Message-ID: <4f18fd17db2a5c6f39fa8b8587e3a5fe8fe75379.1713190701.git.aburgess@redhat.com> (raw)
In-Reply-To: <cover.1713190701.git.aburgess@redhat.com>

I spotted that the two functions:

  record_full_open_1
  record_full_core_open_1

both took two arguments, neither of which are used.

I stumbled onto this while reviewing how filename_completer is used.
The 'record full restore' command uses filename_completer and invokes
the cmd_record_full_restore function.

The cmd_record_full_restore function calls core_file_command and then
record_full_open, which then calls one of the above functions.

As 'record full restore' takes a filename, this is passed to
cmd_record_full_restore, which forwards the filename to both
core_file_command and record_full_open.  However, record_full_open
never actually uses the filename that is passed in.

The record_full_open function is also used for 'target record-full'.

I propose that record_full_open should no longer expect to see any
user supplied arguments passed in (it doesn't use any).  In fact, I've
added a check that if we do get any user supplied arguments we'll
throw an error.

Now that we know record_full_open isn't being passed any user
arguments we can stop passing the arguments to record_full_open_1 and
record_full_core_open_1, this will make no user visible difference as
these arguments were not used.

It is possible that a user was previously doing:

  (gdb) target record-full blah blah blah

And this previously would work fine, the 'blah blah blah' was
ignored.  Now this will give an error.  Other than this case there
should be no user visible changes after this commit.
---
 gdb/NEWS                                     |  4 ++++
 gdb/record-full.c                            | 15 +++++++-----
 gdb/testsuite/gdb.base/record-full-error.exp | 24 ++++++++++++++++++++
 3 files changed, 37 insertions(+), 6 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/record-full-error.exp

diff --git a/gdb/NEWS b/gdb/NEWS
index feb3a37393a..942453c7064 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -36,6 +36,10 @@ set unwindonsignal on|off
 show unwindonsignal
   These commands are now aliases for the new set/show unwind-on-signal.
 
+target record-full
+  This command now gives an error if any unexpected arguments are
+  found after the command.
+
 * New commands
 
 info missing-debug-handler
diff --git a/gdb/record-full.c b/gdb/record-full.c
index 4c3667f48ba..4ef85b0cdb6 100644
--- a/gdb/record-full.c
+++ b/gdb/record-full.c
@@ -910,7 +910,7 @@ record_full_async_inferior_event_handler (gdb_client_data data)
 /* Open the process record target for 'core' files.  */
 
 static void
-record_full_core_open_1 (const char *name, int from_tty)
+record_full_core_open_1 ()
 {
   regcache *regcache = get_thread_regcache (inferior_thread ());
   int regnum = gdbarch_num_regs (regcache->arch ());
@@ -933,7 +933,7 @@ record_full_core_open_1 (const char *name, int from_tty)
 /* Open the process record target for 'live' processes.  */
 
 static void
-record_full_open_1 (const char *name, int from_tty)
+record_full_open_1 ()
 {
   if (record_debug)
     gdb_printf (gdb_stdlog, "Process record: record_full_open_1\n");
@@ -957,11 +957,14 @@ static void record_full_init_record_breakpoints (void);
 /* Open the process record target.  */
 
 static void
-record_full_open (const char *name, int from_tty)
+record_full_open (const char *args, int from_tty)
 {
   if (record_debug)
     gdb_printf (gdb_stdlog, "Process record: record_full_open\n");
 
+  if (args != nullptr)
+    error (_("trailing junk: '%s'"), args);
+
   record_preopen ();
 
   /* Reset */
@@ -971,9 +974,9 @@ record_full_open (const char *name, int from_tty)
   record_full_list->next = NULL;
 
   if (current_program_space->core_bfd ())
-    record_full_core_open_1 (name, from_tty);
+    record_full_core_open_1 ();
   else
-    record_full_open_1 (name, from_tty);
+    record_full_open_1 ();
 
   /* Register extra event sources in the event loop.  */
   record_full_async_inferior_event_token
@@ -2520,7 +2523,7 @@ static void
 cmd_record_full_restore (const char *args, int from_tty)
 {
   core_file_command (args, from_tty);
-  record_full_open (args, from_tty);
+  record_full_open (nullptr, from_tty);
 }
 
 /* Save the execution log to a file.  We use a modified elf corefile
diff --git a/gdb/testsuite/gdb.base/record-full-error.exp b/gdb/testsuite/gdb.base/record-full-error.exp
new file mode 100644
index 00000000000..63ef03dcc0e
--- /dev/null
+++ b/gdb/testsuite/gdb.base/record-full-error.exp
@@ -0,0 +1,24 @@
+# This testcase is part of GDB, the GNU debugger.
+#
+# Copyright 2024 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/>.
+
+# Check an error when passing unexpected arguments to 'target
+# record-full'.
+
+gdb_start
+
+gdb_test "target record-full blah" \
+    "trailing junk: 'blah'"
-- 
2.25.4


  parent reply	other threads:[~2024-04-15 14:19 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-15 14:19 [PATCH 0/3] Small cleanups in gdb/record*.c code Andrew Burgess
2024-04-15 14:19 ` [PATCH 1/3] gdb/record: remove unnecessary use of filename_completer Andrew Burgess
2024-04-15 14:19 ` [PATCH 2/3] gdb/record: add an assert in cmd_record_start Andrew Burgess
2024-04-15 14:19 ` Andrew Burgess [this message]
2024-04-15 14:41   ` [PATCH 3/3] gdb/record: minor clean, remove some unneeded arguments Eli Zaretskii
2024-04-16 14:12   ` Keith Seitz
2024-04-16 17:18 ` [PATCH 0/3] Small cleanups in gdb/record*.c code Tom Tromey
2024-04-17 13:48   ` Andrew Burgess

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4f18fd17db2a5c6f39fa8b8587e3a5fe8fe75379.1713190701.git.aburgess@redhat.com \
    --to=aburgess@redhat.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).