This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
Home Products Support Community News


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

To: Peter Teoh <htmldeveloper@xxxxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxx>
Subject: Re: [Xen-devel] Re: Xenstored: eliminate double free and fixes for memory leak
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
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
In-reply-to: <804dabb00709170007j3051fe7ahde41205423735499@xxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: Acf5DNKBERvpQmUAEdyfogAX8io7RQ==
Thread-topic: [Xen-devel] Re: Xenstored: eliminate double free and fixes for memory leak
User-agent: Microsoft-Entourage/
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

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