[Mesa-dev] [PATCH 1/9] ralloc: Allow reparenting to a NULL context

[Jason Ekstrand]

On Thu, Aug 17, 2017 at 5:49 PM, Kenneth Graunke <kenneth at 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 <kenneth at 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170817/8d286403/attachment.html>