Re: [Xen-devel] [BUGFIX][PATCH 3/4] hvm_save_one: return correct data.

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 12.12.13 at 01:56, Don Slutz <dslutz@xxxxxxxxxxx> wrote:
> From: Don Slutz <dslutz@xxxxxxxxxxx>
> 
> It is possible that hvm_sr_handlers[typecode].save does not use all
> the provided room.  In that case, using:
> 
>    instance * hvm_sr_handlers[typecode].size
> 
> does not select the correct instance.  Add code to search for the
> correct instance.
> 
> Signed-off-by: Don Slutz <dslutz@xxxxxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

But this needs to be cleaned up coding style wise and ...

> --- a/xen/common/hvm/save.c
> +++ b/xen/common/hvm/save.c
> @@ -112,13 +112,27 @@ int hvm_save_one(struct domain *d, uint16_t typecode, 
> uint16_t instance,
>                 d->domain_id, typecode);
>          rv = -EFAULT;
>      }
> -    else if ( copy_to_guest(handle,
> -                            ctxt.data 
> -                            + (instance * hvm_sr_handlers[typecode].size) 
> -                            + sizeof (struct hvm_save_descriptor), 
> -                            hvm_sr_handlers[typecode].size
> -                            - sizeof (struct hvm_save_descriptor)) )
> -        rv = -EFAULT;
> +    else
> +    {
> +        uint32_t off;
> +
> +        rv = -EBADSLT;
> +        for (off = 0; off < ctxt.cur; off += hvm_sr_handlers[typecode].size) 
> {
> +            struct hvm_save_descriptor *desc
> +                   = (struct hvm_save_descriptor *)&ctxt.data[off];

.. this could be const, and the cast could simply be (void *), ...

> +            if (instance == desc->instance) {
> +                rv = 0;
> +                if ( copy_to_guest(handle,
> +                                   ctxt.data
> +                                   + off

... this doesn't need to be on a separate line, and ...

> +                                   + sizeof (struct hvm_save_descriptor),
> +                                   hvm_sr_handlers[typecode].size
> +                                   - sizeof (struct hvm_save_descriptor)) )

... both these sizeof()s would now better be sizeof(*desc).

Jan


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