This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Trace state variables
- From: Tom Tromey <tromey at redhat dot com>
- To: Stan Shebs <stan at codesourcery dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Tue, 15 Dec 2009 12:17:15 -0700
- Subject: Re: [PATCH] Trace state variables
- References: <4B279456.1010508@codesourcery.com>
- Reply-to: tromey at redhat dot com
>>>>> "Stan" == Stan Shebs <stan@codesourcery.com> writes:
Stan> Since internalvar handling has changed recently, I've left off the
Stan> value.c bit to make "print $foo" work.
Will this be forthcoming?
Stan> + case BINOP_ASSIGN:
It seems a little odd to add BINOP_ASSIGN but not BINOP_ASSIGN_MODIFY.
Stan> extern int remote_supports_cond_tracepoints (void);
Stan> + extern char *unpack_varlen_hex (char *buff, ULONGEST *result);
It seems like this could be in a header somewhere.
Stan> + /* The list of all trace state variables. We don't retain pointers to
Stan> + any of these for any reason - API is by name or number only - so it
Stan> + works to have a vector of objects. */
Stan> +
Stan> + VEC(tsv_s) *tvariables;
Stan> +
Stan> + /* The next integer to assign to a variable. */
Stan> +
Stan> + int next_tsv_number = 1;
It seems like these, plus some of the new functions, could be static.
It is hard to say for sure since I don't know what future patches might do.
Stan> + void
Stan> + trace_variable_command (char *args, int from_tty)
Stan> + {
[...]
Stan> + /* All the possible valid arguments are expressions. */
Stan> + expr = parse_expression (args);
Stan> + old_chain = make_cleanup (free_current_contents, &expr);
I think set_cmd_completer (..., expression_completer) is probably in
order, in the file's initialize function.
Stan> + void
Stan> + delete_trace_variable_command (char *args, int from_tty)
It would be nice to have a completer function for this.
Stan> + static void
Stan> + tvariables_info (char *args, int from_tty)
[...]
Stan> + printf_filtered (_("Name\t\t Initial\tCurrent\n"));
I think that brand-new formatted output should be done using the ui_out
machinery. Is there some drawback to doing that? I really don't
know... I would have expected ui_out to be used universally but instead
it seems somewhat random, and I don't know why. (I assume that whoever
was doing this transition ran out of steam... ?)
Stan> + typedef struct trace_state_variable tsv_s;
Stan> + DEF_VEC_O(tsv_s);
AFAICT, this VEC is only used in a single .c file. It is better to put
the DEF_VEC there, to avoid duplicating the static functions in other
compilation units where they aren't needed.
Tom