From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15263 invoked by alias); 27 Jul 2012 04:31:17 -0000 Received: (qmail 15242 invoked by uid 22791); 27 Jul 2012 04:31:13 -0000 X-SWARE-Spam-Status: No, hits=-6.3 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,RCVD_IN_DNSWL_HI,RCVD_IN_HOSTKARMA_W,SPF_HELO_PASS,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 27 Jul 2012 04:30:54 +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 q6R4UsRI002396 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Fri, 27 Jul 2012 00:30:54 -0400 Received: from psique (ovpn-113-52.phx2.redhat.com [10.3.113.52]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id q6R4UqV7024904; Fri, 27 Jul 2012 00:30:53 -0400 From: Sergio Durigan Junior To: Keith Seitz Cc: "gdb-patches\@sourceware.org ml" Subject: Re: [RFA 3/5] Explicit linespecs - implementation References: <50120F6C.2000308@redhat.com> X-URL: http://www.redhat.com Date: Fri, 27 Jul 2012 04:31:00 -0000 In-Reply-To: <50120F6C.2000308@redhat.com> (Keith Seitz's message of "Thu, 26 Jul 2012 20:47:56 -0700") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-IsSubscribed: yes 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 X-SW-Source: 2012-07/txt/msg00665.txt.bz2 On Friday, July 27 2012, Keith Seitz wrote: > Hi, > > Ok, so here's what everyone has been waiting for. This patch adds the > implementation of explicit linespecs, including CLI and MI support. > > Reminder: You must apply the previous patch (breakpoint-api) before > applying this patch. > > Questions/comments/concerns? Not really a review, but a nitpicking instead. > ChangeLog > 2012-07-26 Keith Seitz > > * breakpoint.h (struct breakpoint): Add EXPLICIT. > * breakpoint.c (update_breakpoints_after_exec): > Don't delete the breakpoint if B->EXPLICIT is non-NULL. > (print_breakpoint_location): For pending explicit breakpoints, > use explicit_linespec_to_string to print out an appropriate > address string. > (explicit_linespec_unsupported): New function. > (init_breakpoint_sal): If ELS in non-NULL, copy it into > the struct breakpoint. > (parse_breakpoint_sals): If ELS is non-NULL, call > decode_explicit_linespec instead of decode_line_full. > (create_breakpoint_1): Deal with explicit linespecs. > (break_command_1): Check for an explicit linespec. > (say_where): For pending explicit breakpoints, use > explicit_linepsec_to_string to print out an address string. ^^^^^^^^ "linespec". > diff --git a/gdb/linespec.h b/gdb/linespec.h > index 51b111b..95da829 100644 > --- a/gdb/linespec.h > +++ b/gdb/linespec.h > @@ -96,6 +96,10 @@ struct linespec_result > by the user. This will be freed by destroy_linespec_result. */ > char *addr_string; > > + /* The explicit linespec returned by the parser or NULL if the > + input could not be parsed. */ > + struct explicit_linespec *explicit; > + > /* The sals. The vector will be freed by > destroy_linespec_result. */ > VEC (linespec_sals) *sals; > @@ -114,6 +118,27 @@ extern void destroy_linespec_result (struct linespec_result *); > extern struct cleanup * > make_cleanup_destroy_linespec_result (struct linespec_result *); > > +/* Allocate and initialize a new explicit_linespec. */ > +extern explicit_linespec *new_explicit_linespec (void); > + > +/* Free the explicit linespec represented by OBJ. */ > +extern void free_explicit_linespec (void *obj); > + > +/* Copy the given explicit linespec. Free memory with > + free_explicit_linespec. */ > +extern explicit_linespec *copy_explicit_linespec (explicit_linespec *src); > + > +/* Return a string representation of the explicit linespec ELS, > + appending ARG (conditional/thread/task). */ > +extern char *explicit_linespec_to_string (explicit_linespec *els, > + const char *arg); > + > +/* Attempt to convert the string in *ARGP into an explicit_linespec. > + Returns the explicit linespec (which must be free'd by the caller) and > + advacnes ARGP over all processed input. Returns NULL if *ARGP does > + not describe an explicit linespec. */ > +explicit_linespec *string_to_explicit_linespec (char **argp); There should be a space between the comment and the function prototype. Also, the comments are in both .c and .h, and I believe you could just leave them in one of these files. > /* Decode a linespec using the provided default symtab and line. */ > > extern struct symtabs_and_lines > @@ -162,6 +187,13 @@ extern void decode_line_full (char **argptr, int flags, > const char *select_mode, > const char *filter); > > +extern void decode_explicit_linespec (struct explicit_linespec *els, int flags, > + struct symtab *default_symtab, > + int default_line, > + struct linespec_result *canonical, > + const char *select_mode, > + const char *filter); Missing comment. > diff --git a/gdb/mi/mi-cmd-break.c b/gdb/mi/mi-cmd-break.c > index 52ea90c..b96074d 100644 > --- a/gdb/mi/mi-cmd-break.c > +++ b/gdb/mi/mi-cmd-break.c > - if (oind >= argc) > + if (oind >= argc && !explicit) > error (_("-break-insert: Missing ")); > - if (oind < argc - 1) > - error (_("-break-insert: Garbage following ")); > - address = argv[oind]; > + if (explicit) > + { > + if (oind < argc) > + error (_("-break-insert: Garbage following explicit linesepc")); ^^^^^^^^ "linespec". Thanks, -- Sergio