Hi nobled, <div class="gmail_quote">On Fri, Dec 3, 2010 at 12:08 AM, nobled <<a href="mailto:nobled@dreamwidth.org">nobled@dreamwidth.org</a>> wrote: <blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;"> Minimize code duplication and avoid null-free in case any of the allocations failed. --- src/gallium/drivers/r300/r300_context.c | 30 +++++++++--------------------- 1 files changed, 9 insertions(+), 21 deletions(-) diff --git a/src/gallium/drivers/r300/r300_context.c b/src/gallium/drivers/r300/r300_context.c index 2b06a24..757da60 100644 --- a/src/gallium/drivers/r300/r300_context.c +++ b/src/gallium/drivers/r300/r300_context.c @@ -139,27 +139,11 @@ r300_update_num_contexts(r300->screen, -1); - /* Free the structs allocated in r300_setup_atoms() */ - if (r300->aa_state.state) { - FREE(r300->aa_state.state); - FREE(r300->blend_color_state.state); - FREE(r300->clip_state.state); - FREE(r300->fb_state.state); - FREE(r300->gpu_flush.state); - FREE(r300->hyperz_state.state); - FREE(r300->invariant_state.state); - FREE(r300->rs_block_state.state); - FREE(r300->scissor_state.state); - FREE(r300->textures_state.state); - FREE(r300->vap_invariant_state.state); - FREE(r300->viewport_state.state); - FREE(r300->ztop_state.state); - FREE(r300->fs_constants.state); - FREE(r300->vs_constants.state); - if (!r300->screen->caps.has_tcl) { - FREE(r300->vertex_stream_state.state); - } - } + /* Free the structs allocated in r300_setup_atoms(). */ + foreach(atom, &r300->atom_list) + if (atom->state != NULL) + FREE(atom->state); + </blockquote><div> I don't think this is correct. We should only release those atoms we allocate manually (as is listed in the code). There are also the atoms in "atom_list" which are set via the pipe_context::bind_* functions and should not be released here. Instead, they should be released by a state tracker using the appropriate delete_* functions. Moreover, free(NULL) is legal and has no effect. I will commit the other patches. Thanks. Best regards Marek </div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;"> FREE(r300); } @@ -420,11 +404,15 @@ r300->context.destroy = r300_destroy_context; make_empty_list(&r300->query_list); + make_empty_list(&r300->atom_list); util_slab_create(&r300->pool_transfers, sizeof(struct pipe_transfer), 64, UTIL_SLAB_SINGLETHREADED); + /* The above calls must not fail, since r300_destroy_context() + depends on them. */ + r300->cs = rws->cs_create(rws); if (r300->cs == NULL) goto fail; -- 1.7.0.4 _______________________________________________ mesa-dev mailing list <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div>