Re: [Xen-devel] [PATCH v5 14/28] x86, xsplice: Print payload's symbol name and payload name in backtraces

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 24.03.16 at 21:00, <konrad.wilk@xxxxxxxxxx> wrote:
> @@ -331,16 +332,17 @@ static char *pointer(char *str, char *end, const char 
> **fmt_ptr,
>      {
>          unsigned long sym_size, sym_offset;
>          char namebuf[KSYM_NAME_LEN+1];
> +        bool_t payload = 0;
>  
>          /* Advance parents fmt string, as we have consumed 's' or 'S' */
>          ++*fmt_ptr;
>  
>          s = symbols_lookup((unsigned long)arg, &sym_size, &sym_offset, 
> namebuf);
> -
> -        /* If the symbol is not found, fall back to printing the address */
> +        /* If the symbol is not found, fall back to printing the address. */
>          if ( !s )
>              break;
> -

Please don't drop blank lines like this.

> +        if ( strncmp(namebuf, s, KSYM_NAME_LEN) )
> +            payload = 1;

What is this about? A comment is absolutely needed here, the
more that without context "payload" is also an unclear term.
And then - would simply comparing the two pointers suffice?

> +static const char *xsplice_symbols_lookup(unsigned long addr,
> +                                          unsigned long *symbolsize,
> +                                          unsigned long *offset,
> +                                          char *namebuf)
> +{
> +    struct payload *data;
> +    unsigned int i;
> +    int best;
> +
> +    /*
> +     * No locking since this list is only ever changed during apply or revert
> +     * context.
> +     */
> +    list_for_each_entry ( data, &applied_list, applied_list )
> +    {
> +        if ( !((void *)addr >= data->text_addr &&
> +               (void *)addr < (data->text_addr + data->text_size)) )
> +            continue;

It may be just me, but I find such !() constructs harder to understand
than the equivalent without that negation.

> +        best = -1;
> +
> +        for ( i = 0; i < data->nsyms; i++ )
> +        {
> +            if ( data->symtab[i].value <= addr &&
> +                 ( best == -1 ||

Stray blank.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel