Re: [PATCH] xen/arm: Correctly handle non-page aligned pointer in raw_copy_*

[Julien Grall <julien.grall@linaro.org>]

On 02/18/2014 03:25 PM, Ian Campbell wrote:
> On Tue, 2014-02-18 at 15:08 +0000, Julien Grall wrote:
>> On 02/18/2014 03:00 PM, Ian Campbell wrote:
>>> On Tue, 2014-02-18 at 14:59 +0000, Ian Campbell wrote:
>>>
>>>>> As the offset is only computed one time per function,
>>>>
>>>> We set offset = 0 at the end of the first iteration.
>>>
>>> Ah, we do in raw_copy_to_guest_helper and raw_clear_guest but not
>>> raw_copy_from_guest -- which I think is the actual bug here.
>>
>> I didn't notice the offset = 0 at then end of raw_copy_to_guest.
>>
>> I can send a patch to only set offset to 0 in raw_copy_from_guest. But I
>> think it's less clear than this patch. What do you think?
> 
> I think the approach currently used by the (non-buggy) functions is
> better -- it makes it obvious that after the first iteration things
> *have* to now be aligned.

Ok. I will resend the patch.

> I also wouldn't be surprised if the compiler had trouble proving this
> and so ended up needlessly recalculating offset instead of optimising it
> out.
> 
> If you find the code unclear please feel free to add comments etc.

I will add comment.

-- 
Julien Grall