Re: [PATCH v5 09/17] xenctx: Add command line option -T (--tag-trace)

[Ian Campbell <Ian.Campbell@citrix.com>]

On Thu, 2014-03-20 at 15:06 -0400, Don Slutz wrote:
> If specified, add stack addr to Call and Stack trace.

This could safely be unconditional I think.

Or if not then it should share an option with --tag-stack-dump, e.g.
-a,--print-addresses

> @@ -755,9 +758,15 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int vcpu, int width)
>                      p = map_page(ctx, vcpu, stack);
>                      if (!p)
>                          return -1;
> -                    printf("|   ");
> +                    if ( xenctx.tag_call_trace )
> +                    {
> +                        print_stack_word(stack, width);
> +                        printf(": |   ");
> +                    } else {
> +                        printf("|   ");
> +                    }

if (trace)
	print stack work
	print ": "
}
print ("|    ")

(the examples you gave have no vertical | in them -- are they out of
date?)

> @@ -769,7 +778,13 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int vcpu, int width)
>                  return -1;
>              frame = read_stack_word(p, width);
>              if (xenctx.stack_trace) {
> -                printf("|-- ");
> +                if ( xenctx.tag_call_trace )
> +                {
> +                    print_stack_word(stack, width);
> +                    printf(": |-- ");
> +                } else {
> +                    printf("|-- ");
> +                }

Same comment as above.

>                  print_stack_word(read_stack_word(p, width), width);
>                  printf("\n");
>              }
> @@ -780,7 +795,13 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int vcpu, int width)
>                  if (!p)
>                      return -1;
>                  word = read_stack_word(p, width);
> -                printf("%c [<", xenctx.stack_trace ? '|' : ' ');
> +                if ( xenctx.tag_call_trace )
> +                {
> +                    print_stack_word(stack, width);
> +                    printf(": %c [<", xenctx.stack_trace ? '|' : ' ');
> +                } else {
> +                    printf("%c [<", xenctx.stack_trace ? '|' : ' ');
> +                }

and again (here the code duplication is especially intolerable)

>                  print_stack_word(word, width);
>                  printf(">]");
>                  print_symbol(word);
> @@ -796,13 +817,25 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int vcpu, int width)
>                  return -1;
>              word = read_stack_word(p, width);
>              if (is_kernel_text(word)) {
> -                printf("  [<");
> +                if ( xenctx.tag_call_trace )
> +                {
> +                    print_stack_word(stack, width);
> +                    printf(":  [<");
> +                } else {
> +                    printf("  [<");
> +                }

again.

>                  print_stack_word(word, width);
>                  printf(">]");
>                  print_symbol(word);
>                  printf("\n");
>              } else if (xenctx.stack_trace) {
> -                printf("    ");
> +                if ( xenctx.tag_call_trace )
> +                {
> +                    print_stack_word(stack, width);
> +                    printf(":    ");
> +                } else {
> +                    printf("    ");
> +                }

again.

It almost like a print_stack_trace helper is in order.