On 8/11/19 3:12 PM, Frediano Ziglio 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.htmOn 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?
C/C++ documentation classify these types as integral, not integer.also s/a/an
fixed, thanks
"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?
It sounds reasonable, although the value came from "addr_delta" field of QXLDevMemSlotwhich is part of SPICE ABI (so cannot be changed).
Actually I thought to change address_delta to match everywhere to uint64_t of addr_delta
can this delta be actually negative? seems not since it's coming from addr_delta,wouldn't be
better to keep it unsignedSnir.
The value is used only for addition and we support only two's complement architectures
so no difference between signed or unsigned in this case (only additions means no extension,
division or others).
Moving to uin64_t instead of intptr_t or uintptr_t would cause 64 bit arithmetic even on
32 bit architectures for no reasons.
Would be uintptr_t (all unsigned but 32 bit) fine?
Snir.
Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx> --- 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; };Frediano
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel