[Spice-devel] [PATCH spice-server] Use (u)intptr_t for virtual addresses

Snir Sheriber ssheribe at redhat.com
Sun Aug 11 12:00:47 UTC 2019


On 8/11/19 2:56 PM, Snir Sheriber wrote:
>
> Hi
>
> On 7/23/19 11:22 AM, Frediano Ziglio wrote:
>> 
>>
>>
>>       When to Say "a" or "an" | Pronunciation | EnglishClub
>>
>> https://www.englishclub.com/pronunciation/a-an.htm


Oops, wrong pasting, please ignore :P


>> On LLP64 platforms (like Windows) a virtual address cannot
>> be represented by a "unsigned long" type, so use uintptr_t
>> which is defined as a integral type large like a pointer.
>
>
> This sentence sounds a bit odd to me
>
> should be integer?
>
> also s/a/an
>
>
>> "address_delta" is a difference of pointers so use same
>> type size.
>
>
> Not a big deal but wouldn't be preferable to be consistent with 
> addr_delta?
>
> Snir.
>
>> Signed-off-by: Frediano Ziglio<fziglio at redhat.com>
>> ---
>>   server/memslot.c       | 22 ++++++++++++----------
>>   server/memslot.h       | 20 ++++++++++----------
>>   server/red-parse-qxl.c |  2 +-
>>   server/spice-qxl.h     |  4 ++--
>>   4 files changed, 25 insertions(+), 23 deletions(-)
>>
>> diff --git a/server/memslot.c b/server/memslot.c
>> index 2a1771b02..182d2b7e9 100644
>> --- a/server/memslot.c
>> +++ b/server/memslot.c
>> @@ -21,7 +21,7 @@
>>   
>>   #include "memslot.h"
>>   
>> -static unsigned long __get_clean_virt(RedMemSlotInfo *info, QXLPHYSICAL addr)
>> +static uintptr_t __get_clean_virt(RedMemSlotInfo *info, QXLPHYSICAL addr)
>>   {
>>       return addr & info->memslot_clean_virt_mask;
>>   }
>> @@ -37,7 +37,8 @@ static void print_memslots(RedMemSlotInfo *info)
>>                   !info->mem_slots[i][x].virt_end_addr) {
>>                   continue;
>>               }
>> -            printf("id %d, group %d, virt start %lx, virt end %lx, generation %u, delta %lx\n",
>> +            printf("id %d, group %d, virt start %" PRIxPTR ", virt end %" PRIxPTR ", generation %u,"
>> +                   " delta %" PRIxPTR "\n",
>>                      x, i, info->mem_slots[i][x].virt_start_addr,
>>                      info->mem_slots[i][x].virt_end_addr, info->mem_slots[i][x].generation,
>>                      info->mem_slots[i][x].address_delta);
>> @@ -46,7 +47,7 @@ static void print_memslots(RedMemSlotInfo *info)
>>   }
>>   
>>   /* return 1 if validation successfull, 0 otherwise */
>> -int memslot_validate_virt(RedMemSlotInfo *info, unsigned long virt, int slot_id,
>> +int memslot_validate_virt(RedMemSlotInfo *info, uintptr_t virt, int slot_id,
>>                             uint32_t add_size, uint32_t group_id)
>>   {
>>       MemSlot *slot;
>> @@ -60,8 +61,9 @@ int memslot_validate_virt(RedMemSlotInfo *info, unsigned long virt, int slot_id,
>>       if (virt < slot->virt_start_addr || (virt + add_size) > slot->virt_end_addr) {
>>           print_memslots(info);
>>           spice_warning("virtual address out of range"
>> -              "    virt=0x%lx+0x%x slot_id=%d group_id=%d\n"
>> -              "    slot=0x%lx-0x%lx delta=0x%lx",
>> +              "    virt=0x%" G_GINTPTR_MODIFIER "x+0x%x slot_id=%d group_id=%d\n"
>> +              "    slot=0x%" G_GINTPTR_MODIFIER "x-0x%" G_GINTPTR_MODIFIER "x"
>> +              " delta=0x%" G_GINTPTR_MODIFIER "x",
>>                 virt, add_size, slot_id, group_id,
>>                 slot->virt_start_addr, slot->virt_end_addr, slot->address_delta);
>>           return 0;
>> @@ -69,9 +71,9 @@ int memslot_validate_virt(RedMemSlotInfo *info, unsigned long virt, int slot_id,
>>       return 1;
>>   }
>>   
>> -unsigned long memslot_max_size_virt(RedMemSlotInfo *info,
>> -                                    unsigned long virt, int slot_id,
>> -                                    uint32_t group_id)
>> +uintptr_t memslot_max_size_virt(RedMemSlotInfo *info,
>> +                                uintptr_t virt, int slot_id,
>> +                                uint32_t group_id)
>>   {
>>       MemSlot *slot;
>>   
>> @@ -91,7 +93,7 @@ void *memslot_get_virt(RedMemSlotInfo *info, QXLPHYSICAL addr, uint32_t add_size
>>   {
>>       int slot_id;
>>       int generation;
>> -    unsigned long h_virt;
>> +    uintptr_t h_virt;
>>   
>>       MemSlot *slot;
>>   
>> @@ -171,7 +173,7 @@ void memslot_info_destroy(RedMemSlotInfo *info)
>>   }
>>   
>>   void memslot_info_add_slot(RedMemSlotInfo *info, uint32_t slot_group_id, uint32_t slot_id,
>> -                           uint64_t addr_delta, unsigned long virt_start, unsigned long virt_end,
>> +                           uint64_t addr_delta, uintptr_t virt_start, uintptr_t virt_end,
>>                              uint32_t generation)
>>   {
>>       spice_assert(info->num_memslots_groups > slot_group_id);
>> diff --git a/server/memslot.h b/server/memslot.h
>> index 00728c4b6..45381feb9 100644
>> --- a/server/memslot.h
>> +++ b/server/memslot.h
>> @@ -25,9 +25,9 @@
>>   
>>   typedef struct MemSlot {
>>       int generation;
>> -    unsigned long virt_start_addr;
>> -    unsigned long virt_end_addr;
>> -    long address_delta;
>> +    uintptr_t virt_start_addr;
>> +    uintptr_t virt_end_addr;
>> +    intptr_t address_delta;
>>   } MemSlot;
>>   
>>   typedef struct RedMemSlotInfo {
>> @@ -39,8 +39,8 @@ typedef struct RedMemSlotInfo {
>>       uint8_t memslot_id_shift;
>>       uint8_t memslot_gen_shift;
>>       uint8_t internal_groupslot_id;
>> -    unsigned long memslot_gen_mask;
>> -    unsigned long memslot_clean_virt_mask;
>> +    uintptr_t memslot_gen_mask;
>> +    uintptr_t memslot_clean_virt_mask;
>>   } RedMemSlotInfo;
>>   
>>   static inline int memslot_get_id(RedMemSlotInfo *info, uint64_t addr)
>> @@ -53,11 +53,11 @@ static inline int memslot_get_generation(RedMemSlotInfo *info, uint64_t addr)
>>       return (addr >> info->memslot_gen_shift) & info->memslot_gen_mask;
>>   }
>>   
>> -int memslot_validate_virt(RedMemSlotInfo *info, unsigned long virt, int slot_id,
>> +int memslot_validate_virt(RedMemSlotInfo *info, uintptr_t virt, int slot_id,
>>                             uint32_t add_size, uint32_t group_id);
>> -unsigned long memslot_max_size_virt(RedMemSlotInfo *info,
>> -                                    unsigned long virt, int slot_id,
>> -                                    uint32_t group_id);
>> +uintptr_t memslot_max_size_virt(RedMemSlotInfo *info,
>> +                                uintptr_t virt, int slot_id,
>> +                                uint32_t group_id);
>>   void *memslot_get_virt(RedMemSlotInfo *info, QXLPHYSICAL addr, uint32_t add_size,
>>                          int group_id);
>>   
>> @@ -68,7 +68,7 @@ void memslot_info_init(RedMemSlotInfo *info,
>>                          uint8_t internal_groupslot_id);
>>   void memslot_info_destroy(RedMemSlotInfo *info);
>>   void memslot_info_add_slot(RedMemSlotInfo *info, uint32_t slot_group_id, uint32_t slot_id,
>> -                           uint64_t addr_delta, unsigned long virt_start, unsigned long virt_end,
>> +                           uint64_t addr_delta, uintptr_t virt_start, uintptr_t virt_end,
>>                              uint32_t generation);
>>   void memslot_info_del_slot(RedMemSlotInfo *info, uint32_t slot_group_id, uint32_t slot_id);
>>   void memslot_info_reset(RedMemSlotInfo *info);
>> diff --git a/server/red-parse-qxl.c b/server/red-parse-qxl.c
>> index eb2c0b538..01fd60580 100644
>> --- a/server/red-parse-qxl.c
>> +++ b/server/red-parse-qxl.c
>> @@ -1335,7 +1335,7 @@ static bool red_get_message(QXLInstance *qxl_instance, RedMemSlotInfo *slots, in
>>   {
>>       QXLMessage *qxl;
>>       int memslot_id;
>> -    unsigned long len;
>> +    uintptr_t len;
>>       uint8_t *end;
>>   
>>       /*
>> diff --git a/server/spice-qxl.h b/server/spice-qxl.h
>> index 2f47910b9..5349d9275 100644
>> --- a/server/spice-qxl.h
>> +++ b/server/spice-qxl.h
>> @@ -187,8 +187,8 @@ struct QXLDevMemSlot {
>>       uint32_t slot_group_id;
>>       uint32_t slot_id;
>>       uint32_t generation;
>> -    unsigned long virt_start;
>> -    unsigned long virt_end;
>> +    uintptr_t virt_start;
>> +    uintptr_t virt_end;
>>       uint64_t addr_delta;
>>       uint32_t qxl_ram_size;
>>   };
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20190811/88dd7067/attachment.html>


More information about the Spice-devel mailing list