[Freeipa-devel] [PATCH] Make Data Provider a mandatory service

Simo Sorce ssorce at redhat.com
Thu May 28 14:01:57 UTC 2009


On Thu, 2009-05-28 at 15:14 +0200, Jakub Hrozek wrote:
> On Thu, 2009-05-28 at 08:33 -0400, Stephen Gallagher wrote:
> > You're right, I wasn't paying attention. Please make on additional
> > change regarding the realloc. If talloc_realloc returns NULL, you've
> > leaked the original memory, since it's only freed on success. It
> > should
> > look something like:
> > 
> > char *tmp_array = talloc_realloc(ctx, ctx->services, char *, i+2)
> > if (tmp_array == NULL) {
> >     return ENOMEM;
> > }
> > ctx->services = tmp_array;
> > 
> > At least this way, the ctx->services value remains reachable.
> > 
> 
> You are right, done and attached. To my defense, I've seen this
> potentially bad usage elsewhere in the code (i.e. confdb.c:372).

There is a fundamental difference in confdb.c, there we use a mem_ctx we
always free at the end of the function, so we are not leaking memory,
and we are not interested in the original value of the memory if we fail
that realloc.

So that use of talloc_realloc() is correct in that context.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York




More information about the Freeipa-devel mailing list