[Mesa-dev] [PATCH 1/5] Mesa: Change save_attrib_data() to return boolean

[Juha-Pekka Heikkilä]

On Thu, Dec 12, 2013 at 3:14 AM, Brian Paul <brianp at vmware.com> wrote:
> On 12/11/2013 02:05 AM, Juha-Pekka Heikkila wrote:
>>
>> Change save_attrib_data() to return true/false depending on success.
>>
>> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila at gmail.com>
>
>
> I'd like to reconsider these changes from scratch.
>
> The basic issue is just null-checking of the malloc calls in
> glPush[Client]Attrib().  Why can't we just do something like the (partial)
> patch below?
>

The patch below is really similar to my original try at this. Ian
pointed out from my first try if save_attrib_data would fail we'd lose
"attr" thus I now have evolved the patch to the way it is. I think now
it would not in any case leak anything. The original patch for this
issue with later comments was here:
http://lists.freedesktop.org/archives/mesa-dev/2013-December/049628.html

As a small bonus using helper in glPushAttrib() make the function more
clean looking and easier to follow.

>
>
> diff --git a/src/mesa/main/attrib.c b/src/mesa/main/attrib.c
> index 8a2a268..1ab0406 100644
> --- a/src/mesa/main/attrib.c
> +++ b/src/mesa/main/attrib.c
> @@ -195,7 +195,8 @@ save_attrib_data(struct gl_attrib_node **head,
>        *head = n;
>     }
>     else {
> -      /* out of memory! */
> +      GET_CURRENT_CONTEXT(ctx);
> +      _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
>     }
>  }
>
> @@ -222,6 +223,8 @@ _mesa_PushAttrib(GLbitfield mask)
>     if (mask & GL_ACCUM_BUFFER_BIT) {
>        struct gl_accum_attrib *attr;
>        attr = MALLOC_STRUCT( gl_accum_attrib );
> +      if (!attr)
> +         goto out_of_memory;
>        memcpy( attr, &ctx->Accum, sizeof(struct gl_accum_attrib) );
>        save_attrib_data(&head, GL_ACCUM_BUFFER_BIT, attr);
>     }
> @@ -230,6 +233,8 @@ _mesa_PushAttrib(GLbitfield mask)
>        GLuint i;
>        struct gl_colorbuffer_attrib *attr;
>        attr = MALLOC_STRUCT( gl_colorbuffer_attrib );
> +      if (!attr)
> +         goto out_of_memory;
>        memcpy( attr, &ctx->Color, sizeof(struct gl_colorbuffer_attrib) );
>        /* push the Draw FBO's DrawBuffer[] state, not
> ctx->Color.DrawBuffer[] */
>        for (i = 0; i < ctx->Const.MaxDrawBuffers; i ++)
> @@ -241,6 +246,8 @@ _mesa_PushAttrib(GLbitfield mask)
>        struct gl_current_attrib *attr;
>        FLUSH_CURRENT( ctx, 0 );
>        attr = MALLOC_STRUCT( gl_current_attrib );
> +      if (!attr)
> +         goto out_of_memory;
>        memcpy( attr, &ctx->Current, sizeof(struct gl_current_attrib) );
>        save_attrib_data(&head, GL_CURRENT_BIT, attr);
>     }
> @@ -420,8 +427,7 @@ _mesa_PushAttrib(GLbitfield mask)
>        GLuint u, tex;
>
>        if (!texstate) {
> -         _mesa_error(ctx, GL_OUT_OF_MEMORY,
> "glPushAttrib(GL_TEXTURE_BIT)");
> -         goto end;
> +         goto out_of_memory;
>        }
>
>        _mesa_lock_context_textures(ctx);
> @@ -476,9 +482,16 @@ _mesa_PushAttrib(GLbitfield mask)
>        save_attrib_data(&head, GL_MULTISAMPLE_BIT_ARB, attr);
>     }
>
> +   goto end;
> +
> +out_of_memory:
> +   _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
> +
>  end:
> -   ctx->AttribStack[ctx->AttribStackDepth] = head;
> -   ctx->AttribStackDepth++;
> +   if (head) {
> +      ctx->AttribStack[ctx->AttribStackDepth] = head;
> +      ctx->AttribStackDepth++;
> +   }
>  }
>
>
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev