[PATCH] tiddyup tools/xenstore/talloc.c

[PATCH] tiddyup tools/xenstore/talloc.c

[Jerone Young]

Add check so that p is not operated on if context is not also null.

Signed-off-by: Jerone Young <jyoung5@us.ibm.com>

--- tools/xenstore/talloc.c.old 2005-08-11 11:48:59.000000000 -0500
+++ tools/xenstore/talloc.c     2005-08-11 12:30:14.000000000 -0500
@@ -292,7 +292,7 @@ static int talloc_unreference(const void

        for (h=tc->refs;h;h=h->next) {
                struct talloc_chunk *p = talloc_parent_chunk(h);
-               if ((p==NULL && context==NULL) || p+1 == context) break;
+               if (p==NULL || context==NULL || p+1 == context) break;
        }
        if (h == NULL) {
                return -1;


-- 
Jerone Young
IBM Linux Technology Center
jyoung5@us.ibm.com
512-838-1157 (T/L: 678-1157)

Re: [PATCH] tiddyup tools/xenstore/talloc.c

[Keir Fraser]

On 11 Aug 2005, at 22:15, Jerone Young wrote:

> Add check so that p is not operated on if context is not also null.
>
> Signed-off-by: Jerone Young <jyoung5@us.ibm.com>

No idea on this one. I think that case is probably impossible so the 
test could just be simplified, but probably only tridge knows for sure. 
:-)

I think xenstore can probably be weaned off talloc at some point.

  -- Keir

Re: [PATCH] tiddyup tools/xenstore/talloc.c

[Jerone Young]

On Thu, 2005-08-11 at 22:58 +0100, Keir Fraser wrote:
> On 11 Aug 2005, at 22:15, Jerone Young wrote:
> 
> > Add check so that p is not operated on if context is not also null.
> >
> > Signed-off-by: Jerone Young <jyoung5@us.ibm.com>
> 
> No idea on this one. I think that case is probably impossible so the 
> test could just be simplified, but probably only tridge knows for sure. 
> :-)

I agree..the case is very very very not likely but I figured I would
submit the patch anyway so our tree was nice and safe ;-)

> 
> I think xenstore can probably be weaned off talloc at some point.
> 
>   -- Keir
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
> 
-- 
Jerone Young
IBM Linux Technology Center
jyoung5@us.ibm.com
512-838-1157 (T/L: 678-1157)

Re: [PATCH] tiddyup tools/xenstore/talloc.c

[Rusty Russell]

On Thu, 2005-08-11 at 22:58 +0100, Keir Fraser wrote:
> On 11 Aug 2005, at 22:15, Jerone Young wrote:
> 
> > Add check so that p is not operated on if context is not also null.
> >
> > Signed-off-by: Jerone Young <jyoung5@us.ibm.com>
> 
> No idea on this one. I think that case is probably impossible so the 
> test could just be simplified, but probably only tridge knows for sure. 
> :-)

Actually the code is fine: it looks for a reference to the given parent
context.  p + 1 cannot be NULL, since either p is NULL, or (p + 1) has
at some stage been returned by _talloc(), which cannot return NULL.

> I think xenstore can probably be weaned off talloc at some point.

Heh, good luck with that!  Removing the destructors and replacing them
with explicit calls would not be too complicated, as the structures are
all fairly simple, but you'll be chasing down memory leaks for a long
time through all the failure paths.

You will have lots and lots of fun handling out-of-memory conditions,
which we do at the moment although, regrettably, not everywhere: if
destroy_path() fails we abort the daemon 8(.

Actually, the OOM code is ugly and I'd love to get rid of it, but I know
people like these kind of checks, even if they're never 100%.

Rusty.
-- 
A bad analogy is like a leaky screwdriver -- Richard Braakman