[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] Re: Xenstored: eliminate double free and fixes for memory leak


  • To: Peter Teoh <htmldeveloper@xxxxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxx>
  • From: Keir Fraser <Keir.Fraser@xxxxxxxxxxxx>
  • Date: Mon, 17 Sep 2007 10:26:27 +0100
  • Delivery-date: Mon, 17 Sep 2007 02:27:07 -0700
  • List-id: Xen developer discussion <xen-devel.lists.xensource.com>
  • Thread-index: Acf5DNKBERvpQmUAEdyfogAX8io7RQ==
  • Thread-topic: [Xen-devel] Re: Xenstored: eliminate double free and fixes for memory leak

I don't believe there are any memory leaks in xenstored -- talloc() works
somewhat differently than malloc() in that each allocation can specify a
parent 'context'. This creates a hierarchy of allocated memory blocks: any
memory block freed also automatically frees its children. When making a
talloc*() call, the first argument is usually the parent context or parent
allocation.

So, in the case of canonicalize(), either it returns the passed-in node, or
it returns a new string as a child of node. In the latter case, when the
original node string is freed, any string allocated by canonicalize() will
also be freed. If you trace any of your other leaks you should find that the
memory is actually freed, probably by the talloc_free() in
consider_message(). Take a look at xenstore/talloc_guide.txt for more info
about talloc.

As for not checking allocations for returning NULL, this is consistently not
done throughout xenstored. Mainly because there isn't much we can do in that
case, except perhaps log the error and then exit (which basically takes out
the host, as xenstored cannot be restarted). Rather than adding NULL checks
throughout xenstored, it would be acceptable to add a NULL check in
_talloc() which then logs and exits 'cleanly'.

 -- Keir


On 17/9/07 08:07, "Peter Teoh" <htmldeveloper@xxxxxxxxx> wrote:

> On top of abovementioned problems I missed out this one:
> 
> canonicalize() may return an existing pointer as a value - so no
> memory deallocation is needed, but it also may return a newly
> allocated memory - for which deallocation is needed.   And currently
> all the results returned from canonicalize() are not freed.
> 
> Please let me know how to fix this.
> 
> Thanks.
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.