From patchwork Fri Nov 7 22:19:58 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: 'Timothy Arceri' via Patchwork Forward X-Patchwork-Id: 40450 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-wg0-f71.google.com (mail-wg0-f71.google.com [74.125.82.71]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id A8A07218C0 for ; Fri, 7 Nov 2014 22:20:30 +0000 (UTC) Received: by mail-wg0-f71.google.com with SMTP id b13sf2353308wgh.2 for ; Fri, 07 Nov 2014 14:20:29 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:delivered-to:mailing-list:precedence:list-id :list-unsubscribe:list-subscribe:list-archive:list-post:list-help :sender:delivered-to:message-id:date:from:user-agent:mime-version:to :cc:subject:x-original-sender:x-original-authentication-results :reply-to:content-type; bh=fwymPboTDfGVoinatOXRVIotb1SmcPmUsGNLHq6v3i8=; b=jLNYrdDMwdjUQYIMlKvHrEAqVMMCoQtbsotBZ6jSvU2+C9bT36XO6KNPIYWPdPJnTW bZ0v5+Tg6HK0R1N2p5Gm4K+k/r9p48ToQjE/zHJM9oeImT6Mhe9NG/QbWsZDnXHNTSI1 wOORh4LYmRROdJEaehLpnuZXySz1sKI67TgVGme1oa5KFlTxEoeMAjfrrKcl9zmoeEUT hnY3s75H6l+/6xWSq55jamorzVmLmKZoyueQm8IlATyX9HUkgFt6xjmx4GRXUXZYOVeb npHpNlmVczbEPyap84shrFiQx/FikNkhaKjHSxpwqVsd5LzTrbyaSM8w8LCcexLSVVOy /RCA== X-Gm-Message-State: ALoCoQnob7G2QzjsRLo4GjBCfMr5I2uiaN85e2jc4OMogAz0S94AsoS45cbvKKp7AUI6tl4DX4+S X-Received: by 10.180.19.226 with SMTP id i2mr1122763wie.5.1415398829903; Fri, 07 Nov 2014 14:20:29 -0800 (PST) X-BeenThere: patchwork-forward@linaro.org Received: by 10.152.37.226 with SMTP id b2ls247076lak.36.gmail; Fri, 07 Nov 2014 14:20:29 -0800 (PST) X-Received: by 10.112.151.70 with SMTP id uo6mr13682271lbb.2.1415398829736; Fri, 07 Nov 2014 14:20:29 -0800 (PST) Received: from mail-la0-x233.google.com (mail-la0-x233.google.com. [2a00:1450:4010:c03::233]) by mx.google.com with ESMTPS id z3si16759429lad.17.2014.11.07.14.20.29 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Fri, 07 Nov 2014 14:20:29 -0800 (PST) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 2a00:1450:4010:c03::233 as permitted sender) client-ip=2a00:1450:4010:c03::233; Received: by mail-la0-f51.google.com with SMTP id q1so5155603lam.24 for ; Fri, 07 Nov 2014 14:20:29 -0800 (PST) X-Received: by 10.112.142.200 with SMTP id ry8mr13982625lbb.26.1415398829329; Fri, 07 Nov 2014 14:20:29 -0800 (PST) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patch@linaro.org Received: by 10.112.184.201 with SMTP id ew9csp281146lbc; Fri, 7 Nov 2014 14:20:27 -0800 (PST) X-Received: by 10.68.197.138 with SMTP id iu10mr6503432pbc.143.1415398826913; Fri, 07 Nov 2014 14:20:26 -0800 (PST) Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id xw5si7458126pac.182.2014.11.07.14.20.26 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 07 Nov 2014 14:20:26 -0800 (PST) Received-SPF: pass (google.com: domain of gdb-patches-return-117357-patch=linaro.org@sourceware.org designates 209.132.180.131 as permitted sender) client-ip=209.132.180.131; Received: (qmail 12508 invoked by alias); 7 Nov 2014 22:20:19 -0000 Mailing-List: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org Precedence: list List-Id: List-Unsubscribe: , List-Subscribe: List-Archive: List-Post: , List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 12494 invoked by uid 89); 7 Nov 2014 22:20:18 -0000 X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00, FREEMAIL_FROM, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.2 X-HELO: nm19-vm4.bullet.mail.ir2.yahoo.com Received: from nm19-vm4.bullet.mail.ir2.yahoo.com (HELO nm19-vm4.bullet.mail.ir2.yahoo.com) (212.82.96.237) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Fri, 07 Nov 2014 22:20:16 +0000 Received: from [212.82.98.125] by nm19.bullet.mail.ir2.yahoo.com with NNFMP; 07 Nov 2014 22:20:12 -0000 Received: from [46.228.39.80] by tm18.bullet.mail.ir2.yahoo.com with NNFMP; 07 Nov 2014 22:20:12 -0000 Received: from [127.0.0.1] by smtp117.mail.ir2.yahoo.com with NNFMP; 07 Nov 2014 22:20:12 -0000 X-Yahoo-SMTP: U4ByNJCswBDrWOndmGo3cYknl59DpQ-- Message-ID: <545D458E.1010405@yahoo.fr> Date: Fri, 07 Nov 2014 23:19:58 +0100 From: "'wapiflapi' via Patchwork Forward" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 MIME-Version: 1.0 To: gdb-patches@sourceware.org CC: sergiodj@redhat.com Subject: [PATCH] fix some bugs in python completion code X-Original-Sender: wapiflapi@yahoo.fr X-Original-Authentication-Results: mx.google.com; spf=pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 2a00:1450:4010:c03::233 as permitted sender) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org; dkim=pass header.i=@sourceware.org; dmarc=fail (p=NONE dis=NONE) header.from=yahoo.fr X-Google-Group-Id: 836684582541 X-Original-From: wapiflapi Reply-To: wapiflapi 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 * 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 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);