[Mesa-dev] [PATCH 3/5] mesa: Verify memory allocations success in _mesa_PushAttrib

Brian Paul brianp at vmware.com
Thu Dec 12 07:11:54 PST 2013


On 12/11/2013 02:05 AM, Juha-Pekka Heikkila wrote:
> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila at gmail.com>
> ---
>   src/mesa/main/attrib.c | 227 +++++++++++++++++++++++++++++++------------------
>   1 file changed, 144 insertions(+), 83 deletions(-)
>
> diff --git a/src/mesa/main/attrib.c b/src/mesa/main/attrib.c
> index 87797d1..e41b981 100644
> --- a/src/mesa/main/attrib.c
> +++ b/src/mesa/main/attrib.c
> @@ -201,6 +201,30 @@ save_attrib_data(struct gl_attrib_node **head,
>      return true;
>   }
>
> +static bool
> +push_attrib_helper(struct gl_context *ctx, GLbitfield mask,
> +                   struct gl_attrib_node **head, GLuint alloc_size,
> +                   void *memcpy_target)

Let's rename some of those parameters:
alloc_size -> attr_size
memcpy_target -> attr_data (and make it const void *).

And please put a comment on the function describing what it does.


> +{
> +   void *attribute;
> +
> +   attribute = MALLOC( alloc_size );

You can remove the space after ( and before ) in function calls, more below.


> +   if (attribute == NULL) {
> +      _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
> +      return false;
> +   }
> +
> +   if (save_attrib_data(head, mask, attribute)) {
> +      memcpy( attribute, memcpy_target, alloc_size );
> +   }
> +   else {
> +      FREE( attribute );
> +      _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
> +      return false;
> +   }
> +   return true;
> +}
> +
>
>   void GLAPIENTRY
>   _mesa_PushAttrib(GLbitfield mask)
> @@ -222,42 +246,58 @@ _mesa_PushAttrib(GLbitfield mask)
>      head = NULL;
>
>      if (mask & GL_ACCUM_BUFFER_BIT) {
> -      struct gl_accum_attrib *attr;
> -      attr = MALLOC_STRUCT( gl_accum_attrib );
> -      memcpy( attr, &ctx->Accum, sizeof(struct gl_accum_attrib) );
> -      save_attrib_data(&head, GL_ACCUM_BUFFER_BIT, attr);
> +      if(!push_attrib_helper(ctx, GL_ACCUM_BUFFER_BIT, &head,

Space between if and (.


> +                             sizeof(struct gl_accum_attrib),
> +                             (void*)&ctx->Accum))
> +         goto end;
>      }
>
>      if (mask & GL_COLOR_BUFFER_BIT) {
>         GLuint i;
>         struct gl_colorbuffer_attrib *attr;
>         attr = MALLOC_STRUCT( gl_colorbuffer_attrib );
> -      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 ++)
> -         attr->DrawBuffer[i] = ctx->DrawBuffer->ColorDrawBuffer[i];
> -      save_attrib_data(&head, GL_COLOR_BUFFER_BIT, attr);
> +      if (attr == NULL) {
> +         _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
> +         goto end;
> +      }
> +
> +    if (save_attrib_data(&head, GL_COLOR_BUFFER_BIT, attr)) {
> +         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 ++)
> +            attr->DrawBuffer[i] = ctx->DrawBuffer->ColorDrawBuffer[i];
> +      }
> +      else {
> +         FREE( attr );
> +         _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
> +         goto end;
> +      }
>      }
>
>      if (mask & GL_CURRENT_BIT) {
> -      struct gl_current_attrib *attr;
>         FLUSH_CURRENT( ctx, 0 );
> -      attr = MALLOC_STRUCT( gl_current_attrib );
> -      memcpy( attr, &ctx->Current, sizeof(struct gl_current_attrib) );
> -      save_attrib_data(&head, GL_CURRENT_BIT, attr);
> +      if (!push_attrib_helper(ctx, GL_CURRENT_BIT, &head,
> +                              sizeof(struct gl_current_attrib),
> +                              (void*)&ctx->Current))
> +         goto end;
>      }
>
>      if (mask & GL_DEPTH_BUFFER_BIT) {
> -      struct gl_depthbuffer_attrib *attr;
> -      attr = MALLOC_STRUCT( gl_depthbuffer_attrib );
> -      memcpy( attr, &ctx->Depth, sizeof(struct gl_depthbuffer_attrib) );
> -      save_attrib_data(&head, GL_DEPTH_BUFFER_BIT, attr);
> +      if (!push_attrib_helper(ctx, GL_DEPTH_BUFFER_BIT, &head,
> +                              sizeof(struct gl_depthbuffer_attrib),
> +                              (void*)&ctx->Depth))
> +         goto end;
>      }
>
>      if (mask & GL_ENABLE_BIT) {
>         struct gl_enable_attrib *attr;
>         GLuint i;
>         attr = MALLOC_STRUCT( gl_enable_attrib );
> +      if (attr == NULL) {
> +         _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
> +         goto end;
> +      }
> +
>         /* Copy enable flags from all other attributes into the enable struct. */
>         attr->AlphaTest = ctx->Color.AlphaEnabled;
>         attr->AutoNormal = ctx->Eval.AutoNormal;
> @@ -324,97 +364,112 @@ _mesa_PushAttrib(GLbitfield mask)
>         /* GL_ARB_fragment_program */
>         attr->FragmentProgram = ctx->FragmentProgram.Enabled;
>
> -      save_attrib_data(&head, GL_ENABLE_BIT, attr);
> +      if (!save_attrib_data(&head, GL_ENABLE_BIT, attr)) {
> +         FREE( attr );
> +         _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
> +         goto end;
> +      }
>
>         /* GL_ARB_framebuffer_sRGB / GL_EXT_framebuffer_sRGB */
>         attr->sRGBEnabled = ctx->Color.sRGBEnabled;
>      }
>
>      if (mask & GL_EVAL_BIT) {
> -      struct gl_eval_attrib *attr;
> -      attr = MALLOC_STRUCT( gl_eval_attrib );
> -      memcpy( attr, &ctx->Eval, sizeof(struct gl_eval_attrib) );
> -      save_attrib_data(&head, GL_EVAL_BIT, attr);
> +      if (!push_attrib_helper(ctx, GL_EVAL_BIT, &head,
> +                              sizeof(struct gl_eval_attrib),
> +                              (void*)&ctx->Eval))
> +         goto end;
>      }
>
>      if (mask & GL_FOG_BIT) {
> -      struct gl_fog_attrib *attr;
> -      attr = MALLOC_STRUCT( gl_fog_attrib );
> -      memcpy( attr, &ctx->Fog, sizeof(struct gl_fog_attrib) );
> -      save_attrib_data(&head, GL_FOG_BIT, attr);
> +      if (!push_attrib_helper(ctx, GL_FOG_BIT, &head,
> +                              sizeof(struct gl_fog_attrib),
> +                              (void*)&ctx->Fog))
> +         goto end;
>      }
>
>      if (mask & GL_HINT_BIT) {
> -      struct gl_hint_attrib *attr;
> -      attr = MALLOC_STRUCT( gl_hint_attrib );
> -      memcpy( attr, &ctx->Hint, sizeof(struct gl_hint_attrib) );
> -      save_attrib_data(&head, GL_HINT_BIT, attr);
> +      if (!push_attrib_helper(ctx, GL_HINT_BIT, &head,
> +                              sizeof(struct gl_hint_attrib),
> +                              (void*)&ctx->Hint))
> +         goto end;
>      }
>
>      if (mask & GL_LIGHTING_BIT) {
> -      struct gl_light_attrib *attr;
> -      FLUSH_CURRENT(ctx, 0);	/* flush material changes */
> -      attr = MALLOC_STRUCT( gl_light_attrib );
> -      memcpy( attr, &ctx->Light, sizeof(struct gl_light_attrib) );
> -      save_attrib_data(&head, GL_LIGHTING_BIT, attr);
> +      FLUSH_CURRENT(ctx, 0);   /* flush material changes */
> +      if (!push_attrib_helper(ctx, GL_LIGHTING_BIT, &head,
> +                              sizeof(struct gl_light_attrib),
> +                              (void*)&ctx->Light))
> +         goto end;
>      }
>
>      if (mask & GL_LINE_BIT) {
> -      struct gl_line_attrib *attr;
> -      attr = MALLOC_STRUCT( gl_line_attrib );
> -      memcpy( attr, &ctx->Line, sizeof(struct gl_line_attrib) );
> -      save_attrib_data(&head, GL_LINE_BIT, attr);
> +      if (!push_attrib_helper(ctx, GL_LINE_BIT, &head,
> +                              sizeof(struct gl_line_attrib),
> +                              (void*)&ctx->Line))
> +         goto end;
>      }
>
>      if (mask & GL_LIST_BIT) {
> -      struct gl_list_attrib *attr;
> -      attr = MALLOC_STRUCT( gl_list_attrib );
> -      memcpy( attr, &ctx->List, sizeof(struct gl_list_attrib) );
> -      save_attrib_data(&head, GL_LIST_BIT, attr);
> +      if (!push_attrib_helper(ctx, GL_LIST_BIT, &head,
> +                              sizeof(struct gl_list_attrib),
> +                              (void*)&ctx->List))
> +         goto end;
>      }
>
>      if (mask & GL_PIXEL_MODE_BIT) {
>         struct gl_pixel_attrib *attr;
>         attr = MALLOC_STRUCT( gl_pixel_attrib );
> -      memcpy( attr, &ctx->Pixel, sizeof(struct gl_pixel_attrib) );
> -      /* push the Read FBO's ReadBuffer state, not ctx->Pixel.ReadBuffer */
> -      attr->ReadBuffer = ctx->ReadBuffer->ColorReadBuffer;
> -      save_attrib_data(&head, GL_PIXEL_MODE_BIT, attr);
> +      if (attr == NULL) {
> +         _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
> +         goto end;
> +      }
> +
> +      if (save_attrib_data(&head, GL_PIXEL_MODE_BIT, attr)) {
> +         memcpy( attr, &ctx->Pixel, sizeof(struct gl_pixel_attrib) );
> +         /* push the Read FBO's ReadBuffer state, not ctx->Pixel.ReadBuffer */
> +         attr->ReadBuffer = ctx->ReadBuffer->ColorReadBuffer;
> +      }
> +      else {
> +         FREE( attr );
> +         _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
> +         goto end;
> +      }
>      }
>
>      if (mask & GL_POINT_BIT) {
> -      struct gl_point_attrib *attr;
> -      attr = MALLOC_STRUCT( gl_point_attrib );
> -      memcpy( attr, &ctx->Point, sizeof(struct gl_point_attrib) );
> -      save_attrib_data(&head, GL_POINT_BIT, attr);
> +      if (!push_attrib_helper(ctx, GL_POINT_BIT, &head,
> +                              sizeof(struct gl_point_attrib),
> +                              (void*)&ctx->Point))
> +         goto end;
>      }
>
>      if (mask & GL_POLYGON_BIT) {
> -      struct gl_polygon_attrib *attr;
> -      attr = MALLOC_STRUCT( gl_polygon_attrib );
> -      memcpy( attr, &ctx->Polygon, sizeof(struct gl_polygon_attrib) );
> -      save_attrib_data(&head, GL_POLYGON_BIT, attr);
> +      if (!push_attrib_helper(ctx, GL_POLYGON_BIT, &head,
> +                              sizeof(struct gl_polygon_attrib),
> +                              (void*)&ctx->Polygon))
> +         goto end;
>      }
>
>      if (mask & GL_POLYGON_STIPPLE_BIT) {
> -      GLuint *stipple;
> -      stipple = malloc( 32*sizeof(GLuint) );
> -      memcpy( stipple, ctx->PolygonStipple, 32*sizeof(GLuint) );
> -      save_attrib_data(&head, GL_POLYGON_STIPPLE_BIT, stipple);
> +      if (!push_attrib_helper(ctx, GL_POLYGON_STIPPLE_BIT, &head,
> +                              sizeof(32*sizeof(GLuint)),

???

How about sizeof(ctx->PolygonStipple) ?


> +                              (void*)&ctx->PolygonStipple))
> +         goto end;
>      }
>
>      if (mask & GL_SCISSOR_BIT) {
> -      struct gl_scissor_attrib *attr;
> -      attr = MALLOC_STRUCT( gl_scissor_attrib );
> -      memcpy( attr, &ctx->Scissor, sizeof(struct gl_scissor_attrib) );
> -      save_attrib_data(&head, GL_SCISSOR_BIT, attr);
> +      if (!push_attrib_helper(ctx, GL_SCISSOR_BIT, &head,
> +                              sizeof(struct gl_scissor_attrib),
> +                              (void*)&ctx->Scissor))
> +         goto end;
>      }
>
>      if (mask & GL_STENCIL_BUFFER_BIT) {
> -      struct gl_stencil_attrib *attr;
> -      attr = MALLOC_STRUCT( gl_stencil_attrib );
> -      memcpy( attr, &ctx->Stencil, sizeof(struct gl_stencil_attrib) );
> -      save_attrib_data(&head, GL_STENCIL_BUFFER_BIT, attr);
> +      if (!push_attrib_helper(ctx, GL_STENCIL_BUFFER_BIT, &head,
> +                              sizeof(struct gl_stencil_attrib),
> +                              (void*)&ctx->Stencil))
> +         goto end;
>      }
>
>      if (mask & GL_TEXTURE_BIT) {
> @@ -426,6 +481,12 @@ _mesa_PushAttrib(GLbitfield mask)
>            goto end;
>         }
>
> +      if (!save_attrib_data(&head, GL_TEXTURE_BIT, texstate)) {
> +         FREE( texstate );
> +         _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib(GL_TEXTURE_BIT)");
> +         goto end;
> +      }
> +
>         _mesa_lock_context_textures(ctx);
>
>         /* copy/save the bulk of texture state here */
> @@ -452,35 +513,35 @@ _mesa_PushAttrib(GLbitfield mask)
>         _mesa_reference_shared_state(ctx, &texstate->SharedRef, ctx->Shared);
>
>         _mesa_unlock_context_textures(ctx);
> -
> -      save_attrib_data(&head, GL_TEXTURE_BIT, texstate);
>      }
>
>      if (mask & GL_TRANSFORM_BIT) {
> -      struct gl_transform_attrib *attr;
> -      attr = MALLOC_STRUCT( gl_transform_attrib );
> -      memcpy( attr, &ctx->Transform, sizeof(struct gl_transform_attrib) );
> -      save_attrib_data(&head, GL_TRANSFORM_BIT, attr);
> +      if (!push_attrib_helper(ctx, GL_TRANSFORM_BIT, &head,
> +                              sizeof(struct gl_transform_attrib),
> +                              (void*)&ctx->Transform))
> +         goto end;
>      }
>
>      if (mask & GL_VIEWPORT_BIT) {
> -      struct gl_viewport_attrib *attr;
> -      attr = MALLOC_STRUCT( gl_viewport_attrib );
> -      memcpy( attr, &ctx->Viewport, sizeof(struct gl_viewport_attrib) );
> -      save_attrib_data(&head, GL_VIEWPORT_BIT, attr);
> +      if (!push_attrib_helper(ctx, GL_VIEWPORT_BIT, &head,
> +                              sizeof(struct gl_viewport_attrib),
> +                              (void*)&ctx->Viewport))
> +         goto end;
>      }
>
>      /* GL_ARB_multisample */
>      if (mask & GL_MULTISAMPLE_BIT_ARB) {
> -      struct gl_multisample_attrib *attr;
> -      attr = MALLOC_STRUCT( gl_multisample_attrib );
> -      memcpy( attr, &ctx->Multisample, sizeof(struct gl_multisample_attrib) );
> -      save_attrib_data(&head, GL_MULTISAMPLE_BIT_ARB, attr);
> +      if (!push_attrib_helper(ctx, GL_MULTISAMPLE_BIT_ARB, &head,
> +                              sizeof(struct gl_multisample_attrib),
> +                              (void*)&ctx->Multisample))
> +         goto end;
>      }
>
>   end:
> -   ctx->AttribStack[ctx->AttribStackDepth] = head;
> -   ctx->AttribStackDepth++;
> +   if (head != NULL) {
> +       ctx->AttribStack[ctx->AttribStackDepth] = head;
> +       ctx->AttribStackDepth++;
> +   }
>   }
>
>
>



More information about the mesa-dev mailing list