On Thu, Dec 12, 2013 at 3:14 AM, Brian Paul <bri...@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.heikk...@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@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
