[Mesa-dev] [PATCH] mesa: handle zero-size buffers in MapBuffer and ranges in MapBufferRange

[Ian Romanick]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 08/30/2011 10:15 AM, Marek Olšák wrote:
> Moved from st/mesa.
> This also fixes an assertion failure in piglit/fdo31934 on gallium drivers.

So... I guess I don't understand why the mapping can't just fail and set
GL_OUT_OF_MEMORY.  Returning a pointer to some random bit of storage
seems like a recipe for bugs that are excruciating for app developers to
debug.

I'd be more inclined to return an invalid pointer, like (void
*)0xdeadbeef, so the reads or writes through it will cause the segfault
that the app deserves.

> ---
>  src/mesa/main/bufferobj.c                    |   18 +++++++++++++-
>  src/mesa/state_tracker/st_cb_bufferobjects.c |   32 ++++++-------------------
>  2 files changed, 25 insertions(+), 25 deletions(-)
> 
> diff --git a/src/mesa/main/bufferobj.c b/src/mesa/main/bufferobj.c
> index c453f9c..515abc5 100644
> --- a/src/mesa/main/bufferobj.c
> +++ b/src/mesa/main/bufferobj.c
> @@ -1004,6 +1004,13 @@ _mesa_MapBufferARB(GLenum target, GLenum access)
>        return NULL;
>     }
>  
> +   /* Handle zero-size buffers here rather than in drivers. */
> +   if (!bufObj->Size) {
> +      /* Dummy data whose pointer is used for zero-size buffers. */
> +      static long bufferobj_zero_length = 0;
> +      return &bufferobj_zero_length;
> +   }
> +
>     ASSERT(ctx->Driver.MapBufferRange);
>     map = ctx->Driver.MapBufferRange(ctx, 0, bufObj->Size, accessFlags, bufObj);
>     if (!map) {
> @@ -1410,7 +1417,16 @@ _mesa_MapBufferRange(GLenum target, GLintptr offset, GLsizeiptr length,
>                    "glMapBufferRange(buffer already mapped)");
>        return NULL;
>     }
> -      
> +
> +   /* We go out of way here to hide the degenerate yet valid case of zero
> +    * length range.
> +    */
> +   if (!length) {
> +      /* Dummy data whose pointer is used for zero-size buffers. */
> +      static long bufferobj_zero_length = 0;
> +      return &bufferobj_zero_length;
> +   }
> +
>     ASSERT(ctx->Driver.MapBufferRange);
>     map = ctx->Driver.MapBufferRange(ctx, offset, length, access, bufObj);
>     if (!map) {
> diff --git a/src/mesa/state_tracker/st_cb_bufferobjects.c b/src/mesa/state_tracker/st_cb_bufferobjects.c
> index a451b44..aab7444 100644
> --- a/src/mesa/state_tracker/st_cb_bufferobjects.c
> +++ b/src/mesa/state_tracker/st_cb_bufferobjects.c
> @@ -229,13 +229,6 @@ st_bufferobj_data(struct gl_context *ctx,
>  
>  
>  /**
> - * Dummy data whose's pointer is used for zero size buffers or ranges.
> - */
> -static long st_bufferobj_zero_length = 0;
> -
> -
> -
> -/**
>   * Called via glMapBufferRange().
>   */
>  static void *
> @@ -280,24 +273,15 @@ st_bufferobj_map_range(struct gl_context *ctx,
>     assert(offset < obj->Size);
>     assert(offset + length <= obj->Size);
>  
> -   /*
> -    * We go out of way here to hide the degenerate yet valid case of zero
> -    * length range from the pipe driver.
> -    */
> -   if (!length) {
> -      obj->Pointer = &st_bufferobj_zero_length;
> -   }
> -   else {
> -      obj->Pointer = pipe_buffer_map_range(pipe, 
> -                                           st_obj->buffer,
> -                                           offset, length,
> -                                           flags,
> -                                           &st_obj->transfer);
> -      if (obj->Pointer) {
> -         obj->Pointer = (ubyte *) obj->Pointer + offset;
> -      }
> +   obj->Pointer = pipe_buffer_map_range(pipe,
> +                                        st_obj->buffer,
> +                                        offset, length,
> +                                        flags,
> +                                        &st_obj->transfer);
> +   if (obj->Pointer) {
> +      obj->Pointer = (ubyte *) obj->Pointer + offset;
>     }
> -   
> +
>     if (obj->Pointer) {
>        obj->Offset = offset;
>        obj->Length = length;

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAk5dMdkACgkQX1gOwKyEAw8v8QCfWUbfzcT5cm6TG7bxKTeIkFJr
mREAoIICC08C/dEbMiLVL/jjv4NaWeZQ
=1eIR
-----END PGP SIGNATURE-----