[Spice-devel] Wrong math in spice-common common/canvas_base.c ?

Frediano Ziglio fziglio at redhat.com
Tue Jan 23 14:02:23 UTC 2018


> 
> Hi,
>   was looking at usage of SPICE_UNALIGNED_CAST and I found this code:
> 
> 
>     /* round down X to even 32 pixels (i.e. uint32_t) */
>     extents.x1 = extents.x1 & ~(0x1f);
> 
>     mask_data_src = (uint8_t *)mask_data + mask_stride * extents.y1 +
>     extents.x1 / 32;
>     mask_data = SPICE_UNALIGNED_CAST(uint32_t *, mask_data_src);
> 
>     mask_x -= extents.x1;
>     mask_y -= extents.y1;
>     mask_width = extents.x2 - extents.x1;
>     mask_height = extents.y2 - extents.y1;
> 
>     subimage = pixman_image_create_bits(PIXMAN_a1, mask_width, mask_height,
>                                         mask_data, mask_stride);
>     pixman_region32_init_from_image(&mask_region,
>                                     subimage);
>     pixman_image_unref(subimage);
> 
> 
> the mask_data_src computation seems wrong to me however there seem no reports
> of rendering
> issues. Particularly the "extents.x1 / 32" part.
> The mask_data pointer is used to build a 1 bit image (PIXMAN_a1) used later
> to initialize
> a region (pixman_region32_init_from_image).
> The initial mask_data (before mask_data_src computation) points to a uint32_t
> which is the
> image raw data start. The fact that this data is used as a 1 bit image raw
> data seems to
> indicate that the image is 1 bit too. mask_stride is expressed in bytes (OT
> and always
> 4 multiple) and mask_data is cast to uint8_t* computing mask_data_src so it
> seems that
> the "extents.x1 / 32" offset should be in bytes however is is a pointer to 1
> bit raw data
> should be "extents.x1 / 8".
> 
> Are we just lucky and issues never happens for some reasons (like very rare
> rendering)
> or the computation is wrong?
> 

Tried to reproduce with some recording, I put an exit call in the code,
apparently is a dead path, at least for my recordings!

Frediano


More information about the Spice-devel mailing list