[binutils-gdb] gdb: 'target ...' commands now expect quoted/escaped filenames

Andrew Burgess aburgess@sourceware.org
Sat Sep 7 19:55:40 GMT 2024


https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=03ad29c86c232484f9090582bbe6f221bc87c323

commit 03ad29c86c232484f9090582bbe6f221bc87c323
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Wed Jun 19 11:14:08 2024 +0100

    gdb: 'target ...' commands now expect quoted/escaped filenames
    
    This commit changes the 'target ...' commands that accept a filename
    to take a quoted or escaped filename rather than a literal filename.
    
    What this means in practice is that if you are specifying a filename
    that contains no white space or quote characters, then nothing should
    change, e.g.:
    
      target exec /path/to/some/file
    
    works both before and after this commit.
    
    However, if a user wishes to specify a file containing white space
    then either the entire filename needs to be quoted, or the special
    white space needs to be escaped.  Before this patch a user could
    write:
    
      target exec /path/to a file/containing spaces
    
    But after this commit the user would have to choose one of:
    
      target exec "/path/to a file/containing spaces"
    
    or
    
      target exec /path/to\ a\ file/containing\ spaces
    
    Obviously this is a potentially breaking change.  The benefit of
    making this change is consistency.  Commands that take multiple
    arguments (one of which is a filename) or in the future, commands that
    take filename options, will always need to use quoted/escaped
    filenames, so converting all unquoted filename commands to use quoting
    or escaping makes the UI more consistent.
    
    Additionally (though this is probably not a common problem), GDB
    strips trailing white space from commands that the user enters.  As
    such it is not possible to reference any file that ends in white space
    unless the quoting / escaping style is used.  Though I suspect very
    few users run into this problem!
    
    The downside obviously is that this is a UI breaking change.
    
    Reviewed-By: Eli Zaretskii <eliz@gnu.org>

Diff:
---
 gdb/NEWS                                       |  4 ++++
 gdb/corelow.c                                  | 23 ++++++++++++-----------
 gdb/doc/gdb.texinfo                            |  9 +++++++++
 gdb/exec.c                                     |  7 +++++--
 gdb/testsuite/gdb.base/batch-exit-status.exp   |  4 ++--
 gdb/testsuite/gdb.base/filename-completion.exp |  4 ++--
 gdb/tracectf.c                                 | 11 ++++++-----
 gdb/tracefile-tfile.c                          | 16 ++++++++--------
 8 files changed, 48 insertions(+), 30 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 87856131f77..19e05af625a 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -63,6 +63,10 @@ remove-symbol-file -a ADDRESS
   multiple terms, e.g. 'function + 0x1000' (without quotes),
   previously only a single term could be given.
 
+target core
+target exec
+target tfile
+target ctf
 compile file
 maint print c-tdesc
 save gdb-index
diff --git a/gdb/corelow.c b/gdb/corelow.c
index 99a61fc41b5..16f07c23faa 100644
--- a/gdb/corelow.c
+++ b/gdb/corelow.c
@@ -860,7 +860,10 @@ core_target_open (const char *arg, int from_tty)
   int flags;
 
   target_preopen (from_tty);
-  if (!arg)
+
+  std::string filename = extract_single_filename_arg (arg);
+
+  if (filename.empty ())
     {
       if (current_program_space->core_bfd ())
 	error (_("No core file specified.  (Use `detach' "
@@ -869,25 +872,23 @@ core_target_open (const char *arg, int from_tty)
 	error (_("No core file specified."));
     }
 
-  gdb::unique_xmalloc_ptr<char> filename (tilde_expand (arg));
-  if (strlen (filename.get ()) != 0
-      && !IS_ABSOLUTE_PATH (filename.get ()))
-    filename = make_unique_xstrdup (gdb_abspath (filename).c_str ());
+  if (!IS_ABSOLUTE_PATH (filename.c_str ()))
+    filename = gdb_abspath (filename);
 
   flags = O_BINARY | O_LARGEFILE;
   if (write_files)
     flags |= O_RDWR;
   else
     flags |= O_RDONLY;
-  scratch_chan = gdb_open_cloexec (filename.get (), flags, 0).release ();
+  scratch_chan = gdb_open_cloexec (filename.c_str (), flags, 0).release ();
   if (scratch_chan < 0)
-    perror_with_name (filename.get ());
+    perror_with_name (filename.c_str ());
 
-  gdb_bfd_ref_ptr temp_bfd (gdb_bfd_fopen (filename.get (), gnutarget,
+  gdb_bfd_ref_ptr temp_bfd (gdb_bfd_fopen (filename.c_str (), gnutarget,
 					   write_files ? FOPEN_RUB : FOPEN_RB,
 					   scratch_chan));
   if (temp_bfd == NULL)
-    perror_with_name (filename.get ());
+    perror_with_name (filename.c_str ());
 
   if (!bfd_check_format (temp_bfd.get (), bfd_core))
     {
@@ -896,7 +897,7 @@ core_target_open (const char *arg, int from_tty)
 	 thing, on error it does not free all the storage associated
 	 with the bfd).  */
       error (_("\"%s\" is not a core dump: %s"),
-	     filename.get (), bfd_errmsg (bfd_get_error ()));
+	     filename.c_str (), bfd_errmsg (bfd_get_error ()));
     }
 
   current_program_space->cbfd = std::move (temp_bfd);
@@ -1940,7 +1941,7 @@ void
 _initialize_corelow ()
 {
   add_target (core_target_info, core_target_open,
-	      deprecated_filename_completer);
+	      filename_maybe_quoted_completer);
   add_cmd ("core-file-backed-mappings", class_maintenance,
 	   maintenance_print_core_file_backed_mappings,
 	   _("Print core file's file-backed mappings."),
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 7721e236712..780eaf41f16 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -16245,6 +16245,9 @@ the data was saved, as well as the current trace frame you are examining.
 Both @var{filename} and @var{dirname} must be on a filesystem accessible to
 the host.
 
+The @var{filename} and @var{dirname} arguments supports escaping and
+quoting, see @ref{Filename Arguments,,Filenames As Command Arguments}.
+
 @smallexample
 (@value{GDBP}) target ctf ctf.ctf
 (@value{GDBP}) tfind
@@ -23167,11 +23170,17 @@ configuration):
 An executable file.  @samp{target exec @var{program}} is the same as
 @samp{exec-file @var{program}}.
 
+The @var{program} argument supports escaping and quoting, see
+@ref{Filename Arguments,,Filenames As Command Arguments}.
+
 @item target core @var{filename}
 @cindex core dump file target
 A core dump file.  @samp{target core @var{filename}} is the same as
 @samp{core-file @var{filename}}.
 
+The @var{filename} argument supports escaping and quoting, see
+@ref{Filename Arguments,,Filenames As Command Arguments}.
+
 @item target remote @var{medium}
 @cindex remote target
 A remote system connected to @value{GDBN} via a serial line or network
diff --git a/gdb/exec.c b/gdb/exec.c
index 91f92b094a3..82d9266b7e3 100644
--- a/gdb/exec.c
+++ b/gdb/exec.c
@@ -143,7 +143,10 @@ static void
 exec_target_open (const char *args, int from_tty)
 {
   target_preopen (from_tty);
-  exec_file_attach (args, from_tty);
+
+  std::string filename = extract_single_filename_arg (args);
+  exec_file_attach (filename.empty () ? nullptr : filename.c_str (),
+		    from_tty);
 }
 
 /* This is the target_close implementation.  Clears all target
@@ -1120,5 +1123,5 @@ will be loaded as well."),
 			&setlist, &showlist);
 
   add_target (exec_target_info, exec_target_open,
-	      deprecated_filename_completer);
+	      filename_maybe_quoted_completer);
 }
diff --git a/gdb/testsuite/gdb.base/batch-exit-status.exp b/gdb/testsuite/gdb.base/batch-exit-status.exp
index 3721fd56018..51514640958 100644
--- a/gdb/testsuite/gdb.base/batch-exit-status.exp
+++ b/gdb/testsuite/gdb.base/batch-exit-status.exp
@@ -92,5 +92,5 @@ test_exit_status 1 "-batch -x $good_commands -ex \"set not-a-thing 4\"" \
 set test "No such file or directory"
 set no_such_re ": $test\\."
 test_exit_status 1 "-batch \"\"" "1x: $test" ^[multi_line $no_such_re ""]$
-test_exit_status 1 "-batch \"\" \"\"" "2x: $test" \
-    ^[multi_line $no_such_re $no_such_re ""]$
+test_exit_status 1 "-batch \"\" \"\"" "$test and No core file specified" \
+    ^[multi_line $no_such_re "No core file specified\\." ""]$
diff --git a/gdb/testsuite/gdb.base/filename-completion.exp b/gdb/testsuite/gdb.base/filename-completion.exp
index d9943afb2e0..95a9fbaa857 100644
--- a/gdb/testsuite/gdb.base/filename-completion.exp
+++ b/gdb/testsuite/gdb.base/filename-completion.exp
@@ -124,6 +124,7 @@ proc run_quoting_and_escaping_tests { root } {
     # which require whitespace to be escaped in unquoted filenames.
     foreach_with_prefix cmd { file exec-file symbol-file add-symbol-file \
 				  remove-symbol-file \
+				  "target core" "target exec" "target tfile" \
 				  "maint print c-tdesc" "compile file" \
 				  "save gdb-index" "save gdb-index -dwarf-5" } {
 	gdb_start
@@ -300,8 +301,7 @@ proc run_unquoted_tests_core { root cmd { prefix "" } } {
 proc run_unquoted_tests { root } {
     # Test all the commands which allow quoting of filenames, and
     # which require whitespace to be escaped in unquoted filenames.
-    foreach_with_prefix cmd { "set logging file" \
-				  "target core" "add-auto-load-safe-path" } {
+    foreach_with_prefix cmd { "set logging file" "add-auto-load-safe-path" } {
 	run_unquoted_tests_core $root $cmd
     }
 
diff --git a/gdb/tracectf.c b/gdb/tracectf.c
index 25a8fe7ead9..b4997f8589f 100644
--- a/gdb/tracectf.c
+++ b/gdb/tracectf.c
@@ -1111,7 +1111,7 @@ ctf_read_tp (struct uploaded_tp **uploaded_tps)
    second packet which contains events on trace blocks.  */
 
 static void
-ctf_target_open (const char *dirname, int from_tty)
+ctf_target_open (const char *args, int from_tty)
 {
   struct bt_ctf_event *event;
   uint32_t event_id;
@@ -1119,10 +1119,11 @@ ctf_target_open (const char *dirname, int from_tty)
   struct uploaded_tsv *uploaded_tsvs = NULL;
   struct uploaded_tp *uploaded_tps = NULL;
 
-  if (!dirname)
+  std::string dirname = extract_single_filename_arg (args);
+  if (dirname.empty ())
     error (_("No CTF directory specified."));
 
-  ctf_open_dir (dirname);
+  ctf_open_dir (dirname.c_str ());
 
   target_preopen (from_tty);
 
@@ -1162,7 +1163,7 @@ ctf_target_open (const char *dirname, int from_tty)
   start_pos = bt_iter_get_pos (bt_ctf_get_iter (ctf_iter));
   gdb_assert (start_pos->type == BT_SEEK_RESTORE);
 
-  trace_dirname = make_unique_xstrdup (dirname);
+  trace_dirname = make_unique_xstrdup (dirname.c_str ());
   current_inferior ()->push_target (&ctf_ops);
 
   inferior_appeared (current_inferior (), CTF_PID);
@@ -1722,6 +1723,6 @@ _initialize_ctf ()
 {
 #if HAVE_LIBBABELTRACE
   add_target (ctf_target_info, ctf_target_open,
-	      deprecated_filename_completer);
+	      filename_maybe_quoted_completer);
 #endif
 }
diff --git a/gdb/tracefile-tfile.c b/gdb/tracefile-tfile.c
index 66769216a6b..b59b5c73325 100644
--- a/gdb/tracefile-tfile.c
+++ b/gdb/tracefile-tfile.c
@@ -462,24 +462,24 @@ tfile_target_open (const char *arg, int from_tty)
   struct uploaded_tsv *uploaded_tsvs = NULL;
 
   target_preopen (from_tty);
-  if (!arg)
+  std::string filename = extract_single_filename_arg (arg);
+  if (filename.empty ())
     error (_("No trace file specified."));
 
-  gdb::unique_xmalloc_ptr<char> filename (tilde_expand (arg));
-  if (!IS_ABSOLUTE_PATH (filename.get ()))
-    filename = make_unique_xstrdup (gdb_abspath (filename).c_str ());
+  if (!IS_ABSOLUTE_PATH (filename.c_str ()))
+    filename = gdb_abspath (filename);
 
   flags = O_BINARY | O_LARGEFILE;
   flags |= O_RDONLY;
-  scratch_chan = gdb_open_cloexec (filename.get (), flags, 0).release ();
+  scratch_chan = gdb_open_cloexec (filename.c_str (), flags, 0).release ();
   if (scratch_chan < 0)
-    perror_with_name (filename.get ());
+    perror_with_name (filename.c_str ());
 
   /* Looks semi-reasonable.  Toss the old trace file and work on the new.  */
 
   current_inferior ()->unpush_target (&tfile_ops);
 
-  trace_filename = std::move (filename);
+  trace_filename = make_unique_xstrdup (filename.c_str ());
   trace_fd = scratch_chan;
 
   /* Make sure this is clear.  */
@@ -1121,5 +1121,5 @@ void
 _initialize_tracefile_tfile ()
 {
   add_target (tfile_target_info, tfile_target_open,
-	      deprecated_filename_completer);
+	      filename_maybe_quoted_completer);
 }


More information about the Gdb-cvs mailing list