From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 28804 invoked by alias); 19 May 2015 20:41:27 -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 28793 invoked by uid 89); 19 May 2015 20:41:26 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.3 required=5.0 tests=AWL,BAYES_00,KAM_LAZY_DOMAIN_SECURITY,SPF_HELO_PASS,T_RP_MATCHES_RCVD autolearn=no 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; Tue, 19 May 2015 20:41:25 +0000 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t4JKfNSb002672 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Tue, 19 May 2015 16:41:23 -0400 Received: from valrhona.uglyboxes.com (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t4JKfM13014597 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Tue, 19 May 2015 16:41:23 -0400 Message-ID: <555B9FF2.4030203@redhat.com> Date: Tue, 19 May 2015 20:41:00 -0000 From: Keith Seitz User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Doug Evans CC: gdb-patches@sourceware.org Subject: Re: [PATCH v4 7/9] Explicit locations: add UI features for CLI References: <20150507180523.19629.77846.stgit@valrhona.uglyboxes.com> <20150507180602.19629.40685.stgit@valrhona.uglyboxes.com> In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit X-IsSubscribed: yes X-SW-Source: 2015-05/txt/msg00499.txt.bz2 On 05/17/2015 11:54 PM, Doug Evans wrote: > Keith Seitz writes: >> >> diff --git a/gdb/location.c b/gdb/location.c >> index 7882b2d..779bcfa 100644 >> --- a/gdb/location.c >> +++ b/gdb/location.c >> @@ -442,6 +442,203 @@ event_location_to_string (struct event_location *location) >> return EL_STRING (location); >> } >> >> +/* A lexer for explicit locations. This function will advance INP >> + past any strings that it lexes. Returns a malloc'd copy of the >> + lexed string or NULL if no lexing was done. */ >> + >> +static char * >> +explicit_location_lex_one (const char **inp, >> + const struct language_defn *language) >> +{ >> + const char *start = *inp; >> + >> + if (*start == '\0') >> + return NULL; >> + >> + /* If quoted, skip to the ending quote. */ >> + if (strchr (get_gdb_linespec_parser_quote_characters (), *start)) >> + { >> + char quote_char = *start; >> + >> + /* If the input is not an Ada operator, skip to the matching >> + closing quote and return the string. */ >> + if (!(language->la_language == language_ada >> + && quote_char == '\"' && is_ada_operator (start))) >> + { >> + const char *end = find_toplevel_char (start + 1, quote_char); >> + >> + if (end == NULL) >> + error (_("Unmatched quote, %s."), start); >> + *inp = end + 1; >> + return savestring (start + 1, *inp - start - 2); >> + } >> + } >> + >> + /* If the input starts with '-' or '+', the string ends with the next >> + whitespace. */ >> + if (*start == '-' || *start == '+') >> + *inp = skip_to_space_const (*inp); > > I suspect this is just following what the existing code does, > but why not also watch for commas when there's a leading +,-? > If this is just following code and there's an issue here > I'd leave it for another day to change. Good catch. I've implemented this and added a few tests for it. >> +/* See description in location.h. */ >> + >> +struct event_location * >> +string_to_explicit_location (const char **argp, >> + const struct language_defn *language, >> + int dont_throw) >> +{ >> + struct cleanup *cleanup; >> + struct event_location *location; >> + >> + /* It is assumed that input beginning with '-' and a non-digit >> + character is an explicit location. */ >> + if (argp == NULL >> + || *argp == '\0' >> + || *argp[0] != '-' >> + || !isalpha ((*argp)[1])) >> + return NULL; >> + >> + location = new_explicit_location (NULL); >> + cleanup = make_cleanup_delete_event_location (location); >> + >> + /* Process option/argument pairs. dprintf_command >> + requires that processing stop on ','. */ >> + while ((*argp)[0] != '\0' && (*argp)[0] != ',') >> + { >> + int len; >> + char *opt, *oarg; >> + const char *start; >> + struct cleanup *inner; >> + >> + /* If *ARGP starts with a keyword, stop processing >> + options. */ >> + if (linespec_lexer_lex_keyword (*argp) != NULL) >> + break; >> + >> + /* Mark the start of the string in case we need to rewind. */ >> + start = *argp; >> + >> + /* Get the option string. */ >> + opt = explicit_location_lex_one (argp, language); >> + inner = make_cleanup (xfree, opt); >> + >> + *argp = skip_spaces_const (*argp); >> + >> + /* Get the argument string. */ >> + oarg = explicit_location_lex_one (argp, language); >> + >> + *argp = skip_spaces_const (*argp); >> + >> + /* Use the length of the option to allow abbreviations. */ >> + len = strlen (opt); >> + >> + /* All options have a required argument. Checking for this required >> + argument is deferred until later. */ >> + if (strncmp (opt, "-source", len) == 0) >> + EL_EXPLICIT (location)->source_filename = oarg; >> + else if (strncmp (opt, "-function", len) == 0) >> + EL_EXPLICIT (location)->function_name = oarg; >> + else if (strncmp (opt, "-line", len) == 0) >> + { >> + if (oarg != NULL) >> + { >> + TRY >> + { >> + EL_EXPLICIT (location)->line_offset >> + = linespec_parse_line_offset (oarg); >> + } >> + CATCH (e, RETURN_MASK_ERROR) >> + { >> + xfree (oarg); > > Could other exception types leak oarg here? Not that I can see. When any successful argument value is parsed, it is added to the event_location, which has a cleanup on it which will free any defined members when an exception occurs. The only two functions (in this loop) that could throw an exception are explicit_location_lex_one and linespec_parse_line_offset. In the former case, the option name has a cleanup when parsing the value. The value is either saved into the event_location or discarded if we are going to throw an exception. linespec_parse_line_offset can throw an error (GENERIC_ERROR), but it is already caught and memory for oarg is freed. Nothing can generate a RETURN_QUIT as far as I can tell. Did you have a case specifically in mind? Keith