[Mesa-dev] [PATCH 1/4] i965: Remove spurious casts in copy_image_with_memcpy()

[Kristian Høgsberg]

On Tue, Nov 18, 2014 at 9:02 PM, Chad Versace
<chad.versace at linux.intel.com> wrote:
> If a pointer points to raw, untyped memory and is never dereferenced,
> then declare it as 'void*' instead of casting it to 'void*'.
>
> Signed-off-by: Chad Versace <chad.versace at linux.intel.com>
> ---
>  src/mesa/drivers/dri/i965/intel_copy_image.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/intel_copy_image.c b/src/mesa/drivers/dri/i965/intel_copy_image.c
> index 341220c..cb44474 100644
> --- a/src/mesa/drivers/dri/i965/intel_copy_image.c
> +++ b/src/mesa/drivers/dri/i965/intel_copy_image.c
> @@ -144,7 +144,7 @@ copy_image_with_memcpy(struct brw_context *brw,
>                         int src_width, int src_height)
>  {
>     bool same_slice;
> -   uint8_t *mapped, *src_mapped, *dst_mapped;
> +   void *mapped, *src_mapped, *dst_mapped;

Making these void * means that this code below:

      src_mapped = mapped + ((src_y - map_y1) / src_bh) * src_stride +
                            ((src_x - map_x1) / src_bw) * cpp;

(same for dst_mapped) becomes arithmetic on void pointers. gcc
supports that and treats it as uint8_t pointer arithmetic [1], but I'm
not aware of any official C standard that allows it.  I don't think we
rely on that elsewhere and I don't think this patch improves
readability, portability or "pointer safety" of the code in question.
If you wanted to avoid void * arithmetic and casting to uint8_t, you'd
have to cast to an integer (uintptr_t) do the math there, then cast
back, but that's way more likely to hit a pointer size or 32/64 bit
issue.

Kristian

[1] http://gcc.gnu.org/onlinedocs/gcc-4.8.0/gcc/Pointer-Arith.html

>     int src_stride, dst_stride, i, cpp;
>     int map_x1, map_y1, map_x2, map_y2;
>     GLuint src_bw, src_bh;
> @@ -176,7 +176,7 @@ copy_image_with_memcpy(struct brw_context *brw,
>        intel_miptree_map(brw, src_mt, src_level, src_z,
>                          map_x1, map_y1, map_x2 - map_x1, map_y2 - map_y1,
>                          GL_MAP_READ_BIT | GL_MAP_WRITE_BIT,
> -                        (void **)&mapped, &src_stride);
> +                        &mapped, &src_stride);
>
>        dst_stride = src_stride;
>
> @@ -188,10 +188,10 @@ copy_image_with_memcpy(struct brw_context *brw,
>     } else {
>        intel_miptree_map(brw, src_mt, src_level, src_z,
>                          src_x, src_y, src_width, src_height,
> -                        GL_MAP_READ_BIT, (void **)&src_mapped, &src_stride);
> +                        GL_MAP_READ_BIT, &src_mapped, &src_stride);
>        intel_miptree_map(brw, dst_mt, dst_level, dst_z,
>                          dst_x, dst_y, src_width, src_height,
> -                        GL_MAP_WRITE_BIT, (void **)&dst_mapped, &dst_stride);
> +                        GL_MAP_WRITE_BIT, &dst_mapped, &dst_stride);
>     }
>
>     src_width /= (int)src_bw;
> --
> 2.1.0-rc0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev