[Mesa-dev] [PATCH 1/2] egl/drm: Fix misused x and y offsets on swrast_put_image2()

[Eric Engestrom]

On Tuesday, 2017-07-18 03:11:50 +0900, Gwan-gyeong Mun wrote:
> It fixes misused x and y offsets on the calculation of the memory copy regions.
> And it adds limits of the height and the width on the copy region.
> 
> Signed-off-by: Mun Gwan-gyeong <elongbug at gmail.com>

Fixes: 8430af5ebe1ee8119e14 "Add support for swrast to the DRM EGL platform"
Cc: Giovanni Campagna <gcampagna at src.gnome.org>

> ---
>  src/egl/drivers/dri2/platform_drm.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/src/egl/drivers/dri2/platform_drm.c b/src/egl/drivers/dri2/platform_drm.c
> index 4176fde7d1..36e9c74a56 100644
> --- a/src/egl/drivers/dri2/platform_drm.c
> +++ b/src/egl/drivers/dri2/platform_drm.c
> @@ -528,6 +528,9 @@ swrast_put_image2(__DRIdrawable *driDrawable,
>     struct dri2_egl_surface *dri2_surf = loaderPrivate;
>     int internal_stride;
>     struct gbm_dri_bo *bo;
> +   int x_offset = x * 4;
> +   int copy_width = width * 4;

These `4` look a bit magical here. I also don't think only 32bpp
formats are supported here; I see GBM_FORMAT_RGB565 a few lines below
for instance.
Any chance you could get the bpp from `bo->base.format` instead?

The names could also be improved: x_bytes & width_bytes maybe?

With that addressed, both patches are
Reviewed-by: Eric Engestrom <eric.engestrom at imgtec.com>

> +   char *src, *dst;
>  
>     if (op != __DRI_SWRAST_IMAGE_OP_DRAW &&
>         op != __DRI_SWRAST_IMAGE_OP_SWAP)
> @@ -542,9 +545,19 @@ swrast_put_image2(__DRIdrawable *driDrawable,
>  
>     internal_stride = bo->base.stride;
>  
> +   if (height > dri2_surf->base.Height - y)
> +      height = dri2_surf->base.Height - y;
> +
> +   if (copy_width > internal_stride - x_offset)
> +      copy_width = internal_stride - x_offset;
> +
> +   dst = bo->map + x_offset + (y * internal_stride);
> +   src = data;
> +
>     for (int i = 0; i < height; i++) {
> -      memcpy(bo->map + (x + i) * internal_stride + y,
> -             data + i * stride, stride);
> +      memcpy(dst, src, copy_width);
> +      dst += internal_stride;
> +      src += stride;
>     }
>  
>     gbm_dri_bo_unmap_dumb(bo);
> -- 
> 2.13.3
>