Re: [RFA 3/5] Explicit linespecs - implementation

[Sergio Durigan Junior <sergiodj at redhat dot com>]

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  <keiths@redhat.com>
>
> 	* 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 <location>"));
> -  if (oind < argc - 1)
> -    error (_("-break-insert: Garbage following <location>"));
> -  address = argv[oind];
> +  if (explicit)
> +    {
> +      if (oind < argc)
> +	error (_("-break-insert: Garbage following explicit linesepc"));
                                                            ^^^^^^^^

"linespec".

Thanks,

-- 
Sergio