From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 18515 invoked by alias); 16 Mar 2010 13:17:39 -0000 Received: (qmail 18501 invoked by uid 22791); 16 Mar 2010 13:17:37 -0000 X-SWARE-Spam-Status: No, hits=-2.4 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 16 Mar 2010 13:17:32 +0000 Received: (qmail 7738 invoked from network); 16 Mar 2010 13:17:30 -0000 Received: from unknown (HELO wind.localnet) (vladimir@127.0.0.2) by mail.codesourcery.com with ESMTPA; 16 Mar 2010 13:17:30 -0000 From: Vladimir Prus To: Pedro Alves Subject: Re: [MI tracepoints 6/9] trace variable commands Date: Tue, 16 Mar 2010 13:17:00 -0000 User-Agent: KMail/1.12.2 (Linux/2.6.31-20-generic-pae; KDE/4.3.2; i686; ; ) Cc: gdb-patches@sourceware.org References: <201003141158.36113.vladimir@codesourcery.com> <201003151800.38101.pedro@codesourcery.com> In-Reply-To: <201003151800.38101.pedro@codesourcery.com> MIME-Version: 1.0 Content-Type: Multipart/Mixed; boundary="Boundary-00=_oT4nL71WxocYXOK" Message-Id: <201003161617.28532.vladimir@codesourcery.com> 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: 2010-03/txt/msg00578.txt.bz2 --Boundary-00=_oT4nL71WxocYXOK Content-Type: Text/Plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Content-length: 1747 On Monday 15 March 2010 21:00:38 Pedro Alves wrote: > On Sunday 14 March 2010 08:58:36, Vladimir Prus wrote: > > + name = argv[0]; > > + if (name[0] != '$') > > + error (_("Name of trace variable should start with '$'")); > > + ++name; > > + > > + expr = parse_expression (argv[0]); > > + back_to = make_cleanup (xfree, expr); > > + > > + if (expr->nelts == 3 && expr->elts[0].opcode == OP_INTERNALVAR) > > + { > > + struct internalvar *intvar = expr->elts[1].internalvar; > > + if (intvar) > > + name = internalvar_name (intvar); > > + } > > + > > + if (!name || *name == '\0') > > + error (_("Invalid name of trace variable")); > > Waitaminute. Is there a merge error here? > > Repeating the snippet: > > > + name = argv[0]; > > + if (name[0] != '$') > > + error (_("Name of trace variable should start with '$'")); > > + ++name; > > + > > I think this whole bit above shouldn't be here. Yes, it's a merge error. > > + expr = parse_expression (argv[0]); > > + back_to = make_cleanup (xfree, expr); > > + > > + if (expr->nelts == 3 && expr->elts[0].opcode == OP_INTERNALVAR) > > + { > > + struct internalvar *intvar = expr->elts[1].internalvar; > > + if (intvar) > > + name = internalvar_name (intvar); <<<<<<<< (1) > > + } > > + > > + if (!name || *name == '\0') > > + error (_("Invalid name of trace variable")); > > Otherwise, it looks like you can get here with an invalid name, if > the expression did parse sucessfully, but (1) wasn't reached at all. At least after removing the unnecessary bit, if (!) is not executed, name will be NULL -- which seems right behaviour. Thanks, -- Vladimir Prus CodeSourcery vladimir@codesourcery.com (650) 331-3385 x722 --Boundary-00=_oT4nL71WxocYXOK Content-Type: text/x-patch; charset="UTF-8"; name="6-trace-variables-2.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="6-trace-variables-2.diff" Content-length: 7793 commit 1b7506f06e31d6ae934fc50e73eb1d8bde3ccd28 Author: vladimir Date: Thu Aug 6 07:50:47 2009 +0000 -trace-define-variable and -trace-list-variables. * tracepoint.c (create_trace_state_variable): Make private copy of name, as opposed to assuming the pointer lives forever. (tvariables_info_1): New. (tvariables_info): Use the above. * tracepoint.h (create_trace_state_variable, tvariables_info_1): Declare. * mi/mi-cmds.c (mi_cmds): Register -trace-define-variable and -trace-list-variables. * mi/mi-cmds.h (mi_cmd_trace_define_variable) (mi_cmd_trace_list_variables): New. * mi/mi-main.c (mi_cmd_trace_define_variable) (mi_cmd_trace_list_variables): New. diff --git a/gdb/mi/mi-cmds.c b/gdb/mi/mi-cmds.c index 6b260fc..a07ee3b 100644 --- a/gdb/mi/mi-cmds.c +++ b/gdb/mi/mi-cmds.c @@ -106,6 +106,8 @@ struct mi_cmd mi_cmds[] = { "thread-info", { NULL, 0 }, mi_cmd_thread_info }, { "thread-list-ids", { NULL, 0 }, mi_cmd_thread_list_ids}, { "thread-select", { NULL, 0 }, mi_cmd_thread_select}, + { "trace-define-variable", { NULL, 0 }, mi_cmd_trace_define_variable }, + { "trace-list-variables", { NULL, 0 }, mi_cmd_trace_list_variables }, { "trace-start", { NULL, 0 }, mi_cmd_trace_start }, { "trace-status", { NULL, 0 }, mi_cmd_trace_status }, { "trace-stop", { NULL, 0 }, mi_cmd_trace_stop }, diff --git a/gdb/mi/mi-cmds.h b/gdb/mi/mi-cmds.h index b5ff61f..dc2b2c6 100644 --- a/gdb/mi/mi-cmds.h +++ b/gdb/mi/mi-cmds.h @@ -89,6 +89,8 @@ extern mi_cmd_argv_ftype mi_cmd_target_file_delete; extern mi_cmd_argv_ftype mi_cmd_thread_info; extern mi_cmd_argv_ftype mi_cmd_thread_list_ids; extern mi_cmd_argv_ftype mi_cmd_thread_select; +extern mi_cmd_argv_ftype mi_cmd_trace_define_variable; +extern mi_cmd_argv_ftype mi_cmd_trace_list_variables; extern mi_cmd_argv_ftype mi_cmd_trace_start; extern mi_cmd_argv_ftype mi_cmd_trace_status; extern mi_cmd_argv_ftype mi_cmd_trace_stop; diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c index 5aebc50..1f1c014 100644 --- a/gdb/mi/mi-main.c +++ b/gdb/mi/mi-main.c @@ -2082,6 +2082,52 @@ print_diff (struct mi_timestamp *start, struct mi_timestamp *end) } void +mi_cmd_trace_define_variable (char *command, char **argv, int argc) +{ + struct expression *expr; + struct cleanup *back_to; + LONGEST initval = 0; + struct trace_state_variable *tsv; + char *name = 0; + + if (argc != 1 && argc != 2) + error (_("Usage: -trace-define-variable VARIABLE [VALUE]")); + + expr = parse_expression (argv[0]); + back_to = make_cleanup (xfree, expr); + + if (expr->nelts == 3 && expr->elts[0].opcode == OP_INTERNALVAR) + { + struct internalvar *intvar = expr->elts[1].internalvar; + if (intvar) + name = internalvar_name (intvar); + } + + if (!name || *name == '\0') + error (_("Invalid name of trace variable")); + + tsv = find_trace_state_variable (name); + if (!tsv) + tsv = create_trace_state_variable (name); + + if (argc == 2) + initval = value_as_long (parse_and_eval (argv[1])); + + tsv->initial_value = initval; + + do_cleanups (back_to); +} + +void +mi_cmd_trace_list_variables (char *command, char **argv, int argc) +{ + if (argc != 0) + error (_("-trace-list-variables: no arguments are allowed")); + + tvariables_info_1 (); +} + +void mi_cmd_trace_start (char *command, char **argv, int argc) { start_tracing (); diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c index b7fae78..8e55d82 100644 --- a/gdb/tracepoint.c +++ b/gdb/tracepoint.c @@ -271,7 +271,7 @@ create_trace_state_variable (const char *name) struct trace_state_variable tsv; memset (&tsv, 0, sizeof (tsv)); - tsv.name = name; + tsv.name = xstrdup (name); tsv.number = next_tsv_number++; return VEC_safe_push (tsv_s, tvariables, &tsv); } @@ -300,6 +300,7 @@ delete_trace_state_variable (const char *name) for (ix = 0; VEC_iterate (tsv_s, tvariables, ix, tsv); ++ix) if (strcmp (name, tsv->name) == 0) { + xfree ((void *)tsv->name); VEC_unordered_remove (tsv_s, tvariables, ix); return; } @@ -401,17 +402,15 @@ delete_trace_variable_command (char *args, int from_tty) dont_repeat (); } -/* List all the trace state variables. */ - -static void -tvariables_info (char *args, int from_tty) +void +tvariables_info_1 (void) { struct trace_state_variable *tsv; int ix; - char *reply; - ULONGEST tval; + int count = 0; + struct cleanup *back_to; - if (VEC_length (tsv_s, tvariables) == 0) + if (VEC_length (tsv_s, tvariables) == 0 && !ui_out_is_mi_like_p (uiout)) { printf_filtered (_("No trace state variables.\n")); return; @@ -422,24 +421,56 @@ tvariables_info (char *args, int from_tty) tsv->value_known = target_get_trace_state_variable_value (tsv->number, &(tsv->value)); - printf_filtered (_("Name\t\t Initial\tCurrent\n")); + back_to = make_cleanup_ui_out_table_begin_end (uiout, 3, + count, "trace-variables"); + ui_out_table_header (uiout, 15, ui_left, "name", "Name"); + ui_out_table_header (uiout, 11, ui_left, "initial", "Initial"); + ui_out_table_header (uiout, 11, ui_left, "current", "Current"); + + ui_out_table_body (uiout); for (ix = 0; VEC_iterate (tsv_s, tvariables, ix, tsv); ++ix) { - printf_filtered ("$%s", tsv->name); - print_spaces_filtered (17 - strlen (tsv->name), gdb_stdout); - printf_filtered ("%s ", plongest (tsv->initial_value)); - print_spaces_filtered (11 - strlen (plongest (tsv->initial_value)), gdb_stdout); + struct cleanup *back_to2; + char *c; + char *name; + + back_to2 = make_cleanup_ui_out_tuple_begin_end (uiout, "variable"); + + name = concat ("$", tsv->name, NULL); + make_cleanup (xfree, name); + ui_out_field_string (uiout, "name", name); + ui_out_field_string (uiout, "initial", plongest (tsv->initial_value)); + if (tsv->value_known) - printf_filtered (" %s", plongest (tsv->value)); + c = plongest (tsv->value); + else if (ui_out_is_mi_like_p (uiout)) + /* For MI, we prefer not to use magic string constants, but rather + omit the field completely. The difference between unknown and + undefined does not seem important enough to represent. */ + c = NULL; else if (current_trace_status ()->running || traceframe_number >= 0) /* The value is/was defined, but we don't have it. */ - printf_filtered (_(" ")); + c = ""; else /* It is not meaningful to ask about the value. */ - printf_filtered (_(" ")); - printf_filtered ("\n"); + c = ""; + if (c) + ui_out_field_string (uiout, "current", c); + ui_out_text (uiout, "\n"); + + do_cleanups (back_to2); } + + do_cleanups (back_to); +} + +/* List all the trace state variables. */ + +static void +tvariables_info (char *args, int from_tty) +{ + tvariables_info_1 (); } /* ACTIONS functions: */ diff --git a/gdb/tracepoint.h b/gdb/tracepoint.h index 2e9193c..d03f09a 100644 --- a/gdb/tracepoint.h +++ b/gdb/tracepoint.h @@ -145,6 +145,7 @@ extern void end_actions_pseudocommand (char *args, int from_tty); extern void while_stepping_pseudocommand (char *args, int from_tty); extern struct trace_state_variable *find_trace_state_variable (const char *name); +extern struct trace_state_variable *create_trace_state_variable (const char *name); extern void parse_trace_status (char *line, struct trace_status *ts); @@ -162,4 +163,6 @@ extern void stop_tracing (void); extern void trace_status_mi (int on_stop); +extern void tvariables_info_1 (void); + #endif /* TRACEPOINT_H */ --Boundary-00=_oT4nL71WxocYXOK--