This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
[PATCH] fix some bugs in python completion code
- From: wapiflapi <wapiflapi at yahoo dot fr>
- To: gdb-patches at sourceware dot org
- Cc: sergiodj at redhat dot com
- Date: Fri, 07 Nov 2014 23:19:58 +0100
- Subject: [PATCH] fix some bugs in python completion code
- Authentication-results: sourceware.org; auth=none
The attached patch fixes several issues with auto-completion for gdb
python plugins. I cc-ed sergiodj because he contributed most of the
stuff this is touching.
First issue: sizeof/strlen confusion.
The first issue is a simple confusion between sizeof and strlen when
converting the current word to Unicode. This causes the python plugin to
only see those first bytes that fit into the size of a pointer and
returns junks if the word happens to be shorter.
This wasn't picked up in the testsuite because we never use the `word`
argument and always rely on `text`.
Second issue: incorrect value passed to python as word argument.
The second issue this patch addresses is harder to understand. The way
auto-completion works requires to first determine the break-characters
that will be used and only then run the actual completion code. Because
a completer implemented in python can request that an existing gdb
completer be used instead, we don't know for sure which characters
should be considered breaking before running the actual completion. To
handle this the python completer is called in the very beginning and its
result cached for when the actual completion should be returned.
The issue is that the python completer is given the arguments received
by the function that is supposed to set the breaking characters. At that
point the `word` parameter isn't supposed to be used and is always the
same as `text`. This is normal because it can't yet be known. Because of
this the python completer sees the wrong value for `word`. Actually it
always receives the same value as for `text` (basically the whole line).
To fix this we choose a set of default breaking characters that will be
used id they are not overwritten which happens if another completer ends
up being used. Since we set those default breaking characters we are
able to anticipate and compute the word our-self so that we are able to
pass it on to python.
This wasn't picked up in the testsuite because we never use the `word`
argument and always rely on `text`.
Third issue: case-specific duplicate code not repeated enough times.
There is some code handling the specific cases of filename and location
completion that changes the `text` pointer before calling those
completers. It is repeated twice, in both cases when a direct call to a
completer is about to be made. The problem is this isn't done in the
case it is the python completer delegating to one of those. The proposed
patch moves this code to the filename and location completers
themselves. This removes the duplicate code and ensures this should work
no matter how the completers are called.
This can be tested using testsuite/gdb.python/py-completion.py and the
completefilemethod testcase. Completing the first argument as a file
works fine but the second argument fails to auto-complete. This is
because the filename completer received the whole line instead of just
the last word in this case. With the patch this works fine.
Using python2.7 or python3.4 this patch doesn't break any of the tests
in the testsuite/gdb.python;
ChangeLog:
2014-11-07 Wannes Rombouts <wapiflapi@yahoo.fr>
* gdb/completer.c (refactor): Move code preparing for
completers to the completers themselves to ensure it is
always called.
* gdb/python/py-cmd.c (cmdpy_completer_helper): Fix bug
caused by call to sizeof instead of strlen.
(cmdpy_completer_handle_brkchars): Compute current word
and pass it to python as specified in the documentation.
Please criticize if there are issues with the way I fixed this!
Wannes `wapiflapi` Rombouts
Ps: This is my first time contributing and I hope I did everything right!
diff --git a/gdb/completer.c b/gdb/completer.c
index a0f3fa3..95fb6d8 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -118,6 +118,23 @@ filename_completer (struct cmd_list_element *ignore,
int subsequent_name;
VEC (char_ptr) *return_val = NULL;
+ const char *start;
+
+ start = text;
+ /* Many commands which want to complete on
+ file names accept several file names, as
+ in "run foo bar >>baz". So we don't want
+ to complete the entire text after the
+ command, just the last word. To this
+ end, we need to find the beginning of the
+ file name by starting at `word' and going
+ backwards. */
+ for (text = word;
+ text > start
+ && strchr (gdb_completer_file_name_break_characters, text[-1]) == NULL;
+ text--)
+ ;
+
subsequent_name = 0;
while (1)
{
@@ -197,6 +214,17 @@ location_completer (struct cmd_list_element *ignore,
const char *orig_text = text;
size_t text_len;
+ const char *start;
+
+ start = text;
+ /* Commands which complete on locations want to
+ see the entire argument. */
+ for (text = word;
+ text > start
+ && text[-1] != ' ' && text[-1] != '\t';
+ text--)
+ ;
+
/* Do we have an unquoted colon, as in "break foo.c:bar"? */
for (p = text; *p != '\0'; ++p)
{
@@ -659,42 +687,17 @@ complete_line_internal (const char *text,
rl_completer_word_break_characters =
gdb_completer_command_word_break_characters;
}
- else
+ else if (reason == handle_brkchars)
{
- /* It is a normal command; what comes after it is
- completed by the command's completer function. */
if (c->completer == filename_completer)
- {
- /* Many commands which want to complete on
- file names accept several file names, as
- in "run foo bar >>baz". So we don't want
- to complete the entire text after the
- command, just the last word. To this
- end, we need to find the beginning of the
- file name by starting at `word' and going
- backwards. */
- for (p = word;
- p > tmp_command
- && strchr (gdb_completer_file_name_break_characters, p[-1]) == NULL;
- p--)
- ;
- rl_completer_word_break_characters =
- gdb_completer_file_name_break_characters;
- }
- else if (c->completer == location_completer)
- {
- /* Commands which complete on locations want to
- see the entire argument. */
- for (p = word;
- p > tmp_command
- && p[-1] != ' ' && p[-1] != '\t';
- p--)
- ;
- }
- if (reason == handle_brkchars
- && c->completer_handle_brkchars != NULL)
+ rl_completer_word_break_characters =
+ gdb_completer_file_name_break_characters;
+ if (c->completer_handle_brkchars != NULL)
(*c->completer_handle_brkchars) (c, p, word);
- if (reason != handle_brkchars && c->completer != NULL)
+ }
+ else if (reason == handle_completions)
+ {
+ if (c->completer != NULL)
list = (*c->completer) (c, p, word);
}
}
@@ -743,34 +746,18 @@ complete_line_internal (const char *text,
if (reason != handle_brkchars)
list = complete_on_enum (c->enums, p, word);
}
- else
+
+ else if (reason == handle_brkchars)
{
- /* It is a normal command. */
if (c->completer == filename_completer)
- {
- /* See the commentary above about the specifics
- of file-name completion. */
- for (p = word;
- p > tmp_command
- && strchr (gdb_completer_file_name_break_characters,
- p[-1]) == NULL;
- p--)
- ;
- rl_completer_word_break_characters =
- gdb_completer_file_name_break_characters;
- }
- else if (c->completer == location_completer)
- {
- for (p = word;
- p > tmp_command
- && p[-1] != ' ' && p[-1] != '\t';
- p--)
- ;
- }
- if (reason == handle_brkchars
- && c->completer_handle_brkchars != NULL)
+ rl_completer_word_break_characters =
+ gdb_completer_file_name_break_characters;
+ if (c->completer_handle_brkchars != NULL)
(*c->completer_handle_brkchars) (c, p, word);
- if (reason != handle_brkchars && c->completer != NULL)
+ }
+ else if (reason == handle_completions)
+ {
+ if (c->completer != NULL)
list = (*c->completer) (c, p, word);
}
}
diff --git a/gdb/python/py-cmd.c b/gdb/python/py-cmd.c
index 5fc656e..6ebfa5b 100644
--- a/gdb/python/py-cmd.c
+++ b/gdb/python/py-cmd.c
@@ -28,6 +28,9 @@
#include "completer.h"
#include "language.h"
+/* Needed for rl_completer_word_break_characters */
+#include "readline/readline.h"
+
/* Struct representing built-in completion types. */
struct cmdpy_completer
{
@@ -269,7 +272,7 @@ cmdpy_completer_helper (struct cmd_list_element *command,
textobj = PyUnicode_Decode (text, strlen (text), host_charset (), NULL);
if (textobj == NULL)
error (_("Could not convert argument to Python string."));
- wordobj = PyUnicode_Decode (word, sizeof (word), host_charset (), NULL);
+ wordobj = PyUnicode_Decode (word, strlen (word), host_charset (), NULL);
if (wordobj == NULL)
{
Py_DECREF (textobj);
@@ -306,6 +309,28 @@ cmdpy_completer_handle_brkchars (struct cmd_list_element *command,
cleanup = ensure_python_env (get_current_arch (), current_language);
+ /* The following line sets the default break characters in case the
+ python complete method chooses to do the completion itself.
+ This is optional and can be left out to keep the language defaults.
+
+ TODO: We should let the user be able to choose and/or know this.
+
+ Because if a python completer chooses to do the completion instead
+ of relying on for example COMPLETE_EXPRESSION, chances are it is
+ doing something which expects more <shell> like arguments. We use
+ the same reduced set of breaking characters for this as for files.
+ */
+
+ set_gdb_completion_word_break_characters (filename_completer);
+
+ /* Because this is the first time we are called we received a dummy
+ value for word (we aren't supposed to need it). Since we use it
+ we need to compute its correct value ourselves. */
+ word = text + strlen (text);
+ while (word > text
+ && strchr (rl_completer_word_break_characters, word[-1]) == NULL)
+ word--;
+
/* Calling our helper to obtain the PyObject of the Python
function. */
resultobj = cmdpy_completer_helper (command, text, word, 1);