Re: [patch v6 02/12] cli, btrace: add btrace cli

Jan Kratochvil
2012-12-17T18:32:28Z

On Mon, 17 Dec 2012 17:01:31 +0100, markus.t.metzger@intel.com wrote:
[...]
> --- a/gdb/btrace.c
> +++ b/gdb/btrace.c
[...]
> @@ -166,3 +188,581 @@ next_btrace (struct thread_info *tp)
>
> return VEC_index (btrace_block_s, btp->btrace, btp->iterator);
> }
> +
> +/* Enable branch tracing. Turn errors into warnings. */
> +
> +static void
> +warn_enable_btrace (struct thread_info *tp)
> +{
> + volatile struct gdb_exception error;
> +
> + TRY_CATCH (error, RETURN_MASK_ERROR)
> + enable_btrace (tp);
> +
> + if (error.message != NULL)
> + warning (_("%s"), error.message);

"%s" does not need localization.

> +}
> +
> +/* Disable branch tracing. Turn errors into warnings. */
> +
> +static void
> +warn_disable_btrace (struct thread_info *tp)
> +{
> + volatile struct gdb_exception error;
> +
> + TRY_CATCH (error, RETURN_MASK_ERROR)
> + disable_btrace (tp);
> +
> + if (error.message != NULL)
> + warning (_("%s"), error.message);

"%s" does not need localization.

> +}
[...]
> +static void
> +cmd_btrace_disable (char *args, int from_tty)
> +{
> + struct thread_info *tp;
> +
> + if (args != NULL && *args != 0)
> + {
> + ALL_THREADS (tp)
> + if (number_is_in_list (args, tp->num))
> + warn_enable_btrace (tp);

Here should be warn_disable_btrace.

> + }
> + else
> + {
> + tp = find_thread_ptid (inferior_ptid);
> + if (tp == NULL)
> + error (_("Couldn't disable branch tracing: no inferior thread."));
> +
> + disable_btrace (tp);
> + }
> +}
[...]
> +static void
> +do_btrace_list_function (struct btrace_block *trace)
> +{
> + struct minimal_symbol *msymbol;
> + struct symbol *symbol;
> + const char* func;

GDB is using 'const char *' style.

> +
> + func = "??";
> + symbol = find_pc_function (trace->begin);
> + if (symbol != NULL)
> + func = SYMBOL_PRINT_NAME (symbol);
[...]
> +static void
> +do_btrace_list_item (struct btrace_block *trace, enum btrace_list_flags flags)
> +{
> + if (flags & BTR_LIST_ADDRESS)

if ((flags & BTR_LIST_ADDRESS) != 0)

> + do_btrace_list_address (trace);
> +
> + if (flags & BTR_LIST_FUNCTION)

dtto

> + do_btrace_list_function (trace);
> +
> + if (flags & BTR_LIST_LINE)

dtto

> + do_btrace_list_line (trace);
> +}
[...]
> +/* The "btrace list" command. */
> +
> +static void
> +cmd_btrace_list (char *args, int from_tty)
[...]
> + if (flags & BTR_LIST_TOTAL)

if ((flags & BTR_LIST_TOTAL) != 0)

> + {
[...]
> +/* Print the disassembly of a btrace block. */
> +
> +static void
> +do_btrace (struct btrace_block *trace, int flags)
> +{
> + struct gdbarch *gdbarch = target_gdbarch ();
> +
> + if (trace == NULL)
> + error (_("No trace."));
> +
> + if (trace->end < trace->begin)
> + warning (_("Bad trace: %s - %s"), paddress (gdbarch, trace->begin),
> + paddress (gdbarch, trace->end));

Shouldn't this be rather gdb_assert? How to create such case?

> +
> + gdb_disassembly (gdbarch, current_uiout, 0, flags, -1,
> + trace->begin, trace->end + 1);
> +}
[...]

No need for a repost wrt the changes above.

Thanks,
Jan