Re: [PATCH v4 7/9] migration: Simplify alignment and alignment checks

[David Hildenbrand <david@redhat.com>]

On 03.09.21 10:47, David Hildenbrand wrote:
> On 03.09.21 00:32, Peter Xu wrote:
>> On Thu, Sep 02, 2021 at 03:14:30PM +0200, David Hildenbrand wrote:
>>> diff --git a/migration/migration.c b/migration/migration.c
>>> index bb909781b7..ae97c2c461 100644
>>> --- a/migration/migration.c
>>> +++ b/migration/migration.c
>>> @@ -391,7 +391,7 @@ int migrate_send_rp_message_req_pages(MigrationIncomingState *mis,
>>>    int migrate_send_rp_req_pages(MigrationIncomingState *mis,
>>>                                  RAMBlock *rb, ram_addr_t start, uint64_t haddr)
>>>    {
>>> -    void *aligned = (void *)(uintptr_t)(haddr & (-qemu_ram_pagesize(rb)));
>>> +    void *aligned = (void *)QEMU_ALIGN_DOWN(haddr, qemu_ram_pagesize(rb));
>>
>> Is uintptr_t still needed?  I thought it would generate a warning otherwise but
>> not sure.
> 
> It doesn't in my setup, but maybe it will on 32bit archs ...
> 
> I discussed this with Phil in
> 
> https://lkml.kernel.org/r/2c8d80ad-f171-7d5f-3235-92f02fa174b3@redhat.com
> 
> Maybe
> 
> QEMU_ALIGN_PTR_DOWN((void *)haddr, qemu_ram_pagesize(rb)));
> 
> Is really what we want.

... but it would suffer the same issue I think. I just ran it trough the 
gitlab pipeline, including "i386-fedora-cross-compile" ... and it seems 
to compile just fine, which is weird, because I'd also expect

"warning: cast to pointer from integer of different size 
[-Wint-to-pointer-cast]"

We most certainly need the "(void *)(uintptr_t)" to convert from u64 to 
a pointer.

Let's just do it cleanly:

void *unaligned = (void *)(uintptr_t)haddr;
void *aligned = QEMU_ALIGN_PTR_DOWN(unaligned, qemu_ram_pagesize(rb));

Thoughts?

-- 
Thanks,

David / dhildenb