<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<p><br>
</p>
<div class="moz-cite-prefix">On 8/11/19 3:12 PM, Frediano Ziglio
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:1029345715.5453659.1565525540653.JavaMail.zimbra@redhat.com">
<meta http-equiv="content-type" content="text/html; charset=UTF-8">
<div style="font-family: times new roman, new york, times, serif;
font-size: 12pt; color: #000000">
<div><br>
</div>
<blockquote style="border-left:2px solid
#1010FF;margin-left:5px;padding-left:5px;color:#000;font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;">
<div><br>
</div>
<p>Hi<br>
</p>
<div class="moz-cite-prefix">On 7/23/19 11:22 AM, Frediano
Ziglio wrote:<br>
</div>
<blockquote
cite="mid:20190723082230.10381-3-fziglio@redhat.com">
<pre class="moz-quote-pre"><a href="https://www.englishclub.com/pronunciation/a-an.htm" target="_blank" moz-do-not-send="true"><h3 class="LC20lb"><span dir="ltr">When to Say "a" or "an" | Pronunciation | EnglishClub</span></h3>
<div class="TbwUpd"><cite class="iUh30"><span dir="ltr">https://www.englishclub.com/pronunciation/a-an.htm</span></cite></div></a>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.</pre>
</blockquote>
<p><br>
</p>
<p>This sentence sounds a bit odd to me<br>
</p>
<p>should be integer?</p>
</blockquote>
<div>C/C++ documentation classify these types as integral, not
integer.<br>
</div>
<blockquote style="border-left:2px solid
#1010FF;margin-left:5px;padding-left:5px;color:#000;font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;">
<p>also s/a/an</p>
</blockquote>
<div>fixed, thanks<br>
</div>
<blockquote style="border-left:2px solid
#1010FF;margin-left:5px;padding-left:5px;color:#000;font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;">
<p><br>
</p>
<p><br>
</p>
<blockquote
cite="mid:20190723082230.10381-3-fziglio@redhat.com">
<pre class="moz-quote-pre">"address_delta" is a difference of pointers so use same
type size.
</pre>
</blockquote>
<p><br>
</p>
<p>Not a big deal but wouldn't be preferable to be consistent
with addr_delta?</p>
</blockquote>
<div>It sounds reasonable, although the value came from
"addr_delta" field of QXLDevMemSlot<br>
</div>
<div>which is part of SPICE ABI (so cannot be changed).<br>
</div>
</div>
</blockquote>
<p><br>
</p>
<p>Actually I thought to change address_delta to match everywhere to
uint64_t of addr_delta<br>
can this delta be actually negative? seems not since it's coming
from addr_delta,wouldn't be<br>
better to keep it unsigned<br>
</p>
<p>Snir.<br>
</p>
<p><br>
</p>
<blockquote type="cite"
cite="mid:1029345715.5453659.1565525540653.JavaMail.zimbra@redhat.com">
<div style="font-family: times new roman, new york, times, serif;
font-size: 12pt; color: #000000">
<blockquote style="border-left:2px solid
#1010FF;margin-left:5px;padding-left:5px;color:#000;font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;">
<p>Snir.<br>
</p>
<blockquote
cite="mid:20190723082230.10381-3-fziglio@redhat.com">
<pre class="moz-quote-pre">Signed-off-by: Frediano Ziglio <a class="moz-txt-link-rfc2396E" href="mailto:fziglio@redhat.com" target="_blank" moz-do-not-send="true"><fziglio@redhat.com></a>
---
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;
};
</pre>
</blockquote>
</blockquote>
<div>Frediano</div>
<div><br>
</div>
</div>
</blockquote>
</body>
</html>