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

Sergio Durigan Junior
2012-07-27T04:31:17Z

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