Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Andrew Burgess <aburgess@redhat.com>
To: gdb-patches@sourceware.org
Cc: Andrew Burgess <aburgess@redhat.com>
Subject: [PATCH 2/5] gdb: fix bug where quote characters would become nullptr
Date: Tue, 16 Jan 2024 21:23:59 +0000	[thread overview]
Message-ID: <c8a4ea244587334cdc6cc7d2d138ee18819bea82.1705439706.git.aburgess@redhat.com> (raw)
In-Reply-To: <cover.1705439706.git.aburgess@redhat.com>

In gdb_completion_word_break_characters_throw, after calling
complete_line_internal, if the completion function chose to use a
custom word point then we set rl_completer_quote_characters to NULL.

However, nowhere do we set rl_completer_quote_characters back to its
default value, which is setup in init_main (top.c).

An example of something that uses a custom word point for its
completion is 'thread apply all ...'.

An example of something that relies on rl_completer_quote_characters
would be completion of a quoted filename that contains white space.

Consider this shell and GDB session.  The <TAB> markers indicate where
I've used tab to trigger completion:

  $ mkdir /tmp/aaa\ bbb
  $ touch /tmp/aaa\ bbb/xx\ 11
  $ touch /tmp/aaa\ bbb/xx\ 22
  $ gdb -q
  (gdb) file '/tmp/aaa bbb/xx<TAB><TAB>
  xx 11  xx 22
  (gdb) thread apply all hel<TAB>
  (gdb) thread apply all help
  (gdb) file '/tmp/aaa bbb/xx<TAB><TAB>

First I create a directory structure which uses white space within
file and directory names.  Then within GDB I use the 'file' command
and use a single quote to quote the filename.  When I tab complete GDB
correctly offers the two files within the directory '/tmp/aaa bbb/'.

This works because rl_completer_quote_characters contains the single
quote, and so readline knows that it is trying to complete the string
that starts after the single quote: /tmp/aaa bbb/xx

Next I invoke the completer for the 'thread apply all' command, to do
this I type 'thread apply all hel' and hit tab, this expands to the
one completion 'thread apply all help'.  We can run this command or
not, it doesn't matter (there are no threads, so we'll get no output).

Now I repeat the original 'file' completion.  This time though I don't
get offered any completions.

The reason is that the 'thread apply all' completer set
rl_completer_quote_characters to nullptr.  Now, when readline tries to
figure out the word to complete it doesn't see the single quote as the
start of a quoted word, so instead readline falls back to the word
break characters, and in this case spots the white space.  As a result
readline tries to complete the string 'bbb/xx' which obviously doesn't
have any completions.

By setting rl_completer_quote_characters each time completion is
invoked this problem is resolved and the second 'file' command
completes as expected.

I've extended gdb.base/filename-completion.exp to also test with
quoted filenames, and added a 'thread apply all' completion at the
start to expose this bug.

As setting of rl_completer_quote_characters is now all done in the
completer.c file the function get_gdb_completer_quote_characters()
could be made static.  However, as this function is only used one time
to initialise rl_completer_quote_characters, I've instead just deleted
get_gdb_completer_quote_characters() and used
gdb_completer_quote_characters directly.
---
 gdb/completer.c                               | 10 +--
 gdb/completer.h                               |  2 -
 .../gdb.base/filename-completion.exp          | 66 +++++++++++++------
 gdb/top.c                                     |  1 -
 4 files changed, 49 insertions(+), 30 deletions(-)

diff --git a/gdb/completer.c b/gdb/completer.c
index 8168f79de0e..44da6548eb4 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -181,13 +181,6 @@ static const char gdb_completer_file_name_break_characters[] =
    sequences as strings.  */
 static const char gdb_completer_quote_characters[] = "'";
 \f
-/* Accessor for some completer data that may interest other files.  */
-
-const char *
-get_gdb_completer_quote_characters (void)
-{
-  return gdb_completer_quote_characters;
-}
 
 /* This can be used for functions which don't want to complete on
    symbols but don't want to complete on anything else either.  */
@@ -1269,6 +1262,9 @@ complete_line_internal_1 (completion_tracker &tracker,
   set_rl_completer_word_break_characters
     (current_language->word_break_characters ());
 
+  /* Likewise for the quote characters.  */
+  rl_completer_quote_characters = gdb_completer_quote_characters;
+
   /* Decide whether to complete on a list of gdb commands or on
      symbols.  */
   tmp_command = (char *) alloca (point + 1);
diff --git a/gdb/completer.h b/gdb/completer.h
index f604a95011f..0a95ced8a80 100644
--- a/gdb/completer.h
+++ b/gdb/completer.h
@@ -619,8 +619,6 @@ extern void reggroup_completer (struct cmd_list_element *,
 				completion_tracker &tracker,
 				const char *, const char *);
 
-extern const char *get_gdb_completer_quote_characters (void);
-
 extern char *gdb_completion_word_break_characters (void);
 
 /* Set the word break characters array to BREAK_CHARS.  This function
diff --git a/gdb/testsuite/gdb.base/filename-completion.exp b/gdb/testsuite/gdb.base/filename-completion.exp
index 20ca227f96d..7d8ab1a3350 100644
--- a/gdb/testsuite/gdb.base/filename-completion.exp
+++ b/gdb/testsuite/gdb.base/filename-completion.exp
@@ -37,6 +37,9 @@ proc setup_directory_tree {} {
     remote_exec host "mkdir -p ${root}/cc1"
     remote_exec host "touch ${root}/cc2"
 
+    remote_exec host "touch \"${root}/aaa/aa bb\""
+    remote_exec host "touch \"${root}/aaa/aa cc\""
+
     return $root
 }
 
@@ -45,26 +48,49 @@ proc setup_directory_tree {} {
 # sub-directory of the user's home directory ROOT might have been
 # modified to replace the $HOME prefix with a single "~" character.
 proc run_tests { root } {
-    test_gdb_complete_none "file ${root}/xx" \
-	"expand a non-existent filename"
-
-    test_gdb_complete_unique "file ${root}/a" \
-	"file ${root}/aaa/" "" false \
-	"expand a unique filename"
-
-    test_gdb_complete_multiple "file ${root}/" \
-	"b" "b" {
-	    "bb1/"
-	    "bb2/"
-	} "" "" false \
-	"expand multiple directory names"
-
-    test_gdb_complete_multiple "file ${root}/" \
-	"c" "c" {
-	    "cc1/"
-	    "cc2"
-	} "" "" false \
-	"expand mixed directory and file names"
+
+    # Completing 'thread apply all ...' commands uses a custom word
+    # point.  At one point we had a bug where doing this would break
+    # completion of quoted filenames that contained white space.
+    test_gdb_complete_unique "thread apply all hel" \
+	"thread apply all help" " " false \
+	"complete a 'thread apply all' command"
+
+    foreach_with_prefix qc [list "" "'"] {
+	test_gdb_complete_none "file ${qc}${root}/xx" \
+	    "expand a non-existent filename"
+
+	test_gdb_complete_unique "file ${qc}${root}/a" \
+	    "file ${qc}${root}/aaa/" "" false \
+	    "expand a unique filename"
+
+	test_gdb_complete_multiple "file ${qc}${root}/" \
+	    "b" "b" {
+		"bb1/"
+		"bb2/"
+	    } "" "${qc}" false \
+	    "expand multiple directory names"
+
+	test_gdb_complete_multiple "file ${qc}${root}/" \
+	    "c" "c" {
+		"cc1/"
+		"cc2"
+	    } "" "${qc}" false \
+	    "expand mixed directory and file names"
+
+	# GDB does not currently escape word break characters
+	# (e.g. white space) correctly in unquoted filenames.
+	if { $qc ne "" } {
+	    set sp " "
+
+	    test_gdb_complete_multiple "file ${qc}${root}/aaa/" \
+		"a" "a${sp}" {
+		    "aa bb"
+		    "aa cc"
+		} "" "${qc}" false \
+		"expand filenames containing spaces"
+	}
+    }
 }
 
 gdb_start
diff --git a/gdb/top.c b/gdb/top.c
index fb15c719564..543d6cdfbc0 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -2150,7 +2150,6 @@ init_main (void)
   rl_completion_word_break_hook = gdb_completion_word_break_characters;
   rl_attempted_completion_function = gdb_rl_attempted_completion_function;
   set_rl_completer_word_break_characters (default_word_break_characters ());
-  rl_completer_quote_characters = get_gdb_completer_quote_characters ();
   rl_completion_display_matches_hook = cli_display_match_list;
   rl_readline_name = "gdb";
   rl_terminal_name = getenv ("TERM");
-- 
2.25.4


  parent reply	other threads:[~2024-01-16 21:25 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-16 21:23 [PATCH 0/5] Cleanup and changes for file name completion Andrew Burgess
2024-01-16 21:23 ` [PATCH 1/5] gdb: remove skip_quoted and skip_quoted_chars Andrew Burgess
2024-01-16 21:23 ` Andrew Burgess [this message]
2024-01-16 21:24 ` [PATCH 3/5] gdb: allow double quotes for quoting filenames Andrew Burgess
2024-01-16 21:24 ` [PATCH 4/5] gdb: remove some dead code from completer.c Andrew Burgess
2024-01-16 21:24 ` [PATCH 5/5] gdb: remove special case completion word handling for filenames Andrew Burgess
2024-01-17 12:09   ` Eli Zaretskii
2024-01-17 16:29     ` Hannes Domani
2024-01-17 16:52       ` Eli Zaretskii
2024-01-18  9:33       ` Andrew Burgess
2024-03-06 10:23 ` [PATCHv2 0/7] Cleanup and changes for file name completion Andrew Burgess
2024-03-06 10:23   ` [PATCHv2 1/7] gdb: remove skip_quoted and skip_quoted_chars Andrew Burgess
2024-03-06 10:23   ` [PATCHv2 2/7] gdb: fix bug where quote characters would become nullptr Andrew Burgess
2024-03-06 10:23   ` [PATCHv2 3/7] gdb: allow double quotes for quoting filenames Andrew Burgess
2024-03-06 10:23   ` [PATCHv2 4/7] gdb: remove some dead code from completer.c Andrew Burgess
2024-03-06 10:23   ` [PATCHv2 5/7] gdb: remove special case completion word handling for filenames Andrew Burgess
2024-03-06 10:23   ` [PATCHv2 6/7] gdb/completion: make completion_find_completion_word static Andrew Burgess
2024-03-06 10:23   ` [PATCHv2 7/7] gdb: move more completion setup into completer.c Andrew Burgess
2024-03-25 18:30   ` [PATCHv2 0/7] Cleanup and changes for file name completion 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=c8a4ea244587334cdc6cc7d2d138ee18819bea82.1705439706.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