[Mesa-dev] [PATCH 3/3] mesa: handle MapTextureImage() failures in mipmap generation code

[Eric Anholt]

On Sat, 26 Nov 2011 09:23:38 -0700, Brian Paul <brianp at vmware.com> wrote:
> And handle potential malloc failures too.
> ---
>  src/mesa/main/mipmap.c |   81 +++++++++++++++++++++++++++++++++--------------
>  1 files changed, 57 insertions(+), 24 deletions(-)
> 
> diff --git a/src/mesa/main/mipmap.c b/src/mesa/main/mipmap.c
> index 461de9d..c621800 100644
> --- a/src/mesa/main/mipmap.c
> +++ b/src/mesa/main/mipmap.c
> @@ -1825,6 +1825,7 @@ generate_mipmap_uncompressed(struct gl_context *ctx, GLenum target,
>        GLint slice;
>        GLboolean nextLevel;
>        GLubyte **srcMaps, **dstMaps;
> +      GLboolean success = GL_TRUE;
>  
>        /* get src image parameters */
>        srcImage = _mesa_select_tex_image(ctx, texObj, target, level);
> @@ -1873,42 +1874,74 @@ generate_mipmap_uncompressed(struct gl_context *ctx, GLenum target,
>        }
>  
>        /* Map src texture image slices */
> -      srcMaps = (GLubyte **) malloc(srcDepth * sizeof(GLubyte *));
> -      for (slice = 0; slice < srcDepth; slice++) {
> -         ctx->Driver.MapTextureImage(ctx, srcImage, slice,
> -                                     0, 0, srcWidth, srcHeight,
> -                                     GL_MAP_READ_BIT,
> -                                     &srcMaps[slice], &srcRowStride);
> +      srcMaps = (GLubyte **) calloc(srcDepth, sizeof(GLubyte *));
> +      if (srcMaps) {
> +         for (slice = 0; slice < srcDepth; slice++) {
> +            ctx->Driver.MapTextureImage(ctx, srcImage, slice,
> +                                        0, 0, srcWidth, srcHeight,
> +                                        GL_MAP_READ_BIT,
> +                                        &srcMaps[slice], &srcRowStride);
> +            if (!srcMaps[slice]) {
> +               success = GL_FALSE;
> +               break;
> +            }
> +         }
> +      }
> +      else {
> +         success = GL_FALSE;
>        }

Generally for error handling I prefer "if (failure) goto fail; /* label
at the end of the function */" type error handling to massive nesting
like we end up with here -- we end up with a big long "if (success) {
... }" block and then this dangling error handling case where you think
"oh, what is this testing for the failure of, again?"

That said, the patches appear to be correct, and would get a
Reviewed-by: Eric Anholt <eric at anholt.net>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20111126/9f1d9da5/attachment-0001.pgp>