On Thu, Aug 17, 2017 at 5:49 PM, Kenneth Graunke <kenn...@whitecape.org> wrote:
> On Thursday, August 17, 2017 4:54:05 PM PDT Timothy Arceri wrote: > > > > On 18/08/17 09:05, Kenneth Graunke wrote: > > > On Thursday, August 17, 2017 10:22:15 AM PDT Jason Ekstrand wrote: > > >> --- > > >> src/util/ralloc.c | 2 +- > > >> 1 file changed, 1 insertion(+), 1 deletion(-) > > >> > > >> diff --git a/src/util/ralloc.c b/src/util/ralloc.c > > >> index bf46439..4015c7e 100644 > > >> --- a/src/util/ralloc.c > > >> +++ b/src/util/ralloc.c > > >> @@ -285,7 +285,7 @@ ralloc_steal(const void *new_ctx, void *ptr) > > >> return; > > >> > > >> info = get_header(ptr); > > >> - parent = get_header(new_ctx); > > >> + parent = new_ctx ? get_header(new_ctx) : NULL; > > >> > > >> unlink_block(info); > > >> > > >> > > > > > > This patch is: > > > Reviewed-by: Kenneth Graunke <kenn...@whitecape.org> > > > > > > ralloc_adopt() doesn't properly handle NULL either, but frankly... > > > reparenting an unknown set of children to the NULL context sounds like > a > > > recipe for leaks. :) > > > > I agree. I was wondering if we should create a special function for this: > > > > ralloc_steal_to_NULL_ctx(void *ptr) > > > > To void issues like: > > > > ralloc_free(parent->child); > > parent->child = NULL; > > > > ... lots of code in between ... > > > > ralloc_steal(parent->child, ptr); > > > > ralloc_free(parent); > > > > Feel free to tell me I'm being paranoid. > > I think steal to NULL is fine, because you're only stealing one item, and > you can easily hold on to a pointer and free it later. Adopt is bad > because > it steals a whole bunch of things, so unless you had a reference to all of > them, you've effectively leaked them. And if you had such a list, you > could > just steal them all and not use adopt in the first place. > > I'm not really sure why steal-to-NULL is useful, but I guess it can be. > The next patch makes us parent the nir_shader to the vtn_builder while we're doing SPIR-V -> NIR conversion. At the end, we unparent the nir_shader, delete the vtn_builder, and return the shader. if we error out, it means that all we have to do is delete the vtn_builder. I also considered adding a "if (b->shader) ralloc_free(b->shader);" which would work just as wel. --Jason
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
