Re: [Xen-devel] [PATCH v4 6/9] livepatch: Add parsing for the symbol+0x<offset>

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 24.08.16 at 04:22, <konrad.wilk@xxxxxxxxxx> wrote:
> --- a/xen/common/livepatch.c
> +++ b/xen/common/livepatch.c
> @@ -237,13 +237,34 @@ static const char *livepatch_symbols_lookup(unsigned 
> long addr,
>  static int resolve_old_address(struct livepatch_func *f,
>                                 const struct livepatch_elf *elf)
>  {
> +    const char *s;
> +    char *plus = NULL;

Pointless initializer.

> +    unsigned long offset = 0;
> +
>      if ( f->old_addr )
>          return 0;
>  
> -    f->old_addr = (void *)symbols_lookup_by_name(f->name);
> +    s = f->name;

Otoh this could become s'es initializer.

> +    /* +<offset> */
> +    plus = strchr(f->name, '+');

And I think you should prefer using the local variable here.

Furthermore you're losing const here - does f->name really point
to memory that doesn't get mapped r/o?

> +    if ( plus )
> +    {
> +        const char *endp = NULL;

Pointless initializer again (or else ...

> +        offset = simple_strtoul(plus + 1, &endp, 16);
> +
> +        if ( *endp != '\0' )

... the deref here couldn't be unconditional).

> +            return -EINVAL;
> +
> +        /* So that symbol lookup works. */
> +        *plus = '\0';
> +        s = f->name;

Why? f->name didn't change afaict.

Overall - are you sure you want to disallow symbol names containing
+ characters? I.e. you don't want to add support for some form of
quoting?

Jan


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