[Spice-devel] [PATCH 2/3] server: Use the QXLPHYSICAL macros for conversions to/from pointers

Christophe Fergeau cfergeau at redhat.com
Wed Jan 6 03:40:00 PST 2016


Hey,

The patch looks good, however I think it would be better to use
memslots, replay.c registers a dummy one with spice-server with a 0 base
address/offset, so casting between pointers and QXLPHYSICAL works.
Passing the memslot info to red_replay_new() and using this for address
translations would be more robust, but might be a bit overkill as well.

So I'm fine with keeping this patch without mem slots for now.

Christophe

On Mon, Dec 28, 2015 at 11:06:43AM +0100, Francois Gouget wrote:
> This avoids compilation errors with -Werror on 32 bit systems.
> 
> Signed-off-by: Francois Gouget <fgouget at codeweavers.com>
> ---
>  server/red-replay-qxl.c | 92 ++++++++++++++++++++++++-------------------------
>  1 file changed, 46 insertions(+), 46 deletions(-)
> 
> diff --git a/server/red-replay-qxl.c b/server/red-replay-qxl.c
> index 66acf1e..25079e3 100644
> --- a/server/red-replay-qxl.c
> +++ b/server/red-replay-qxl.c
> @@ -277,8 +277,8 @@ static size_t red_replay_data_chunks(SpiceReplay *replay, const char *prefix,
>              return 0;
>          }
>          data_size += next_data_size;
> -        ((QXLDataChunk*)cur->next_chunk)->prev_chunk = (QXLPHYSICAL)cur;
> -        cur = (QXLDataChunk*)cur->next_chunk;
> +        QXLPHYSICAL_TO_PTR(QXLDataChunk*, cur->next_chunk)->prev_chunk = PTR_TO_QXLPHYSICAL(cur);
> +        cur =  QXLPHYSICAL_TO_PTR(QXLDataChunk*, cur->next_chunk);
>          cur->data_size = next_data_size;
>          cur->next_chunk = 0;
>      }
> @@ -291,9 +291,9 @@ static void red_replay_data_chunks_free(SpiceReplay *replay, void *data, size_t
>      QXLDataChunk *cur = (QXLDataChunk *)((uint8_t*)data +
>          (base_size ? base_size - sizeof(QXLDataChunk) : 0));
>  
> -    cur = (QXLDataChunk *)cur->next_chunk;
> +    cur = QXLPHYSICAL_TO_PTR(QXLDataChunk*, cur->next_chunk);
>      while (cur) {
> -        QXLDataChunk *next = (QXLDataChunk *)cur->next_chunk;
> +        QXLDataChunk *next = QXLPHYSICAL_TO_PTR(QXLDataChunk*, cur->next_chunk);
>          free(cur);
>          cur = next;
>      }
> @@ -326,7 +326,7 @@ static QXLPath *red_replay_path(SpiceReplay *replay)
>  
>  static void red_replay_path_free(SpiceReplay *replay, QXLPHYSICAL p)
>  {
> -    QXLPath *qxl = (QXLPath *)p;
> +    QXLPath *qxl = QXLPHYSICAL_TO_PTR(QXLPath*, p);
>  
>      red_replay_data_chunks_free(replay, qxl, sizeof(*qxl));
>  }
> @@ -392,7 +392,7 @@ static QXLImage *red_replay_image(SpiceReplay *replay, uint32_t flags)
>              replay_fscanf(replay, "qp.num_ents %d\n", &num_ents);
>              qp = malloc(sizeof(QXLPalette) + num_ents * sizeof(qp->ents[0]));
>              qp->num_ents = num_ents;
> -            qxl->bitmap.palette = (QXLPHYSICAL)qp;
> +            qxl->bitmap.palette = PTR_TO_QXLPHYSICAL(qp);
>              replay_fscanf(replay, "unique %"PRIu64"\n", &qp->unique);
>              for (i = 0; i < num_ents; i++) {
>                  replay_fscanf(replay, "ents %d\n", &qp->ents[i]);
> @@ -403,7 +403,7 @@ static QXLImage *red_replay_image(SpiceReplay *replay, uint32_t flags)
>          bitmap_size = qxl->bitmap.y * abs(qxl->bitmap.stride);
>          qxl->bitmap.data = 0;
>          if (qxl_flags & QXL_BITMAP_DIRECT) {
> -            qxl->bitmap.data = (QXLPHYSICAL)red_replay_image_data_flat(replay, &bitmap_size);
> +            qxl->bitmap.data = PTR_TO_QXLPHYSICAL(red_replay_image_data_flat(replay, &bitmap_size));
>          } else {
>              size = red_replay_data_chunks(replay, "bitmap.data", (uint8_t**)&qxl->bitmap.data, 0);
>              if (size != bitmap_size) {
> @@ -433,17 +433,17 @@ static QXLImage *red_replay_image(SpiceReplay *replay, uint32_t flags)
>  
>  static void red_replay_image_free(SpiceReplay *replay, QXLPHYSICAL p, uint32_t flags)
>  {
> -    QXLImage *qxl = (QXLImage *)p;
> +    QXLImage *qxl = QXLPHYSICAL_TO_PTR(QXLImage*, p);
>      if (!qxl)
>          return;
>  
>      switch (qxl->descriptor.type) {
>      case SPICE_IMAGE_TYPE_BITMAP:
> -        free((void*)qxl->bitmap.palette);
> +        free(QXLPHYSICAL_TO_PTR(void*, qxl->bitmap.palette));
>          if (qxl->bitmap.flags & QXL_BITMAP_DIRECT) {
> -            free((void*)qxl->bitmap.data);
> +            free(QXLPHYSICAL_TO_PTR(void*, qxl->bitmap.data));
>          } else {
> -            red_replay_data_chunks_free(replay, (void*)qxl->bitmap.data, 0);
> +            red_replay_data_chunks_free(replay, QXLPHYSICAL_TO_PTR(void*, qxl->bitmap.data), 0);
>          }
>          break;
>      case SPICE_IMAGE_TYPE_SURFACE:
> @@ -466,7 +466,7 @@ static void red_replay_brush_ptr(SpiceReplay *replay, QXLBrush *qxl, uint32_t fl
>          replay_fscanf(replay, "u.color %d\n", &qxl->u.color);
>          break;
>      case SPICE_BRUSH_TYPE_PATTERN:
> -        qxl->u.pattern.pat = (QXLPHYSICAL)red_replay_image(replay, flags);
> +        qxl->u.pattern.pat = PTR_TO_QXLPHYSICAL(red_replay_image(replay, flags));
>          red_replay_point_ptr(replay, &qxl->u.pattern.pos);
>          break;
>      }
> @@ -487,7 +487,7 @@ static void red_replay_qmask_ptr(SpiceReplay *replay, QXLQMask *qxl, uint32_t fl
>  
>      replay_fscanf(replay, "flags %d\n", &temp); qxl->flags = temp;
>      red_replay_point_ptr(replay, &qxl->pos);
> -    qxl->bitmap = (QXLPHYSICAL)red_replay_image(replay, flags);
> +    qxl->bitmap = PTR_TO_QXLPHYSICAL(red_replay_image(replay, flags));
>  }
>  
>  static void red_replay_qmask_free(SpiceReplay *replay, QXLQMask *qxl, uint32_t flags)
> @@ -514,7 +514,7 @@ static void red_replay_opaque_ptr(SpiceReplay *replay, QXLOpaque *qxl, uint32_t
>  {
>      int temp;
>  
> -    qxl->src_bitmap = (QXLPHYSICAL)red_replay_image(replay, flags);
> +    qxl->src_bitmap = PTR_TO_QXLPHYSICAL(red_replay_image(replay, flags));
>      red_replay_rect_ptr(replay, "src_area", &qxl->src_area);
>      red_replay_brush_ptr(replay, &qxl->brush, flags);
>      replay_fscanf(replay, "rop_descriptor %d\n", &temp); qxl->rop_descriptor = temp;
> @@ -533,7 +533,7 @@ static void red_replay_copy_ptr(SpiceReplay *replay, QXLCopy *qxl, uint32_t flag
>  {
>      int temp;
>  
> -   qxl->src_bitmap = (QXLPHYSICAL)red_replay_image(replay, flags);
> +   qxl->src_bitmap = PTR_TO_QXLPHYSICAL(red_replay_image(replay, flags));
>     red_replay_rect_ptr(replay, "src_area", &qxl->src_area);
>     replay_fscanf(replay, "rop_descriptor %d\n", &temp); qxl->rop_descriptor = temp;
>     replay_fscanf(replay, "scale_mode %d\n", &temp); qxl->scale_mode = temp;
> @@ -550,7 +550,7 @@ static void red_replay_blend_ptr(SpiceReplay *replay, QXLBlend *qxl, uint32_t fl
>  {
>      int temp;
>  
> -   qxl->src_bitmap = (QXLPHYSICAL)red_replay_image(replay, flags);
> +   qxl->src_bitmap = PTR_TO_QXLPHYSICAL(red_replay_image(replay, flags));
>     red_replay_rect_ptr(replay, "src_area", &qxl->src_area);
>     replay_fscanf(replay, "rop_descriptor %d\n", &temp); qxl->rop_descriptor = temp;
>     replay_fscanf(replay, "scale_mode %d\n", &temp); qxl->scale_mode = temp;
> @@ -565,7 +565,7 @@ static void red_replay_blend_free(SpiceReplay *replay, QXLBlend *qxl, uint32_t f
>  
>  static void red_replay_transparent_ptr(SpiceReplay *replay, QXLTransparent *qxl, uint32_t flags)
>  {
> -   qxl->src_bitmap = (QXLPHYSICAL)red_replay_image(replay, flags);
> +   qxl->src_bitmap = PTR_TO_QXLPHYSICAL(red_replay_image(replay, flags));
>     red_replay_rect_ptr(replay, "src_area", &qxl->src_area);
>     replay_fscanf(replay, "src_color %d\n", &qxl->src_color);
>     replay_fscanf(replay, "true_color %d\n", &qxl->true_color);
> @@ -582,7 +582,7 @@ static void red_replay_alpha_blend_ptr(SpiceReplay *replay, QXLAlphaBlend *qxl,
>  
>      replay_fscanf(replay, "alpha_flags %d\n", &temp); qxl->alpha_flags = temp;
>      replay_fscanf(replay, "alpha %d\n", &temp); qxl->alpha = temp;
> -    qxl->src_bitmap = (QXLPHYSICAL)red_replay_image(replay, flags);
> +    qxl->src_bitmap = PTR_TO_QXLPHYSICAL(red_replay_image(replay, flags));
>      red_replay_rect_ptr(replay, "src_area", &qxl->src_area);
>  }
>  
> @@ -596,7 +596,7 @@ static void red_replay_alpha_blend_ptr_compat(SpiceReplay *replay, QXLCompatAlph
>      int temp;
>  
>      replay_fscanf(replay, "alpha %d\n", &temp); qxl->alpha = temp;
> -    qxl->src_bitmap = (QXLPHYSICAL)red_replay_image(replay, flags);
> +    qxl->src_bitmap = PTR_TO_QXLPHYSICAL(red_replay_image(replay, flags));
>      red_replay_rect_ptr(replay, "src_area", &qxl->src_area);
>  }
>  
> @@ -604,7 +604,7 @@ static void red_replay_rop3_ptr(SpiceReplay *replay, QXLRop3 *qxl, uint32_t flag
>  {
>      int temp;
>  
> -    qxl->src_bitmap = (QXLPHYSICAL)red_replay_image(replay, flags);
> +    qxl->src_bitmap = PTR_TO_QXLPHYSICAL(red_replay_image(replay, flags));
>      red_replay_rect_ptr(replay, "src_area", &qxl->src_area);
>      red_replay_brush_ptr(replay, &qxl->brush, flags);
>      replay_fscanf(replay, "rop3 %d\n", &temp); qxl->rop3 = temp;
> @@ -623,7 +623,7 @@ static void red_replay_stroke_ptr(SpiceReplay *replay, QXLStroke *qxl, uint32_t
>  {
>      int temp;
>  
> -    qxl->path = (QXLPHYSICAL)red_replay_path(replay);
> +    qxl->path = PTR_TO_QXLPHYSICAL(red_replay_path(replay));
>      replay_fscanf(replay, "attr.flags %d\n", &temp); qxl->attr.flags = temp;
>      if (qxl->attr.flags & SPICE_LINE_FLAGS_STYLED) {
>          size_t size;
> @@ -640,7 +640,7 @@ static void red_replay_stroke_free(SpiceReplay *replay, QXLStroke *qxl, uint32_t
>  {
>      red_replay_path_free(replay, qxl->path);
>      if (qxl->attr.flags & SPICE_LINE_FLAGS_STYLED) {
> -        free((void*)qxl->attr.style);
> +        free(QXLPHYSICAL_TO_PTR(void*, qxl->attr.style));
>      }
>      red_replay_brush_free(replay, &qxl->brush, flags);
>  }
> @@ -674,7 +674,7 @@ static void red_replay_text_ptr(SpiceReplay *replay, QXLText *qxl, uint32_t flag
>  {
>      int temp;
>  
> -   qxl->str = (QXLPHYSICAL)red_replay_string(replay);
> +   qxl->str = PTR_TO_QXLPHYSICAL(red_replay_string(replay));
>     red_replay_rect_ptr(replay, "back_area", &qxl->back_area);
>     red_replay_brush_ptr(replay, &qxl->fore_brush, flags);
>     red_replay_brush_ptr(replay, &qxl->back_brush, flags);
> @@ -684,7 +684,7 @@ static void red_replay_text_ptr(SpiceReplay *replay, QXLText *qxl, uint32_t flag
>  
>  static void red_replay_text_free(SpiceReplay *replay, QXLText *qxl, uint32_t flags)
>  {
> -    red_replay_string_free(replay, (QXLString *)qxl->str);
> +    red_replay_string_free(replay, QXLPHYSICAL_TO_PTR(QXLString*, qxl->str));
>      red_replay_brush_free(replay, &qxl->fore_brush, flags);
>      red_replay_brush_free(replay, &qxl->back_brush, flags);
>  }
> @@ -724,7 +724,7 @@ static void red_replay_clip_ptr(SpiceReplay *replay, QXLClip *qxl)
>      replay_fscanf(replay, "type %d\n", &qxl->type);
>      switch (qxl->type) {
>      case SPICE_CLIP_TYPE_RECTS:
> -        qxl->data = (QXLPHYSICAL)red_replay_clip_rects(replay);
> +        qxl->data = PTR_TO_QXLPHYSICAL(red_replay_clip_rects(replay));
>          break;
>      }
>  }
> @@ -733,7 +733,7 @@ static void red_replay_clip_free(SpiceReplay *replay, QXLClip *qxl)
>  {
>      switch (qxl->type) {
>      case SPICE_CLIP_TYPE_RECTS:
> -        red_replay_clip_rects_free(replay, (QXLClipRects*)qxl->data);
> +        red_replay_clip_rects_free(replay, QXLPHYSICAL_TO_PTR(QXLClipRects*, qxl->data));
>          break;
>      }
>  }
> @@ -754,16 +754,16 @@ static void red_replay_composite_ptr(SpiceReplay *replay, QXLComposite *qxl, uin
>      int enabled;
>  
>      replay_fscanf(replay, "flags %d\n", &qxl->flags);
> -    qxl->src = (QXLPHYSICAL)red_replay_image(replay, flags);
> +    qxl->src = PTR_TO_QXLPHYSICAL(red_replay_image(replay, flags));
>  
>      replay_fscanf(replay, "src_transform %d\n", &enabled);
> -    qxl->src_transform = enabled ? (QXLPHYSICAL)red_replay_transform(replay) : 0;
> +    qxl->src_transform = enabled ?  PTR_TO_QXLPHYSICAL(red_replay_transform(replay)) : 0;
>  
>      replay_fscanf(replay, "mask %d\n", &enabled);
> -    qxl->mask = enabled ? (QXLPHYSICAL)red_replay_image(replay, flags) : 0;
> +    qxl->mask = enabled ? PTR_TO_QXLPHYSICAL(red_replay_image(replay, flags)) : 0;
>  
>      replay_fscanf(replay, "mask_transform %d\n", &enabled);
> -    qxl->mask_transform = enabled ? (QXLPHYSICAL)red_replay_transform(replay) : 0;
> +    qxl->mask_transform = enabled ?  PTR_TO_QXLPHYSICAL(red_replay_transform(replay)) : 0;
>  
>      replay_fscanf(replay, "src_origin %" SCNi16 " %" SCNi16 "\n", &qxl->src_origin.x, &qxl->src_origin.y);
>      replay_fscanf(replay, "mask_origin %" SCNi16 " %" SCNi16 "\n", &qxl->mask_origin.x, &qxl->mask_origin.y);
> @@ -772,9 +772,9 @@ static void red_replay_composite_ptr(SpiceReplay *replay, QXLComposite *qxl, uin
>  static void red_replay_composite_free(SpiceReplay *replay, QXLComposite *qxl, uint32_t flags)
>  {
>      red_replay_image_free(replay, qxl->src, flags);
> -    free((void*)qxl->src_transform);
> +    free(QXLPHYSICAL_TO_PTR(void*, qxl->src_transform));
>      red_replay_image_free(replay, qxl->mask, flags);
> -    free((void*)qxl->mask_transform);
> +    free(QXLPHYSICAL_TO_PTR(void*, qxl->mask_transform));
>  
>  }
>  
> @@ -978,9 +978,9 @@ static QXLPHYSICAL red_replay_drawable(SpiceReplay *replay, uint32_t flags)
>      }
>      replay_fscanf(replay, "drawable\n");
>      if (flags & QXL_COMMAND_FLAG_COMPAT) {
> -        return (QXLPHYSICAL)red_replay_compat_drawable(replay, flags);
> +        return PTR_TO_QXLPHYSICAL(red_replay_compat_drawable(replay, flags));
>      } else {
> -        return (QXLPHYSICAL)red_replay_native_drawable(replay, flags);
> +        return PTR_TO_QXLPHYSICAL(red_replay_native_drawable(replay, flags));
>      }
>  }
>  
> @@ -1031,12 +1031,12 @@ static QXLSurfaceCmd *red_replay_surface_cmd(SpiceReplay *replay)
>                  spice_printerr("mismatch %zu != %zu", size, read_size);
>              }
>          } else {
> -            qxl->u.surface_create.data = (QXLPHYSICAL)malloc(size);
> +            qxl->u.surface_create.data = PTR_TO_QXLPHYSICAL(malloc(size));
>          }
>          qxl->surface_id = replay_id_new(replay, qxl->surface_id);
>          break;
>      case QXL_SURFACE_CMD_DESTROY:
> -        qxl->u.surface_create.data = (QXLPHYSICAL)NULL;
> +        qxl->u.surface_create.data = 0;
>          qxl->surface_id = replay_id_get(replay, qxl->surface_id);
>      }
>      return qxl;
> @@ -1048,7 +1048,7 @@ static void red_replay_surface_cmd_free(SpiceReplay *replay, QXLSurfaceCmd *qxl)
>          replay_id_free(replay, qxl->surface_id);
>      }
>  
> -    free((void*)qxl->u.surface_create.data);
> +    free(QXLPHYSICAL_TO_PTR(void*, qxl->u.surface_create.data));
>      free(qxl);
>  }
>  
> @@ -1072,7 +1072,7 @@ static void replay_handle_create_primary(QXLWorker *worker, SpiceReplay *replay)
>          &surface.flags, &surface.type);
>      read_binary(replay, "data", &size, &mem, 0);
>      surface.group_id = 0;
> -    surface.mem = (QXLPHYSICAL)mem;
> +    surface.mem = PTR_TO_QXLPHYSICAL(mem);
>      worker->create_primary_surface(worker, 0, &surface);
>  }
>  
> @@ -1140,13 +1140,13 @@ SPICE_GNUC_VISIBLE QXLCommandExt* spice_replay_next_cmd(SpiceReplay *replay,
>          cmd->cmd.data = red_replay_drawable(replay, cmd->flags);
>          break;
>      case QXL_CMD_UPDATE:
> -        cmd->cmd.data = (QXLPHYSICAL)red_replay_update_cmd(replay);
> +        cmd->cmd.data = PTR_TO_QXLPHYSICAL(red_replay_update_cmd(replay));
>          break;
>      case QXL_CMD_MESSAGE:
> -        cmd->cmd.data = (QXLPHYSICAL)red_replay_message(replay);
> +        cmd->cmd.data = PTR_TO_QXLPHYSICAL(red_replay_message(replay));
>          break;
>      case QXL_CMD_SURFACE:
> -        cmd->cmd.data = (QXLPHYSICAL)red_replay_surface_cmd(replay);
> +        cmd->cmd.data = PTR_TO_QXLPHYSICAL(red_replay_surface_cmd(replay));
>          break;
>      }
>  
> @@ -1155,8 +1155,8 @@ SPICE_GNUC_VISIBLE QXLCommandExt* spice_replay_next_cmd(SpiceReplay *replay,
>      case QXL_CMD_DRAW:
>      case QXL_CMD_UPDATE:
>      case QXL_CMD_SURFACE:
> -        info = (QXLReleaseInfo *)cmd->cmd.data;
> -        info->id = (uint64_t)cmd;
> +        info = QXLPHYSICAL_TO_PTR(QXLReleaseInfo*, cmd->cmd.data);
> +        info->id = (uint64_t)(uintptr_t)cmd;
>      }
>  
>      replay->counter++;
> @@ -1173,17 +1173,17 @@ SPICE_GNUC_VISIBLE void spice_replay_free_cmd(SpiceReplay *replay, QXLCommandExt
>      case QXL_CMD_DRAW: {
>          // FIXME: compat flag must be save somewhere...
>          spice_return_if_fail(cmd->flags == 0);
> -        QXLDrawable *qxl = (QXLDrawable *)cmd->cmd.data;
> +        QXLDrawable *qxl = QXLPHYSICAL_TO_PTR(QXLDrawable*, cmd->cmd.data);
>          red_replay_native_drawable_free(replay, qxl, cmd->flags);
>          break;
>      }
>      case QXL_CMD_UPDATE: {
> -        QXLUpdateCmd *qxl = (QXLUpdateCmd *)cmd->cmd.data;
> +        QXLUpdateCmd *qxl = QXLPHYSICAL_TO_PTR(QXLUpdateCmd*, cmd->cmd.data);
>          free(qxl);
>          break;
>      }
>      case QXL_CMD_SURFACE: {
> -        QXLSurfaceCmd *qxl = (QXLSurfaceCmd *)cmd->cmd.data;
> +        QXLSurfaceCmd *qxl = QXLPHYSICAL_TO_PTR(QXLSurfaceCmd*, cmd->cmd.data);
>          red_replay_surface_cmd_free(replay, qxl);
>          break;
>      }
> -- 
> 2.6.4
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20160106/6cb50a93/attachment-0001.sig>


More information about the Spice-devel mailing list