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

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

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