[Spice-devel] [PATCH spice-server 1/2] memslot: Return void* from memslot_get_virt

[Christophe Fergeau]

On Tue, Jun 19, 2018 at 11:05:24AM +0100, Frediano Ziglio wrote:
> The result of this function is always cast to a pointer, there
> is no reason to return an integer.
> This API looks copied from a kernel environment where virtual
> addresses can have different sizes compare to pointers.


Acked-by: Christophe Fergeau <cfergeau at redhat.com>
but why isn't this removing all the other (Foo*)memslots_get_virt()
casts from red-parse-qxl.c? I can send a followup patch.

Christophe

> 
> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> ---
>  server/memslot.c       | 14 +++++++-------
>  server/memslot.h       |  2 +-
>  server/red-parse-qxl.c |  4 ++--
>  3 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/server/memslot.c b/server/memslot.c
> index 7074b432..a87a717b 100644
> --- a/server/memslot.c
> +++ b/server/memslot.c
> @@ -89,8 +89,8 @@ unsigned long memslot_max_size_virt(RedMemSlotInfo *info,
>   * return virtual address if successful, which may be 0.
>   * returns 0 and sets error to 1 if an error condition occurs.
>   */
> -unsigned long memslot_get_virt(RedMemSlotInfo *info, QXLPHYSICAL addr, uint32_t add_size,
> -                               int group_id, int *error)
> +void* memslot_get_virt(RedMemSlotInfo *info, QXLPHYSICAL addr, uint32_t add_size,
> +                       int group_id, int *error)
>  {
>      int slot_id;
>      int generation;
> @@ -102,7 +102,7 @@ unsigned long memslot_get_virt(RedMemSlotInfo *info, QXLPHYSICAL addr, uint32_t
>      if (group_id > info->num_memslots_groups) {
>          spice_critical("group_id too big");
>          *error = 1;
> -        return 0;
> +        return NULL;
>      }
>  
>      slot_id = memslot_get_id(info, addr);
> @@ -110,7 +110,7 @@ unsigned long memslot_get_virt(RedMemSlotInfo *info, QXLPHYSICAL addr, uint32_t
>          print_memslots(info);
>          spice_critical("slot_id %d too big, addr=%" PRIx64, slot_id, addr);
>          *error = 1;
> -        return 0;
> +        return NULL;
>      }
>  
>      slot = &info->mem_slots[group_id][slot_id];
> @@ -121,7 +121,7 @@ unsigned long memslot_get_virt(RedMemSlotInfo *info, QXLPHYSICAL addr, uint32_t
>          spice_critical("address generation is not valid, group_id %d, slot_id %d, gen %d, slot_gen %d\n",
>                group_id, slot_id, generation, slot->generation);
>          *error = 1;
> -        return 0;
> +        return NULL;
>      }
>  
>      h_virt = __get_clean_virt(info, addr);
> @@ -129,10 +129,10 @@ unsigned long memslot_get_virt(RedMemSlotInfo *info, QXLPHYSICAL addr, uint32_t
>  
>      if (!memslot_validate_virt(info, h_virt, slot_id, add_size, group_id)) {
>          *error = 1;
> -        return 0;
> +        return NULL;
>      }
>  
> -    return h_virt;
> +    return (void*)(uintptr_t)h_virt;
>  }
>  
>  void memslot_info_init(RedMemSlotInfo *info,
> diff --git a/server/memslot.h b/server/memslot.h
> index 71f1210d..d8d67d55 100644
> --- a/server/memslot.h
> +++ b/server/memslot.h
> @@ -58,7 +58,7 @@ int memslot_validate_virt(RedMemSlotInfo *info, unsigned long virt, int slot_id,
>  unsigned long memslot_max_size_virt(RedMemSlotInfo *info,
>                                      unsigned long virt, int slot_id,
>                                      uint32_t group_id);
> -unsigned long memslot_get_virt(RedMemSlotInfo *info, QXLPHYSICAL addr, uint32_t add_size,
> +void *memslot_get_virt(RedMemSlotInfo *info, QXLPHYSICAL addr, uint32_t add_size,
>                         int group_id, int *error);
>  
>  void memslot_info_init(RedMemSlotInfo *info,
> diff --git a/server/red-parse-qxl.c b/server/red-parse-qxl.c
> index 4a45c0f7..d28c935f 100644
> --- a/server/red-parse-qxl.c
> +++ b/server/red-parse-qxl.c
> @@ -371,7 +371,7 @@ static SpiceChunks *red_get_image_data_flat(RedMemSlotInfo *slots, int group_id,
>  {
>      SpiceChunks *data;
>      int error;
> -    unsigned long bitmap_virt;
> +    void *bitmap_virt;
>  
>      bitmap_virt = memslot_get_virt(slots, addr, size, group_id, &error);
>      if (error) {
> @@ -380,7 +380,7 @@ static SpiceChunks *red_get_image_data_flat(RedMemSlotInfo *slots, int group_id,
>  
>      data = spice_chunks_new(1);
>      data->data_size      = size;
> -    data->chunk[0].data  = (void*)bitmap_virt;
> +    data->chunk[0].data  = bitmap_virt;
>      data->chunk[0].len   = size;
>      return data;
>  }
> -- 
> 2.17.1
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20180626/9122e668/attachment-0001.sig>