[Mesa-dev] [PATCH] mesa: handle errors in _mesa_unpack_image instead in unpack_image

[Yuanhan Liu]

Handle errors in _mesa_unpack_image instead in unpack_image. This
would make the error message more detailed and specified.

This patch does:
 1. trigger a GL_INVALID_VALUE if (width < 0 || height < 0 || depth < 0)

 2. do not trigger an error if (width == 0 || height == 0 || depth == 0)

    the old code would trigger a GL_OUT_OF_MEMORY error if user called
    glDrawPixels function with width == 0 and height == 0. This is wrong
    and will misguide user.

 3. trigger a GL_INVALID_ENUM error if bad format or type is met.

 4. do trigger a GL_OUT_OF_MEMORY error just when malloc failed.

Signed-off-by: Yuanhan Liu <yuanhan.liu at linux.intel.com>
---
 src/mesa/main/dlist.c |    9 +--------
 src/mesa/main/pack.c  |   27 ++++++++++++++++++---------
 2 files changed, 19 insertions(+), 17 deletions(-)

diff --git a/src/mesa/main/dlist.c b/src/mesa/main/dlist.c
index 6e075b4..21840e6 100644
--- a/src/mesa/main/dlist.c
+++ b/src/mesa/main/dlist.c
@@ -881,12 +881,8 @@ unpack_image(struct gl_context *ctx, GLuint dimensions,
 {
    if (!_mesa_is_bufferobj(unpack->BufferObj)) {
       /* no PBO */
-      GLvoid *image = _mesa_unpack_image(dimensions, width, height, depth,
+      return _mesa_unpack_image(dimensions, width, height, depth,
                                          format, type, pixels, unpack);
-      if (pixels && !image) {
-         _mesa_error(ctx, GL_OUT_OF_MEMORY, "display list construction");
-      }
-      return image;
    }
    else if (_mesa_validate_pbo_access(dimensions, unpack, width, height,
                                       depth, format, type, INT_MAX, pixels)) {
@@ -908,9 +904,6 @@ unpack_image(struct gl_context *ctx, GLuint dimensions,
 
       ctx->Driver.UnmapBuffer(ctx, unpack->BufferObj);
 
-      if (!image) {
-         _mesa_error(ctx, GL_OUT_OF_MEMORY, "display list construction");
-      }
       return image;
    }
    /* bad access! */
diff --git a/src/mesa/main/pack.c b/src/mesa/main/pack.c
index fd3f89d..ba5917d 100644
--- a/src/mesa/main/pack.c
+++ b/src/mesa/main/pack.c
@@ -5102,12 +5102,15 @@ _mesa_unpack_image( GLuint dimensions,
 {
    GLint bytesPerRow, compsPerRow;
    GLboolean flipBytes, swap2, swap4;
+   GET_CURRENT_CONTEXT(ctx);
 
-   if (!pixels)
-      return NULL;  /* not necessarily an error */
-
-   if (width <= 0 || height <= 0 || depth <= 0)
-      return NULL;  /* generate error later */
+   if (width < 0 || height < 0 || depth < 0) {
+      _mesa_error(ctx, GL_INVALID_VALUE, __func__);
+      return NULL;
+   } else if (!pixels || width == 0 || height == 0 || depth == 0) {
+      /* not necessarily an error */
+      return NULL;
+   }
 
    if (type == GL_BITMAP) {
       bytesPerRow = (width + 7) >> 3;
@@ -5123,8 +5126,12 @@ _mesa_unpack_image( GLuint dimensions,
       if (_mesa_type_is_packed(type))
           components = 1;
 
-      if (bytesPerPixel <= 0 || components <= 0)
-         return NULL;   /* bad format or type.  generate error later */
+      if (bytesPerPixel <= 0 || components <= 0) {
+         /* bad format or type.  generate error later */
+         _mesa_error(ctx, GL_INVALID_ENUM, __func__);
+         return NULL;
+      }
+
       bytesPerRow = bytesPerPixel * width;
       bytesPerComp = bytesPerPixel / components;
       flipBytes = GL_FALSE;
@@ -5139,8 +5146,10 @@ _mesa_unpack_image( GLuint dimensions,
          = (GLubyte *) malloc(bytesPerRow * height * depth);
       GLubyte *dst;
       GLint img, row;
-      if (!destBuffer)
-         return NULL;   /* generate GL_OUT_OF_MEMORY later */
+      if (!destBuffer) {
+         _mesa_error(ctx, GL_OUT_OF_MEMORY, __func__);
+         return NULL;
+      }
 
       dst = destBuffer;
       for (img = 0; img < depth; img++) {
-- 
1.7.4.4