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

[Kenneth Graunke]

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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170817/833d9142/attachment.sig>