Re: [PATCH v4 04/15] xenctx: Add command line options -b and -l

[George Dunlap <george.dunlap@eu.citrix.com>]

On 03/18/2014 10:15 PM, Don Slutz wrote:
>    -b <bytes>, --bytes-per-line <bytes>
>                      change the number of bytes per line output for Stack.
>                      note: rounded to native size (4 or 8 bytes).
>    -l <lines>, --lines <lines>
>                      change the number of lines output for Stack.
>
> Signed-off-by: Don Slutz <dslutz@verizon.com>
> ---
>   tools/xentrace/xenctx.c | 67 ++++++++++++++++++++++++++++++++++++++++---------
>   1 file changed, 55 insertions(+), 12 deletions(-)
>
> diff --git a/tools/xentrace/xenctx.c b/tools/xentrace/xenctx.c
> index 6da38cc..62a8519 100644
> --- a/tools/xentrace/xenctx.c
> +++ b/tools/xentrace/xenctx.c
> @@ -36,6 +36,8 @@ static struct xenctx {
>       int stack_trace;
>       int disp_all;
>       int multiple_pages;
> +    int bytes_per_line;
> +    int lines;
>       int all_vcpus;
>       int self_paused;
>       xc_dominfo_t dominfo;
> @@ -62,6 +64,8 @@ typedef uint64_t guest_word_t;
>   #define FMT_64B_WORD "%016lx"
>   #endif
>   
> +#define MAX_BYTES_PER_LINE 128
> +
>   struct symbol {
>       guest_word_t address;
>       char *name;
> @@ -663,25 +667,33 @@ static int print_stack(vcpu_guest_context_any_t *ctx, int vcpu, int width)
>       guest_word_t *p;
>       int i;
>   
> +    if ( width )
> +        xenctx.bytes_per_line = ((xenctx.bytes_per_line + width - 1) / width) * width;
>       stack_limit = ((stack_pointer(ctx) + XC_PAGE_SIZE)
>                      & ~((guest_word_t) XC_PAGE_SIZE - 1));
>       if ( xenctx.multiple_pages > 1 )
>           stack_limit += (xenctx.multiple_pages - 1) * XC_PAGE_SIZE;
>       printf("\n");
> -    printf("Stack:\n");
> -    for (i=1; i<5 && stack < stack_limit; i++) {
> -        while(stack < stack_limit && stack < stack_pointer(ctx) + i*32) {
> -            p = map_page(ctx, vcpu, stack);
> -            if (!p)
> -                return -1;
> -            word = read_stack_word(p, width);
> -            printf(" ");
> -            print_stack_word(word, width);
> -            stack += width;
> +    if ( xenctx.lines )
> +    {
> +        printf("Stack:\n");
> +        for (i = 1; i < xenctx.lines + 1 && stack < stack_limit; i++)
> +        {
> +            while ( stack < stack_limit &&
> +                    stack < stack_pointer(ctx) + i * xenctx.bytes_per_line )
> +            {
> +                p = map_page(ctx, vcpu, stack);
> +                if ( !p )
> +                    return -1;
> +                word = read_stack_word(p, width);
> +                printf(" ");
> +                print_stack_word(word, width);
> +                stack += width;
> +            }
> +            printf("\n");
>           }
>           printf("\n");
>       }
> -    printf("\n");
>   
>       if(xenctx.stack_trace)
>           printf("Stack Trace:\n");
> @@ -839,19 +851,26 @@ static void usage(void)
>       printf("  -C, --all-vcpus    print info for all vcpus\n");
>       printf("  -m PAGES, --multiple-pages=PAGES\n");
>       printf("                     assume the kernel was compiled with PAGES (default 1) of stacks.\n");
> +    printf("  -b <bytes>, --bytes-per-line <bytes>\n");
> +    printf("                     change the number of bytes per line output for Stack.\n");
> +    printf("                     note: rounded to native size (4 or 8 bytes).\n");
> +    printf("  -l <lines>, --lines <lines>\n");
> +    printf("                     change the number of lines output for Stack.\n");

You should probably put the default values here (32 and 5 
respectively).  (And they should probably be set by a #define, so that 
there's no risk of the defaults in the help diverging from the defaults 
in the code.)

Also, would it make sense to have a value for -l that means "just print 
everything until you hit "stack_limit"?  Obviously someone could just 
write "-l 100000" here, but it might be nicer to have something more 
sensible.

That's just an idea; I think once the default values are in it can have 
my ack.

  -George