From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 57573 invoked by alias); 7 May 2015 17:07:33 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 57560 invoked by uid 89); 7 May 2015 17:07:32 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Thu, 07 May 2015 17:07:31 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t47H7SU1029692 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Thu, 7 May 2015 13:07:29 -0400 Received: from valrhona.uglyboxes.com (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t47H7SBR004014 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Thu, 7 May 2015 13:07:28 -0400 Message-ID: <554B9BD0.3000702@redhat.com> Date: Thu, 07 May 2015 17:07:00 -0000 From: Keith Seitz User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 MIME-Version: 1.0 To: Doug Evans CC: gdb-patches@sourceware.org Subject: Re: [PATCH v3 7/9] Explicit locations: add UI features for CLI References: <20150217220619.1312.39861.stgit@valrhona.uglyboxes.com> <20150217220653.1312.96831.stgit@valrhona.uglyboxes.com> In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2015-05/txt/msg00150.txt.bz2 On 03/01/2015 01:14 PM, Doug Evans wrote: > Keith Seitz writes: > >> +/* A helper function to collect explicit location matches for the given >> + LOCATION, which is attempting to match on WORD. */ >> + >> +static VEC (char_ptr) * >> +collect_explicit_location_matches (struct event_location *location, >> + enum explicit_location_match_type what, >> + const char *word) >> +{ [snip] >> + case MATCH_LABEL: >> + /* Not supported. */ >> + break; >> + >> + default: >> + gdb_assert_not_reached (_("unhandled explicit_location_match_type")); > > ==== > No need for _() here. Fixed. > >> + } >> + >> + return matches; >> +} >> + >> +/* A convenience macro to (safely) back up P to the previous >> + word. */ > > ==== > This comment can be just one line. > I'd rather a comment run past the soft limit (and still fit within the > hard limit) than use an extra line just for one word. > Fixed. >> + >> +static const char * >> +backup_text_ptr (const char *p, const char *text) >> +{ >> + while (p > text && isspace (*p)) >> + --p; >> + for (; p > text && !isspace (p[-1]); --p) >> + ; >> + >> + return p; >> +} >> + > > ==== > Missing function comment. > Doh! Added. >> +static VEC (char_ptr) * >> +explicit_location_completer (struct cmd_list_element *ignore, >> + struct event_location *location, >> + const char *text, const char *word) >> +{ >> + const char *p; >> + VEC (char_ptr) *matches = NULL; >> + >> + /* Find the beginning of the word. This is necessary because >> + we need to know if we are completing an option name or value. We >> + don't get the leading '-' from the completer. */ >> + p = backup_text_ptr (word, text); >> + >> + if (*p == '-') >> + { >> + size_t len = strlen (word); >> + >> + /* Completing on option name. */ > > ==== > I couldn't get "b -" to work. Can do? > Unfortunately, "-" is a valid linespec, so no, "b -" will not do anything. >> + >> + /* Skip over the '-'. */ >> + ++p; >> + >> + if (strncmp (p, "source", len) == 0) [snip] >> + /* Now gather matches */ >> + matches = collect_explicit_location_matches (location, what, new_word); > > ==== > Something's not right here as "b -f " segv's because new_word is NULL. > Yes, indeed. The struct explicit_location uses NULL for unspecified parameters, and these consequently get passed as new_word to collect_explicit_location_matches, which passes it to GDB's completion functions, which do NOT like NULL. I've fixed this and added tests for them. Thank you for catching this serious bug. Keith